diff --git a/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java b/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java index 9948b09b0..78f1d4156 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java @@ -44,7 +44,6 @@ import org.fdroid.fdroid.installer.InstallManagerService; import org.fdroid.fdroid.installer.InstallerService; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.DownloaderFactory; -import org.fdroid.fdroid.net.TreeUriDownloader; import org.xml.sax.InputSource; import org.xml.sax.SAXException; import org.xml.sax.XMLReader; @@ -116,11 +115,7 @@ public class IndexUpdater { } protected String getIndexUrl(@NonNull Repo repo) { - if (repo.address.startsWith("content://")) { - return repo.address + TreeUriDownloader.ESCAPED_SLASH + SIGNED_FILE_NAME; - } else { - return repo.address + "/" + SIGNED_FILE_NAME; - } + return repo.getFileUrl(SIGNED_FILE_NAME); } public boolean hasChanged() { diff --git a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java index 6ed2faf17..187750581 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java +++ b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java @@ -24,7 +24,7 @@ package org.fdroid.fdroid; import android.content.ContentValues; import android.content.Context; -import android.net.Uri; + import androidx.annotation.NonNull; import android.text.TextUtils; import android.util.Log; @@ -97,15 +97,8 @@ public class IndexV1Updater extends IndexUpdater { } @Override - /** - * Storage Access Framework URLs have a crazy encoded path within the URL path. - */ protected String getIndexUrl(@NonNull Repo repo) { - if (repo.address.startsWith("content://")) { - return repo.address + "%2F" + SIGNED_FILE_NAME; - } else { - return Uri.parse(repo.address).buildUpon().appendPath(SIGNED_FILE_NAME).build().toString(); - } + return repo.getFileUrl(SIGNED_FILE_NAME); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index eedd9110b..3d146d645 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -120,7 +120,7 @@ public final class Utils { private static Handler toastHandler; - public static final String FALLBACK_ICONS_DIR = "/icons/"; + public static final String FALLBACK_ICONS_DIR = "icons"; /* * @param dpiMultiplier Lets you grab icons for densities larger or @@ -132,22 +132,22 @@ public final class Utils { final DisplayMetrics metrics = context.getResources().getDisplayMetrics(); final double dpi = metrics.densityDpi * dpiMultiplier; if (dpi >= 640) { - return "/icons-640/"; + return "icons-640"; } if (dpi >= 480) { - return "/icons-480/"; + return "icons-480"; } if (dpi >= 320) { - return "/icons-320/"; + return "icons-320"; } if (dpi >= 240) { - return "/icons-240/"; + return "icons-240"; } if (dpi >= 160) { - return "/icons-160/"; + return "icons-160"; } - return "/icons-120/"; + return "icons-120"; } /** diff --git a/app/src/main/java/org/fdroid/fdroid/data/Apk.java b/app/src/main/java/org/fdroid/fdroid/data/Apk.java index be868e740..85efaa89e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Apk.java @@ -58,7 +58,7 @@ public class Apk extends ValueObject implements Comparable, Parcelable { // these are never set by the Apk/package index metadata @JsonIgnore - String repoAddress; + protected String repoAddress; @JsonIgnore int repoVersion; @JsonIgnore @@ -280,7 +280,8 @@ public class Apk extends ValueObject implements Comparable, Parcelable { @JsonIgnore // prevent tests from failing due to nulls in checkRepoAddress() public String getCanonicalUrl() { checkRepoAddress(); - return repoAddress + "/" + apkName.replace(" ", "%20"); + Repo repo = new Repo(repoAddress); + return repo.getFileUrl(apkName); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index dbbf2d898..9e69bd899 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -735,9 +735,9 @@ public class App extends ValueObject implements Comparable, Parcelable { } else { iconsDir = Utils.FALLBACK_ICONS_DIR; } - return repo.address + iconsDir + iconFromApk; + return repo.getFileUrl(iconsDir, iconFromApk); } - return repo.address + "/" + packageName + "/" + iconUrl; + return repo.getFileUrl(packageName, iconUrl); } public String getFeatureGraphicUrl(Context context) { @@ -745,7 +745,7 @@ public class App extends ValueObject implements Comparable, Parcelable { return null; } Repo repo = RepoProvider.Helper.findById(context, repoId); - return repo.address + "/" + packageName + "/" + featureGraphic; + return repo.getFileUrl(packageName, featureGraphic); } public String getPromoGraphic(Context context) { @@ -753,7 +753,7 @@ public class App extends ValueObject implements Comparable, Parcelable { return null; } Repo repo = RepoProvider.Helper.findById(context, repoId); - return repo.address + "/" + packageName + "/" + promoGraphic; + return repo.getFileUrl(packageName, promoGraphic); } public String getTvBanner(Context context) { @@ -761,7 +761,7 @@ public class App extends ValueObject implements Comparable, Parcelable { return null; } Repo repo = RepoProvider.Helper.findById(context, repoId); - return repo.address + "/" + packageName + "/" + tvBanner; + return repo.getFileUrl(packageName, tvBanner); } public String[] getAllScreenshots(Context context) { @@ -785,7 +785,7 @@ public class App extends ValueObject implements Comparable, Parcelable { String[] result = new String[list.size()]; int i = 0; for (String url : list) { - result[i] = repo.address + "/" + packageName + "/" + url; + result[i] = repo.getFileUrl(packageName, url); i++; } return result; diff --git a/app/src/main/java/org/fdroid/fdroid/data/Repo.java b/app/src/main/java/org/fdroid/fdroid/data/Repo.java index 658715f5c..05593fd82 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Repo.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Repo.java @@ -25,12 +25,14 @@ package org.fdroid.fdroid.data; import android.content.ContentValues; import android.database.Cursor; +import android.net.Uri; import android.text.TextUtils; import com.fasterxml.jackson.annotation.JsonIgnore; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Schema.RepoTable.Cols; +import org.fdroid.fdroid.net.TreeUriDownloader; import java.net.MalformedURLException; import java.net.URL; @@ -141,6 +143,10 @@ public class Repo extends ValueObject { public Repo() { } + public Repo(String address) { + this.address = address; + } + public Repo(Cursor cursor) { checkCursorPosition(cursor); @@ -263,6 +269,40 @@ public class Repo extends ValueObject { return tempName; } + public String getFileUrl(String... pathElements) { + /* Each String in pathElements might contain a /, should keep these as path elements */ + List elements = new ArrayList(); + for (String element : pathElements) { + for (String elementPart : element.split("/")) { + elements.add(elementPart); + } + } + + /** + * Storage Access Framework URLs have this wacky URL-encoded path within the URL path. + * + * i.e. + * content://authority/tree/313E-1F1C%3A/document/313E-1F1C%3Aguardianproject.info%2Ffdroid%2Frepo + * + * Currently don't know a better way to identify these than by content:// prefix, + * seems the Android SDK expects apps to consider them as opaque identifiers. + */ + if (address.startsWith("content://")) { + StringBuilder result = new StringBuilder(address); + for (String element : elements) { + result.append(TreeUriDownloader.ESCAPED_SLASH); + result.append(element); + } + return result.toString(); + } else { // Normal URL + Uri.Builder result = Uri.parse(address).buildUpon(); + for (String element : elements) { + result.appendPath(element); + } + return result.build().toString(); + } + } + private static int toInt(Integer value) { if (value == null) { return 0; diff --git a/app/src/test/java/org/fdroid/fdroid/RepoUrlsTest.java b/app/src/test/java/org/fdroid/fdroid/RepoUrlsTest.java new file mode 100644 index 000000000..94b72015f --- /dev/null +++ b/app/src/test/java/org/fdroid/fdroid/RepoUrlsTest.java @@ -0,0 +1,152 @@ +/* + * Copyright (C) 2021 Angus Gratton + * Copyright (C) 2018 Senecto Limited + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 3 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.fdroid.fdroid; + +import org.fdroid.fdroid.data.Apk; +import org.fdroid.fdroid.data.FDroidProviderTest; +import org.fdroid.fdroid.data.Repo; +import org.fdroid.fdroid.mock.MockApk; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +import static org.junit.Assert.assertEquals; + +@RunWith(RobolectricTestRunner.class) +public class RepoUrlsTest extends FDroidProviderTest { + public static final String TAG = "RepoUrlsTest"; + + /** + * Private class describing a repository URL we're going to test, and + * the file pattern for any files within that URL. + */ + private static class TestRepo { + // Repo URL for the test case + public String repoUrl; + // String format pattern for generating file URLs, should contain a single %s for the filename + public String fileUrlPattern; + + TestRepo(String repoUrl, String fileUrlPattern) { + this.repoUrl = repoUrl; + this.fileUrlPattern = fileUrlPattern; + } + } + + private static final String APK_NAME = "test-v1.apk"; + + private static final TestRepo[] REPOS = { + new TestRepo( + "https://microg.org/fdroid/repo", + "https://microg.org/fdroid/repo/%s"), + new TestRepo( + "http://bdf2wcxujkg6qqff.onion/fdroid/repo", + "http://bdf2wcxujkg6qqff.onion/fdroid/repo/%s"), + new TestRepo( + "http://lysator7eknrfl47rlyxvgeamrv7ucefgrrlhk7rouv3sna25asetwid.onion/pub/fdroid/repo", + "http://lysator7eknrfl47rlyxvgeamrv7ucefgrrlhk7rouv3sna25asetwid.onion/pub/fdroid/repo/%s"), + new TestRepo( + "https://mirrors.nju.edu.cn/fdroid/repo?fingerprint=43238D512C1E5EB2D6569F4A3AFBF5523418B82E0A3ED1552770ABB9A9C9CCAB", + "https://mirrors.nju.edu.cn/fdroid/repo/%s?fingerprint=43238D512C1E5EB2D6569F4A3AFBF5523418B82E0A3ED1552770ABB9A9C9CCAB"), + new TestRepo( + "https://raw.githubusercontent.com/guardianproject/fdroid-repo/master/fdroid/repo", + "https://raw.githubusercontent.com/guardianproject/fdroid-repo/master/fdroid/repo/%s"), + new TestRepo( + "content://com.android.externalstorage.documents/tree/1AFB-2402%3A/document/1AFB-2402%3Atesty.at.or.at%2Ffdroid%2Frepo", + // note: to have a URL-escaped path in a format string pattern, we need to + // %-escape all URL % + "content://com.android.externalstorage.documents/tree/1AFB-2402%%3A/document/1AFB-2402%%3Atesty.at.or.at%%2Ffdroid%%2Frepo%%2F%s"), + new TestRepo( + "content://authority/tree/313E-1F1C%3A/document/313E-1F1C%3Aguardianproject.info%2Ffdroid%2Frepo", + // note: to have a URL-escaped path in a format string pattern, we need to + // %-escape all URL % + "content://authority/tree/313E-1F1C%%3A/document/313E-1F1C%%3Aguardianproject.info%%2Ffdroid%%2Frepo%%2F%s"), + new TestRepo( + "http://10.20.31.244:8888/fdroid/repo?FINGERPRINT=35521D88285A9D06FBE33D35FB8B4BB872D753666CF981728E2249FEE6D2D0F2&SWAP=1&BSSID=FE:EE:DA:45:2D:4E", + "http://10.20.31.244:8888/fdroid/repo/%s?FINGERPRINT=35521D88285A9D06FBE33D35FB8B4BB872D753666CF981728E2249FEE6D2D0F2&SWAP=1&BSSID=FE:EE:DA:45:2D:4E"), + new TestRepo( + "fdroidrepos://briarproject.org/fdroid/repo?fingerprint=1FB874BEE7276D28ECB2C9B06E8A122EC4BCB4008161436CE474C257CBF49BD6", + "fdroidrepos://briarproject.org/fdroid/repo/%s?fingerprint=1FB874BEE7276D28ECB2C9B06E8A122EC4BCB4008161436CE474C257CBF49BD6"), + }; + + @Before + public void setup() { + Preferences.setupForTests(context); + } + + interface GetFileFromRepo { + String get(TestRepo tr); + } + + /** + * Utility test function - go through the list of test repos, + * using the useOfRepo interface to instantiate a repo from the URL + * and return a file of some kind (Apk, index, etc.) and check that + * it matches the test repo's expected URL format. + * + * @param fileName File that 'useOfRepo' will return in the repo, when called + * @param useOfRepo Instance of the function that uses the repo to build a file URL + */ + private void testReposWithFile(String fileName, GetFileFromRepo useOfRepo) { + for (TestRepo tr : REPOS) { + String expectedUrl = String.format(tr.fileUrlPattern, fileName); + System.out.println("Testing URL " + expectedUrl); + String actualUrl = useOfRepo.get(tr); + assertEquals(expectedUrl, actualUrl); + } + } + + @Test + public void testIndexUrls() { + testReposWithFile(IndexUpdater.SIGNED_FILE_NAME, new GetFileFromRepo() { + @Override + public String get(TestRepo tr) { + Repo repo = new Repo(); + repo.address = tr.repoUrl; + IndexUpdater updater = new IndexUpdater(context, repo); + return updater.getIndexUrl(repo); + } + }); + } + + @Test + public void testIndexV1Urls() { + testReposWithFile(IndexV1Updater.SIGNED_FILE_NAME, new GetFileFromRepo() { + @Override + public String get(TestRepo tr) { + Repo repo = new Repo(); + repo.address = tr.repoUrl; + IndexV1Updater updater = new IndexV1Updater(context, repo); + return updater.getIndexUrl(repo); + } + }); + } + + @Test + public void testApkUrls() { + testReposWithFile(APK_NAME, new GetFileFromRepo() { + @Override + public String get(TestRepo tr) { + Apk apk = new MockApk(APK_NAME, 1, tr.repoUrl, APK_NAME); + return apk.getCanonicalUrl(); + } + }); + } +} diff --git a/app/src/test/java/org/fdroid/fdroid/mock/MockApk.java b/app/src/test/java/org/fdroid/fdroid/mock/MockApk.java index 35592526e..e26379181 100644 --- a/app/src/test/java/org/fdroid/fdroid/mock/MockApk.java +++ b/app/src/test/java/org/fdroid/fdroid/mock/MockApk.java @@ -10,6 +10,13 @@ public class MockApk extends Apk { this.versionCode = versionCode; } + public MockApk(String id, int versionCode, String repoAddress, String apkName) { + this.packageName = id; + this.versionCode = versionCode; + this.repoAddress = repoAddress; + this.apkName = apkName; + } + public MockApk(App app, int versionCode) { this.appId = app.getId(); this.versionCode = versionCode;