From 3cb6cc747b3b3f0e9039b1d3bf065c9fbbcad961 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Sun, 8 Nov 2020 19:00:28 +1100 Subject: [PATCH] Add Repo.getFileUrl() method to get file URL in a standard way --- .../java/org/fdroid/fdroid/IndexUpdater.java | 7 +-- .../org/fdroid/fdroid/IndexV1Updater.java | 11 +---- .../main/java/org/fdroid/fdroid/Utils.java | 14 +++--- .../main/java/org/fdroid/fdroid/data/Apk.java | 3 +- .../main/java/org/fdroid/fdroid/data/App.java | 12 +++--- .../java/org/fdroid/fdroid/data/Repo.java | 43 +++++++++++++++++++ .../java/org/fdroid/fdroid/RepoUrlsTest.java | 34 +++++++-------- 7 files changed, 78 insertions(+), 46 deletions(-) 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 451d0a768..85efaa89e 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Apk.java @@ -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..0a4c58e59 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Repo.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Repo.java @@ -25,13 +25,16 @@ 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.lang.reflect.Array; import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; @@ -141,6 +144,10 @@ public class Repo extends ValueObject { public Repo() { } + public Repo(String address) { + this.address = address; + } + public Repo(Cursor cursor) { checkCursorPosition(cursor); @@ -263,6 +270,42 @@ 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 index c8e32467d..6beb8385b 100644 --- a/app/src/test/java/org/fdroid/fdroid/RepoUrlsTest.java +++ b/app/src/test/java/org/fdroid/fdroid/RepoUrlsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018 Senecto Limited + * Copyright (C) 2018-2021 Senecto Limited * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -43,8 +43,13 @@ import static org.junit.Assert.assertEquals; public class RepoUrlsTest extends FDroidProviderTest { private 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; public TestRepo(String repoUrl, String fileUrlPattern) @@ -74,11 +79,13 @@ public class RepoUrlsTest extends FDroidProviderTest { "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 escaped URL-encoding % in format string patterns + // 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 escaped URL-encoding % in format string patterns + // 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", @@ -97,6 +104,13 @@ public class RepoUrlsTest extends FDroidProviderTest { 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) { @@ -146,18 +160,4 @@ public class RepoUrlsTest extends FDroidProviderTest { } }); } - - private Apk insertApk(String packageName, String name, String repoAddress) { - - App app = Assert.insertApp(context, packageName, name); - - ContentValues additionalValues = new ContentValues(); - additionalValues.put(ApkTable.Cols.Repo.ADDRESS, repoAddress); - - Uri contentUri = Assert.insertApk(context, app, 1, additionalValues); - Cursor queryCursor = contentResolver.query(contentUri, ApkTable.Cols.ALL, null, null, null); - queryCursor.moveToFirst(); - return new Apk(queryCursor); - } - }