From 9522012fe15231f665152fc0a3b52352343abcea Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 18 Feb 2019 20:48:21 +0100 Subject: [PATCH 1/7] use built-in constants for HTTP status codes --- .../org/fdroid/fdroid/net/bluetooth/BluetoothServer.java | 7 +++---- .../main/java/org/fdroid/fdroid/net/HttpDownloader.java | 4 ++-- .../org/fdroid/fdroid/net/bluetooth/httpish/Response.java | 3 ++- .../java/org/fdroid/fdroid/views/ManageReposActivity.java | 7 +++---- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/src/full/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java b/app/src/full/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java index 86d8a987d..e4410f095 100644 --- a/app/src/full/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java +++ b/app/src/full/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java @@ -5,7 +5,7 @@ import android.bluetooth.BluetoothServerSocket; import android.bluetooth.BluetoothSocket; import android.util.Log; import android.webkit.MimeTypeMap; - +import fi.iki.elonen.NanoHTTPD; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.localrepo.type.BluetoothSwap; import org.fdroid.fdroid.net.bluetooth.httpish.Request; @@ -15,13 +15,12 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.HttpURLConnection; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import fi.iki.elonen.NanoHTTPD; - /** * Act as a layer on top of LocalHTTPD server, by forwarding requests served * over bluetooth to that server. @@ -157,7 +156,7 @@ public class BluetoothServer extends Thread { Response.Builder builder = null; try { - int statusCode = 404; + int statusCode = HttpURLConnection.HTTP_NOT_FOUND; int totalSize = -1; if (request.getMethod().equals(Request.Methods.HEAD)) { diff --git a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java index 62dc3a56f..9cde774ef 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java @@ -115,7 +115,7 @@ public class HttpDownloader extends Downloader { tmpConn.disconnect(); newFileAvailableOnServer = false; switch (statusCode) { - case 200: + case HttpURLConnection.HTTP_OK: contentLength = tmpConn.getContentLength(); if (!TextUtils.isEmpty(etag) && etag.equals(cacheTag)) { Utils.debugLog(TAG, urlString + " is cached, not downloading"); @@ -123,7 +123,7 @@ public class HttpDownloader extends Downloader { } newFileAvailableOnServer = true; break; - case 404: + case HttpURLConnection.HTTP_NOT_FOUND: notFound = true; return; default: diff --git a/app/src/main/java/org/fdroid/fdroid/net/bluetooth/httpish/Response.java b/app/src/main/java/org/fdroid/fdroid/net/bluetooth/httpish/Response.java index 359a81b05..dd999a14a 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/bluetooth/httpish/Response.java +++ b/app/src/main/java/org/fdroid/fdroid/net/bluetooth/httpish/Response.java @@ -12,6 +12,7 @@ import java.io.InputStream; import java.io.OutputStreamWriter; import java.io.UnsupportedEncodingException; import java.io.Writer; +import java.net.HttpURLConnection; import java.util.HashMap; import java.util.Locale; import java.util.Map; @@ -132,7 +133,7 @@ public class Response { public static class Builder { private InputStream contentStream; - private int statusCode = 200; + private int statusCode = HttpURLConnection.HTTP_OK; private int fileSize = -1; private String etag; diff --git a/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java b/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java index 8e044c17e..d484c2f97 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java @@ -571,8 +571,6 @@ public class ManageReposActivity extends AppCompatActivity private int statusCode = -1; private static final int REFRESH_DIALOG = Integer.MAX_VALUE; - private static final int HTTP_UNAUTHORIZED = 401; - private static final int HTTP_OK = 200; @Override protected String doInBackground(String... params) { @@ -630,7 +628,8 @@ public class ManageReposActivity extends AppCompatActivity statusCode = connection.getResponseCode(); - return statusCode == HTTP_UNAUTHORIZED || statusCode == HTTP_OK; + return statusCode == HttpURLConnection.HTTP_UNAUTHORIZED + || statusCode == HttpURLConnection.HTTP_OK; } @Override @@ -644,7 +643,7 @@ public class ManageReposActivity extends AppCompatActivity if (addRepoDialog.isShowing()) { - if (statusCode == HTTP_UNAUTHORIZED) { + if (statusCode == HttpURLConnection.HTTP_UNAUTHORIZED) { final View view = getLayoutInflater().inflate(R.layout.login, null); final AlertDialog credentialsDialog = new AlertDialog.Builder(context) From 9323ccdfd14e7885b42dab9bac2b789308ac4566 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 18 Feb 2019 22:08:59 +0100 Subject: [PATCH 2/7] add HTTP Last-Modified header to nearby/swap webserver This should support the new cache check scheme when using swap repos. --- .../java/org/fdroid/fdroid/net/LocalHTTPD.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/src/full/java/org/fdroid/fdroid/net/LocalHTTPD.java b/app/src/full/java/org/fdroid/fdroid/net/LocalHTTPD.java index b7a522261..8315909ee 100644 --- a/app/src/full/java/org/fdroid/fdroid/net/LocalHTTPD.java +++ b/app/src/full/java/org/fdroid/fdroid/net/LocalHTTPD.java @@ -50,13 +50,18 @@ import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; +import java.text.DateFormat; +import java.text.SimpleDateFormat; import java.util.Arrays; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.StringTokenizer; +import java.util.TimeZone; /** * A HTTP server for serving the files that are being swapped via WiFi, etc. @@ -79,6 +84,14 @@ public class LocalHTTPD extends NanoHTTPD { protected List rootDirs; + // Date format specified by RFC 7231 section 7.1.1.1. + private static final DateFormat RFC_1123 = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss 'GMT'", Locale.US); + + static { + RFC_1123.setLenient(false); + RFC_1123.setTimeZone(TimeZone.getTimeZone("GMT")); + } + /** * Configure and start the webserver. This also sets the MIME Types only * for files that should be downloadable when a browser is used to display @@ -430,6 +443,7 @@ public class LocalHTTPD extends NanoHTTPD { res.addHeader("Content-Length", "" + newLen); res.addHeader("Content-Range", "bytes " + startFrom + "-" + endAt + "/" + fileLen); res.addHeader("ETag", etag); + res.addHeader("Last-Modified", RFC_1123.format(new Date(file.lastModified()))); } } else { @@ -457,6 +471,7 @@ public class LocalHTTPD extends NanoHTTPD { res = newFixedFileResponse(file, mime); res.addHeader("Content-Length", "" + fileLen); res.addHeader("ETag", etag); + res.addHeader("Last-Modified", RFC_1123.format(new Date(file.lastModified()))); } } } catch (IOException ioe) { From afe6de94a019bf082a5c6b7a518af852ad47c26f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 20 Feb 2019 13:39:22 +0100 Subject: [PATCH 3/7] handle Apache and Nginx ETags when checking if index is current fdroid/fdroidclient#1708 --- .../org/fdroid/fdroid/net/HttpDownloader.java | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java index 9cde774ef..ef86f696a 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java @@ -95,12 +95,35 @@ public class HttpDownloader extends Downloader { } /** - * Get a remote file, checking the HTTP response code and the {@code etag}. - * In order to prevent the {@code etag} from being used as a form of tracking - * cookie, this code never sends the {@code etag} to the server. Instead, it - * uses a {@code HEAD} request to get the {@code etag} from the server, then - * only issues a {@code GET} if the {@code etag} has changed. + * Get a remote file, checking the HTTP response code, if it has changed since + * the last time a download was tried. + *

+ * If the {@code ETag} does not match, it could be caused by the previous + * download of the same file coming from a mirror running on a different + * webserver, e.g. Apache vs Nginx. {@code Content-Length} and + * {@code Last-Modified} are used to check whether the file has changed since + * those are more standardized than {@code ETag}. Plus, Nginx and Apache 2.4 + * defaults use only those two values to generate the {@code ETag} anyway. + * Unfortunately, other webservers and CDNs have totally different methods + * for generating the {@code ETag}. And mirrors that are syncing using a + * method other than {@code rsync} could easily have different {@code Last-Modified} + * times on the exact same file. On top of that, some services like GitHub's + * raw file support {@code raw.githubusercontent.com} and GitLab's raw file + * support do not set the {@code Last-Modified} header at all. So ultimately, + * then {@code ETag} needs to be used first and foremost, then this calculated + * {@code ETag} can serve as a common fallback. + *

+ * In order to prevent the {@code ETag} from being used as a form of tracking + * cookie, this code never sends the {@code ETag} to the server. Instead, it + * uses a {@code HEAD} request to get the {@code ETag} from the server, then + * only issues a {@code GET} if the {@code ETag} has changed. + *

+ * This uses a integer value for {@code Last-Modified} to avoid enabling the + * use of that value as some kind of "cookieless cookie". One second time + * resolution should be plenty since these files change more on the time + * space of minutes or hours. * + * @see update index from any available mirror * @see Cookieless cookies */ @Override @@ -108,7 +131,6 @@ public class HttpDownloader extends Downloader { // get the file size from the server HttpURLConnection tmpConn = getConnection(); tmpConn.setRequestMethod("HEAD"); - String etag = tmpConn.getHeaderField(HEADER_FIELD_ETAG); int contentLength = -1; int statusCode = tmpConn.getResponseCode(); @@ -116,10 +138,21 @@ public class HttpDownloader extends Downloader { newFileAvailableOnServer = false; switch (statusCode) { case HttpURLConnection.HTTP_OK: + String headETag = tmpConn.getHeaderField(HEADER_FIELD_ETAG); contentLength = tmpConn.getContentLength(); - if (!TextUtils.isEmpty(etag) && etag.equals(cacheTag)) { - Utils.debugLog(TAG, urlString + " is cached, not downloading"); - return; + if (!TextUtils.isEmpty(cacheTag)) { + if (cacheTag.equals(headETag)) { + Utils.debugLog(TAG, urlString + " cached, not downloading: " + headETag); + return; + } else { + String calcedETag = String.format("\"%x-%x\"", + tmpConn.getLastModified() / 1000, contentLength); + if (calcedETag.equals(headETag)) { + Utils.debugLog(TAG, urlString + " cached based on calced ETag, not downloading: " + + headETag); + return; + } + } } newFileAvailableOnServer = true; break; From cf9a6b851d7ca4307ab604ce7be78419fd5d8089 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 19 Feb 2019 12:49:47 +0100 Subject: [PATCH 4/7] RepoAdapter: code cleanup Remove unused code and simplify to only present args that are used. This is remnants from: fdroidclient#490 fdroidclient#606 fdroidclient!295 fdroidclient!242 --- .../fdroid/views/ManageReposActivity.java | 3 +-- .../org/fdroid/fdroid/views/RepoAdapter.java | 20 +++---------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java b/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java index d484c2f97..edcedc037 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/ManageReposActivity.java @@ -63,7 +63,6 @@ import org.fdroid.fdroid.IndexUpdater; import org.fdroid.fdroid.R; import org.fdroid.fdroid.UpdateService; import org.fdroid.fdroid.Utils; -import org.fdroid.fdroid.compat.CursorAdapterCompat; import org.fdroid.fdroid.data.NewRepoConfig; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.data.RepoProvider; @@ -114,7 +113,7 @@ public class ManageReposActivity extends AppCompatActivity getSupportActionBar().setDisplayHomeAsUpEnabled(true); final ListView repoList = (ListView) findViewById(R.id.list); - repoAdapter = RepoAdapter.create(this, null, CursorAdapterCompat.FLAG_AUTO_REQUERY); + repoAdapter = new RepoAdapter(this); repoAdapter.setEnabledListener(this); repoList.setAdapter(repoAdapter); repoList.setOnItemClickListener(new AdapterView.OnItemClickListener() { diff --git a/app/src/main/java/org/fdroid/fdroid/views/RepoAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/RepoAdapter.java index 9c1a48b7b..af631a623 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/RepoAdapter.java +++ b/app/src/main/java/org/fdroid/fdroid/views/RepoAdapter.java @@ -9,6 +9,7 @@ import android.view.ViewGroup; import android.widget.CompoundButton; import android.widget.TextView; import org.fdroid.fdroid.R; +import org.fdroid.fdroid.compat.CursorAdapterCompat; import org.fdroid.fdroid.data.Repo; public class RepoAdapter extends CursorAdapter { @@ -21,23 +22,8 @@ public class RepoAdapter extends CursorAdapter { private EnabledListener enabledListener; - public static RepoAdapter create(Context context, Cursor cursor, int flags) { - return new RepoAdapter(context, cursor, flags); - } - - private RepoAdapter(Context context, Cursor c, int flags) { - super(context, c, flags); - inflater = LayoutInflater.from(context); - } - - public RepoAdapter(Context context, Cursor c, boolean autoRequery) { - super(context, c, autoRequery); - inflater = LayoutInflater.from(context); - } - - @SuppressWarnings("deprecation") - private RepoAdapter(Context context, Cursor c) { - super(context, c); + RepoAdapter(Context context) { + super(context, null, CursorAdapterCompat.FLAG_AUTO_REQUERY); inflater = LayoutInflater.from(context); } From 89422e9c8fe21468b4d8e7f2c55feed88a5b26da Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 19 Feb 2019 12:18:34 +0100 Subject: [PATCH 5/7] clarify get mirrors method: Repo.getRandomMirror() --- .../main/java/org/fdroid/fdroid/FDroidApp.java | 4 ++-- .../main/java/org/fdroid/fdroid/data/Repo.java | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index 1f3ec670b..c828876e2 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -263,7 +263,7 @@ public class FDroidApp extends Application { * * @see #resetMirrorVars() * @see #getTimeout() - * @see Repo#getMirror(String) + * @see Repo#getRandomMirror(String) */ public static String getMirror(String urlString, Repo repo2) throws IOException { if (repo2.hasMirrors()) { @@ -286,7 +286,7 @@ public class FDroidApp extends Application { if (numTries == Integer.MAX_VALUE) { numTries = repo2.getMirrorCount(); } - String mirror = repo2.getMirror(lastWorkingMirror); + String mirror = repo2.getRandomMirror(lastWorkingMirror); String newUrl = urlString.replace(lastWorkingMirror, mirror); Utils.debugLog(TAG, "Trying mirror " + mirror + " after " + lastWorkingMirror + " failed," + " timeout=" + timeout / 1000 + "s"); 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 43fbe65e8..1333adc7c 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Repo.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Repo.java @@ -380,12 +380,16 @@ public class Repo extends ValueObject { } /** + * Get a random mirror URL from the list of mirrors for this repo. It will + * remove the URL in {@code mirrorToSkip} from consideration before choosing + * which mirror to return. + *

* The mirror logic assumes that it has a mirrors list with at least once * valid entry in it. In the index format as defined by {@code fdroid update}, * there is always at least one valid URL: the canonical URL. That also means * if there is only one item in the mirrors list, there are no other URLs to try. *

- * The initial state of the repos in the database also include the canonical + * The initial state of the repos in the database also includes the canonical * URL in the mirrors list so the mirror logic works on the first index * update. That makes it possible to do the first index update via SD Card * or USB OTG drive. @@ -394,16 +398,16 @@ public class Repo extends ValueObject { * @see FDroidApp#getMirror(String, Repo) * @see FDroidApp#getTimeout() */ - public String getMirror(String lastWorkingMirror) { - if (TextUtils.isEmpty(lastWorkingMirror)) { - lastWorkingMirror = address; + public String getRandomMirror(String mirrorToSkip) { + if (TextUtils.isEmpty(mirrorToSkip)) { + mirrorToSkip = address; } List shuffledMirrors = getMirrorList(); - Collections.shuffle(shuffledMirrors); if (shuffledMirrors.size() > 1) { + Collections.shuffle(shuffledMirrors); for (String m : shuffledMirrors) { // Return a non default, and not last used mirror - if (!m.equals(lastWorkingMirror)) { + if (!m.equals(mirrorToSkip)) { if (FDroidApp.isUsingTor()) { return m; } else { From dd14b9e315cbf7fe763c1aa946c985556c9f9b2e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 19 Feb 2019 14:06:54 +0100 Subject: [PATCH 6/7] choose random mirror for each package/APK download This spreads downloads across all available mirrors randomly. This could definitely be improved, like choosing the fastest or nearest mirror, or only .onion addresses on Tor. This will improve the current situation and should reduce the load on f-droid.org a lot. fdroidclient#1696 --- .../java/org/fdroid/fdroid/FDroidApp.java | 47 +++++++++++-------- .../org/fdroid/fdroid/IndexV1Updater.java | 4 +- .../java/org/fdroid/fdroid/data/DBHelper.java | 2 +- .../java/org/fdroid/fdroid/data/Repo.java | 2 +- .../installer/InstallManagerService.java | 32 +++++++++++-- 5 files changed, 58 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java index c828876e2..b7558cce6 100644 --- a/app/src/main/java/org/fdroid/fdroid/FDroidApp.java +++ b/app/src/main/java/org/fdroid/fdroid/FDroidApp.java @@ -39,6 +39,7 @@ import android.net.Uri; import android.os.Build; import android.os.Environment; import android.os.StrictMode; +import android.support.annotation.Nullable; import android.support.v4.util.LongSparseArray; import android.text.TextUtils; import android.util.Base64; @@ -66,7 +67,6 @@ import org.fdroid.fdroid.compat.PRNGFixes; import org.fdroid.fdroid.data.AppProvider; import org.fdroid.fdroid.data.InstalledAppProviderService; import org.fdroid.fdroid.data.Repo; -import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.installer.ApkFileProvider; import org.fdroid.fdroid.installer.InstallHistoryService; import org.fdroid.fdroid.localrepo.SDCardScannerService; @@ -245,13 +245,6 @@ public class FDroidApp extends Application { repo = new Repo(); } - /** - * @see #getMirror(String, Repo) - */ - public static String getMirror(String urlString, long repoId) throws IOException { - return getMirror(urlString, RepoProvider.Helper.findById(getInstance(), repoId)); - } - /** * Each time this is called, it will return a mirror from the pool of * mirrors. If it reaches the end of the list of mirrors, it will start @@ -260,17 +253,18 @@ public class FDroidApp extends Application { * again, it will do one last pass through the list with the timeout set to * {@link Downloader#LONGEST_TIMEOUT}. After that, this gives up with a * {@link IOException}. + *

+ * {@link #lastWorkingMirrorArray} is used to track the last mirror URL used, + * so it can be used in the string replacement operating when converting a + * download URL to point to a different mirror. Download URLs can be + * anything from {@code index-v1.jar} to APKs to icons to screenshots. * * @see #resetMirrorVars() * @see #getTimeout() * @see Repo#getRandomMirror(String) */ - public static String getMirror(String urlString, Repo repo2) throws IOException { + public static String getNewMirrorOnError(@Nullable String urlString, Repo repo2) throws IOException { if (repo2.hasMirrors()) { - String lastWorkingMirror = lastWorkingMirrorArray.get(repo2.getId()); - if (lastWorkingMirror == null) { - lastWorkingMirror = repo2.address; - } if (numTries <= 0) { if (timeout == Downloader.DEFAULT_TIMEOUT) { timeout = Downloader.SECOND_TIMEOUT; @@ -286,24 +280,37 @@ public class FDroidApp extends Application { if (numTries == Integer.MAX_VALUE) { numTries = repo2.getMirrorCount(); } - String mirror = repo2.getRandomMirror(lastWorkingMirror); - String newUrl = urlString.replace(lastWorkingMirror, mirror); - Utils.debugLog(TAG, "Trying mirror " + mirror + " after " + lastWorkingMirror + " failed," + - " timeout=" + timeout / 1000 + "s"); - lastWorkingMirrorArray.put(repo2.getId(), mirror); numTries--; - return newUrl; + return switchUrlToNewMirror(urlString, repo2); } else { throw new IOException("No mirrors available"); } } + /** + * Switch the URL in {@code urlString} to come from a random mirror. + */ + public static String switchUrlToNewMirror(@Nullable String urlString, Repo repo2) { + String lastWorkingMirror = lastWorkingMirrorArray.get(repo2.getId()); + if (lastWorkingMirror == null) { + lastWorkingMirror = repo2.address; + } + String mirror = repo2.getRandomMirror(lastWorkingMirror); + lastWorkingMirrorArray.put(repo2.getId(), mirror); + return urlString.replace(lastWorkingMirror, mirror); + } + public static int getTimeout() { return timeout; } + /** + * Reset the retry counter and timeout to defaults, and set the last + * working mirror to the canonical URL. + * + * @see #getNewMirrorOnError(String, Repo) + */ public static void resetMirrorVars() { - // Reset last working mirror, numtries, and timeout for (int i = 0; i < lastWorkingMirrorArray.size(); i++) { lastWorkingMirrorArray.removeAt(i); } diff --git a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java index 8d57e185e..c53b52f3e 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java +++ b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java @@ -141,7 +141,7 @@ public class IndexV1Updater extends IndexUpdater { | SSLHandshakeException | SSLKeyException | SSLPeerUnverifiedException | SSLProtocolException | ProtocolException | UnknownHostException e) { // if the above list changes, also change below and in DownloaderService.handleIntent() - Utils.debugLog(TAG, "Trying to download the index from a mirror"); + Utils.debugLog(TAG, "Trying to download the index from a mirror: " + e.getMessage()); // Mirror logic here, so that the default download code is untouched. String mirrorUrl; String prevMirrorUrl = indexUrl; @@ -149,7 +149,7 @@ public class IndexV1Updater extends IndexUpdater { int n = repo.getMirrorCount() * 3; // 3 is the number of timeouts we have. 10s, 30s & 60s for (int i = 0; i <= n; i++) { try { - mirrorUrl = FDroidApp.getMirror(prevMirrorUrl, repo); + mirrorUrl = FDroidApp.getNewMirrorOnError(prevMirrorUrl, repo); prevMirrorUrl = mirrorUrl; downloader = DownloaderFactory.create(context, mirrorUrl); downloader.setCacheTag(repo.lastetag); diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java index e56a0d139..07df7502d 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -1397,7 +1397,7 @@ public class DBHelper extends SQLiteOpenHelper { /** * Insert a new repo into the database. This also initializes the list of * "mirror" URLs. There should always be at least one URL there, since the - * logic in {@link org.fdroid.fdroid.FDroidApp#getMirror(String, Repo)} + * logic in {@link org.fdroid.fdroid.FDroidApp#switchUrlToNewMirror(String, Repo)} * expects at least one entry in the mirrors list. */ private void insertRepo(SQLiteDatabase db, String name, String address, 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 1333adc7c..2bebb1854 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/Repo.java +++ b/app/src/main/java/org/fdroid/fdroid/data/Repo.java @@ -395,7 +395,7 @@ public class Repo extends ValueObject { * or USB OTG drive. * * @see FDroidApp#resetMirrorVars() - * @see FDroidApp#getMirror(String, Repo) + * @see FDroidApp#switchUrlToNewMirror(String, Repo) * @see FDroidApp#getTimeout() */ public String getRandomMirror(String mirrorToSkip) { diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java index f42ce380f..57302885f 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -11,6 +11,7 @@ import android.content.pm.PackageInfo; import android.net.Uri; import android.os.IBinder; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.support.v4.content.LocalBroadcastManager; import android.text.TextUtils; import android.util.Log; @@ -23,6 +24,7 @@ import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.compat.PackageManagerCompat; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.App; +import org.fdroid.fdroid.data.RepoProvider; import org.fdroid.fdroid.net.Downloader; import org.fdroid.fdroid.net.DownloaderService; @@ -211,7 +213,7 @@ public class InstallManagerService extends Service { long apkFileSize = apkFilePath.length(); if (!apkFilePath.exists() || apkFileSize < apk.size) { Utils.debugLog(TAG, "download " + urlString + " " + apkFilePath); - DownloaderService.queue(this, urlString, apk.repoId, urlString); + DownloaderService.queue(this, switchUrlToNewMirror(urlString, apk.repoId), apk.repoId, urlString); } else if (ApkCache.apkIsCached(apkFilePath, apk)) { Utils.debugLog(TAG, "skip download, we have it, straight to install " + urlString + " " + apkFilePath); sendBroadcast(intent.getData(), Downloader.ACTION_STARTED, apkFilePath); @@ -219,7 +221,7 @@ public class InstallManagerService extends Service { } else { Utils.debugLog(TAG, "delete and download again " + urlString + " " + apkFilePath); apkFilePath.delete(); - DownloaderService.queue(this, urlString, apk.repoId, urlString); + DownloaderService.queue(this, switchUrlToNewMirror(urlString, apk.repoId), apk.repoId, urlString); } return START_REDELIVER_INTENT; // if killed before completion, retry Intent @@ -232,6 +234,24 @@ public class InstallManagerService extends Service { localBroadcastManager.sendBroadcast(intent); } + /** + * Tries to return a version of {@code urlString} from a mirror, if there + * is an error, it just returns {@code urlString}. + * + * @see FDroidApp#getNewMirrorOnError(String, org.fdroid.fdroid.data.Repo) + */ + public String getNewMirrorOnError(@Nullable String urlString, long repoId) { + try { + return FDroidApp.getNewMirrorOnError(urlString, RepoProvider.Helper.findById(this, repoId)); + } catch (IOException e) { + return urlString; + } + } + + public String switchUrlToNewMirror(@Nullable String urlString, long repoId) { + return FDroidApp.switchUrlToNewMirror(urlString, RepoProvider.Helper.findById(this, repoId)); + } + /** * Check if any OBB files are available, and if so, download and install them. This * also deletes any obsolete OBB files, per the spec, since there can be only one @@ -290,13 +310,13 @@ public class InstallManagerService extends Service { } else if (Downloader.ACTION_INTERRUPTED.equals(action)) { localBroadcastManager.unregisterReceiver(this); } else if (Downloader.ACTION_CONNECTION_FAILED.equals(action)) { - DownloaderService.queue(context, urlString, 0, urlString); + DownloaderService.queue(context, getNewMirrorOnError(urlString, 0), 0, urlString); } else { throw new RuntimeException("intent action not handled!"); } } }; - DownloaderService.queue(this, obbUrlString, 0, obbUrlString); + DownloaderService.queue(this, switchUrlToNewMirror(obbUrlString, 0), 0, obbUrlString); localBroadcastManager.registerReceiver(downloadReceiver, DownloaderService.getIntentFilter(obbUrlString)); } @@ -354,7 +374,9 @@ public class InstallManagerService extends Service { break; case Downloader.ACTION_CONNECTION_FAILED: try { - DownloaderService.queue(context, FDroidApp.getMirror(mirrorUrlString, repoId), repoId, urlString); + String currentUrlString = FDroidApp.getNewMirrorOnError(mirrorUrlString, + RepoProvider.Helper.findById(InstallManagerService.this, repoId)); + DownloaderService.queue(context, currentUrlString, repoId, urlString); DownloaderService.setTimeout(FDroidApp.getTimeout()); } catch (IOException e) { appUpdateStatusManager.setDownloadError(urlString, intent.getStringExtra(Downloader.EXTRA_ERROR_MESSAGE)); From 14b4a7e00ad181a2a0a4108ff5b744713c1ed98e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 19 Feb 2019 14:26:13 +0100 Subject: [PATCH 7/7] cache all downloads based on canonical URL, not download URL This makes the download cache be shared across all mirrors used to download rather than having a cache per-mirror. --- .../org/fdroid/fdroid/installer/ApkCache.java | 3 ++- .../fdroid/installer/InstallManagerService.java | 2 +- .../org/fdroid/fdroid/net/DownloaderService.java | 16 ++++++++-------- .../fdroid/views/apps/AppListItemController.java | 7 ++++--- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ApkCache.java b/app/src/main/java/org/fdroid/fdroid/installer/ApkCache.java index 08c18a5e2..c1c15d117 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ApkCache.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ApkCache.java @@ -115,7 +115,8 @@ public class ApkCache { /** * Get the full path for where an APK URL will be downloaded into. */ - public static SanitizedFile getApkDownloadPath(Context context, Uri uri) { + public static SanitizedFile getApkDownloadPath(Context context, String urlString) { + Uri uri = Uri.parse(urlString); File dir = new File(getApkCacheDir(context), uri.getHost() + "-" + uri.getPort()); if (!dir.exists()) { dir.mkdirs(); diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java index 57302885f..3ba2a632a 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallManagerService.java @@ -209,7 +209,7 @@ public class InstallManagerService extends Service { getObb(urlString, apk.getMainObbUrl(), apk.getMainObbFile(), apk.obbMainFileSha256); getObb(urlString, apk.getPatchObbUrl(), apk.getPatchObbFile(), apk.obbPatchFileSha256); - File apkFilePath = ApkCache.getApkDownloadPath(this, intent.getData()); + File apkFilePath = ApkCache.getApkDownloadPath(this, apk.getUrl()); long apkFileSize = apkFilePath.length(); if (!apkFilePath.exists() || apkFileSize < apk.size) { Utils.debugLog(TAG, "download " + urlString + " " + apkFilePath); diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index 0aaea76b7..30b8e4410 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -198,10 +198,10 @@ public class DownloaderService extends Service { */ private void handleIntent(Intent intent) { final Uri uri = intent.getData(); - final SanitizedFile localFile = ApkCache.getApkDownloadPath(this, uri); long repoId = intent.getLongExtra(Downloader.EXTRA_REPO_ID, 0); - String originalUrlString = intent.getStringExtra(Downloader.EXTRA_CANONICAL_URL); - sendBroadcast(uri, Downloader.ACTION_STARTED, localFile, repoId, originalUrlString); + String canonicalUrlString = intent.getStringExtra(Downloader.EXTRA_CANONICAL_URL); + final SanitizedFile localFile = ApkCache.getApkDownloadPath(this, canonicalUrlString); + sendBroadcast(uri, Downloader.ACTION_STARTED, localFile, repoId, canonicalUrlString); try { downloader = DownloaderFactory.create(this, uri, localFile); @@ -219,22 +219,22 @@ public class DownloaderService extends Service { downloader.download(); if (downloader.isNotFound()) { sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile, getString(R.string.download_404), - repoId, originalUrlString); + repoId, canonicalUrlString); } else { - sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile, repoId, originalUrlString); + sendBroadcast(uri, Downloader.ACTION_COMPLETE, localFile, repoId, canonicalUrlString); } } catch (InterruptedException e) { - sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile, repoId, originalUrlString); + sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile, repoId, canonicalUrlString); } catch (ConnectException | HttpRetryException | NoRouteToHostException | SocketTimeoutException | SSLHandshakeException | SSLKeyException | SSLPeerUnverifiedException | SSLProtocolException | ProtocolException | UnknownHostException e) { // if the above list of exceptions changes, also change it in IndexV1Updater.update() Log.e(TAG, e.getLocalizedMessage()); - sendBroadcast(uri, Downloader.ACTION_CONNECTION_FAILED, localFile, repoId, originalUrlString); + sendBroadcast(uri, Downloader.ACTION_CONNECTION_FAILED, localFile, repoId, canonicalUrlString); } catch (IOException e) { e.printStackTrace(); sendBroadcast(uri, Downloader.ACTION_INTERRUPTED, localFile, - e.getLocalizedMessage(), repoId, originalUrlString); + e.getLocalizedMessage(), repoId, canonicalUrlString); } finally { if (downloader != null) { downloader.close(); diff --git a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java index e8b124a77..9427c5f45 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java +++ b/app/src/main/java/org/fdroid/fdroid/views/apps/AppListItemController.java @@ -27,7 +27,6 @@ import android.widget.ImageView; import android.widget.ProgressBar; import android.widget.TextView; import com.nostra13.universalimageloader.core.ImageLoader; -import org.fdroid.fdroid.views.AppDetailsActivity; import org.fdroid.fdroid.AppUpdateStatusManager; import org.fdroid.fdroid.AppUpdateStatusManager.AppUpdateStatus; import org.fdroid.fdroid.R; @@ -39,6 +38,7 @@ import org.fdroid.fdroid.installer.ApkCache; import org.fdroid.fdroid.installer.InstallManagerService; import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.installer.InstallerFactory; +import org.fdroid.fdroid.views.AppDetailsActivity; import org.fdroid.fdroid.views.updates.DismissResult; import java.io.File; @@ -483,8 +483,8 @@ public abstract class AppListItemController extends RecyclerView.ViewHolder { } if (currentStatus != null && currentStatus.status == AppUpdateStatusManager.Status.ReadyToInstall) { - Uri apkDownloadUri = Uri.parse(currentStatus.apk.getUrl()); - File apkFilePath = ApkCache.getApkDownloadPath(activity, apkDownloadUri); + String urlString = currentStatus.apk.getUrl(); + File apkFilePath = ApkCache.getApkDownloadPath(activity, urlString); Utils.debugLog(TAG, "skip download, we have already downloaded " + currentStatus.apk.getUrl() + " to " + apkFilePath); @@ -505,6 +505,7 @@ public abstract class AppListItemController extends RecyclerView.ViewHolder { } }; + Uri apkDownloadUri = Uri.parse(urlString); broadcastManager.registerReceiver(receiver, Installer.getInstallIntentFilter(apkDownloadUri)); Installer installer = InstallerFactory.create(activity, currentStatus.apk); installer.installPackage(Uri.parse(apkFilePath.toURI().toString()), apkDownloadUri);