From 37078762ad3c4ebd3556fb4e2bbab9d222af8e17 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 15 Dec 2022 11:21:43 -0300 Subject: [PATCH] Properly handle not finding an index This enables swap to work with v1 and v0 --- .../fdroid/fdroid/net/HttpDownloaderTest.java | 17 ++++++----- .../java/org/fdroid/fdroid/IndexUpdater.java | 29 +++++++++++-------- .../org/fdroid/fdroid/IndexV1Updater.java | 4 +++ .../fdroid/fdroid/net/DownloaderService.java | 3 +- .../kotlin/org/fdroid/download/Downloader.kt | 2 +- .../org/fdroid/download/HttpDownloader.kt | 2 +- .../kotlin/org/fdroid/download/HttpManager.kt | 10 +++++++ 7 files changed, 44 insertions(+), 23 deletions(-) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java b/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java index 25d47892b..59f4ea6da 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/net/HttpDownloaderTest.java @@ -10,6 +10,7 @@ import org.fdroid.download.DownloadRequest; import org.fdroid.download.HttpDownloader; import org.fdroid.download.HttpManager; import org.fdroid.download.Mirror; +import org.fdroid.download.NotFoundException; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.ProgressListener; import org.fdroid.fdroid.Utils; @@ -60,7 +61,7 @@ public class HttpDownloaderTest { private boolean receivedProgress; @Test - public void downloadUninterruptedTest() throws IOException, InterruptedException { + public void downloadUninterruptedTest() throws Exception { for (Pair pair : URLS) { Log.i(TAG, "URL: " + pair.first + pair.second); File destFile = File.createTempFile("dl-", ""); @@ -75,7 +76,7 @@ public class HttpDownloaderTest { } @Test - public void downloadUninterruptedTestWithProgress() throws IOException, InterruptedException { + public void downloadUninterruptedTestWithProgress() throws Exception { final CountDownLatch latch = new CountDownLatch(1); String path = "index.jar"; List mirrors = Mirror.fromStrings(Collections.singletonList("https://ftp.fau.de/fdroid/repo/")); @@ -95,7 +96,7 @@ public class HttpDownloaderTest { try { httpDownloader.download(); latch.countDown(); - } catch (IOException | InterruptedException e) { + } catch (IOException | NotFoundException | InterruptedException e) { e.printStackTrace(); fail(); } @@ -109,7 +110,7 @@ public class HttpDownloaderTest { } @Test - public void downloadHttpBasicAuth() throws IOException, InterruptedException { + public void downloadHttpBasicAuth() throws Exception { String path = "myusername/supersecretpassword"; List mirrors = Mirror.fromStrings(Collections.singletonList("https://httpbin.org/basic-auth/")); File destFile = File.createTempFile("dl-", ""); @@ -122,7 +123,7 @@ public class HttpDownloaderTest { } @Test(expected = IOException.class) - public void downloadHttpBasicAuthWrongPassword() throws IOException, InterruptedException { + public void downloadHttpBasicAuthWrongPassword() throws Exception { String path = "myusername/supersecretpassword"; List mirrors = Mirror.fromStrings(Collections.singletonList("https://httpbin.org/basic-auth/")); File destFile = File.createTempFile("dl-", ""); @@ -135,7 +136,7 @@ public class HttpDownloaderTest { } @Test(expected = IOException.class) - public void downloadHttpBasicAuthWrongUsername() throws IOException, InterruptedException { + public void downloadHttpBasicAuthWrongUsername() throws Exception { String path = "myusername/supersecretpassword"; List mirrors = Mirror.fromStrings(Collections.singletonList("https://httpbin.org/basic-auth/")); File destFile = File.createTempFile("dl-", ""); @@ -148,7 +149,7 @@ public class HttpDownloaderTest { } @Test - public void downloadThenCancel() throws IOException, InterruptedException { + public void downloadThenCancel() throws Exception { final CountDownLatch latch = new CountDownLatch(2); String path = "index.jar"; List mirrors = Mirror.fromStrings(Collections.singletonList("https://f-droid.org/repo/")); @@ -168,7 +169,7 @@ public class HttpDownloaderTest { try { httpDownloader.download(); fail(); - } catch (IOException e) { + } catch (IOException | NotFoundException e) { e.printStackTrace(); fail(); } catch (InterruptedException e) { diff --git a/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java b/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java index 944afc8ec..9233d20e9 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java +++ b/app/src/main/java/org/fdroid/fdroid/IndexUpdater.java @@ -126,7 +126,8 @@ public class IndexUpdater { return hasChanged; } - private Pair downloadIndex() throws UpdateException { + private Pair downloadIndex() throws UpdateException, + org.fdroid.download.NotFoundException { File destFile = null; Downloader downloader = null; try { @@ -161,19 +162,23 @@ public class IndexUpdater { * @throws UpdateException All error states will come from here. */ public boolean update() throws UpdateException { - final Pair pair = downloadIndex(); - final Downloader downloader = pair.first; - final File destFile = pair.second; - hasChanged = downloader.hasChanged(); + try { + final Pair pair = downloadIndex(); + final Downloader downloader = pair.first; + final File destFile = pair.second; + hasChanged = downloader.hasChanged(); - if (hasChanged) { - // Don't worry about checking the status code for 200. If it was a - // successful download, then we will have a file ready to use: - cacheTag = downloader.getCacheTag(); - processDownloadedFile(destFile); - processRepoPushRequests(repoPushRequestList); + if (hasChanged) { + // Don't worry about checking the status code for 200. If it was a + // successful download, then we will have a file ready to use: + cacheTag = downloader.getCacheTag(); + processDownloadedFile(destFile); + processRepoPushRequests(repoPushRequestList); + } + return true; + } catch (org.fdroid.download.NotFoundException e) { + return false; } - return true; } private ContentValues repoDetailsToSave; diff --git a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java index 359a7ebad..eeb29f6dc 100644 --- a/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java +++ b/app/src/main/java/org/fdroid/fdroid/IndexV1Updater.java @@ -42,6 +42,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.io.FileUtils; import org.fdroid.download.Downloader; +import org.fdroid.download.NotFoundException; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.App; import org.fdroid.fdroid.data.Repo; @@ -126,6 +127,9 @@ public class IndexV1Updater extends IndexUpdater { FileUtils.deleteQuietly(destFile); } throw new IndexUpdater.UpdateException(repo, "Error getting F-Droid index file", e); + } catch (NotFoundException e) { + // did not find an index + return false; } catch (InterruptedException e) { // ignored if canceled, the local database just won't be updated } // TODO is it safe to delete destFile in finally block? 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 5eb2bdf0b..0386016a0 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -34,6 +34,7 @@ import android.util.Log; import android.util.LogPrinter; import org.fdroid.download.Downloader; +import org.fdroid.download.NotFoundException; import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.ProgressListener; import org.fdroid.fdroid.Utils; @@ -266,7 +267,7 @@ public class DownloaderService extends Service { sendBroadcast(uri, DownloaderService.ACTION_INTERRUPTED, localFile, repoId, canonicalUrl); } catch (ConnectException | HttpRetryException | NoRouteToHostException | SocketTimeoutException | SSLHandshakeException | SSLKeyException | SSLPeerUnverifiedException | SSLProtocolException - | ProtocolException | UnknownHostException e) { + | ProtocolException | UnknownHostException | NotFoundException e) { // if the above list of exceptions changes, also change it in IndexV1Updater.update() Log.e(TAG, "CONNECTION_FAILED: " + e.getLocalizedMessage()); sendBroadcast(uri, DownloaderService.ACTION_CONNECTION_FAILED, localFile, repoId, canonicalUrl); diff --git a/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt b/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt index 555335307..ce08e1fd4 100644 --- a/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt +++ b/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt @@ -37,7 +37,7 @@ public abstract class Downloader constructor( * Call this to start the download. * Never call this more than once. Create a new [Downloader], if you need to download again! */ - @Throws(IOException::class, InterruptedException::class) + @Throws(IOException::class, NotFoundException::class, InterruptedException::class) public abstract fun download() @Throws(IOException::class) diff --git a/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt b/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt index 68621f2cc..0fb4022b5 100644 --- a/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt +++ b/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt @@ -99,7 +99,7 @@ public class HttpDownloader constructor( * @see [Cookieless cookies](http://lucb1e.com/rp/cookielesscookies) */ @OptIn(DelicateCoroutinesApi::class) - @Throws(IOException::class, InterruptedException::class) + @Throws(IOException::class, NotFoundException::class, InterruptedException::class) override fun download() { val headInfo = runBlocking { httpManager.head(request, cacheTag) ?: throw IOException() diff --git a/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt b/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt index 5d488c3e3..746c09f27 100644 --- a/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt +++ b/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt @@ -25,6 +25,7 @@ import io.ktor.http.HttpHeaders.ETag import io.ktor.http.HttpHeaders.LastModified import io.ktor.http.HttpHeaders.Range import io.ktor.http.HttpMessageBuilder +import io.ktor.http.HttpStatusCode.Companion.NotFound import io.ktor.http.HttpStatusCode.Companion.PartialContent import io.ktor.http.Url import io.ktor.http.contentLength @@ -89,6 +90,7 @@ public open class HttpManager @JvmOverloads constructor( * This is useful for checking if the repository index has changed before downloading it again. * However, due to non-standard ETags on mirrors, change detection is unreliable. */ + @Throws(NotFoundException::class) public suspend fun head(request: DownloadRequest, eTag: String? = null): HeadInfo? { val response: HttpResponse = try { mirrorChooser.mirrorRequest(request) { mirror, url -> @@ -104,6 +106,7 @@ public open class HttpManager @JvmOverloads constructor( } } catch (e: ResponseException) { log.warn(e) { "Error getting HEAD" } + if (e.response.status == NotFound) throw NotFoundException() return null } val contentLength = response.contentLength() @@ -225,3 +228,10 @@ public open class HttpManager @JvmOverloads constructor( } public class NoResumeException : Exception() + +/** + * Thrown when a file was not found. + * Catching this is useful when checking if a new index version exists + * and then falling back to an older version. + */ +public class NotFoundException(e: Throwable? = null) : Exception(e)