From 27fe848156a6ccd64fc93b6c8b8393c66e5cdcf9 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 17 Mar 2022 16:52:30 -0300 Subject: [PATCH] Improve mirror fallback, e.g. on 404 and update ktor --- download/build.gradle | 4 +- .../kotlin/org/fdroid/download/HttpManager.kt | 33 +-- .../org/fdroid/download/MirrorChooser.kt | 18 +- .../org/fdroid/download/HttpManagerTest.kt | 4 +- .../org/fdroid/download/MirrorChooserTest.kt | 15 ++ gradle/verification-metadata.xml | 195 ++++++++++++++++++ 6 files changed, 244 insertions(+), 25 deletions(-) diff --git a/download/build.gradle b/download/build.gradle index 891109132..ab312bbfc 100644 --- a/download/build.gradle +++ b/download/build.gradle @@ -1,7 +1,7 @@ plugins { id 'org.jetbrains.kotlin.multiplatform' id 'com.android.library' - id "org.jlleitschuh.gradle.ktlint" version "10.1.0" + id "org.jlleitschuh.gradle.ktlint" version "10.2.1" } group = 'org.fdroid' @@ -22,7 +22,7 @@ kotlin { else throw new GradleException("Host OS is not supported in Kotlin/Native.") ext { - ktor_version = "1.6.7" //"2.0.0-beta-1" + ktor_version = "1.6.8" //"2.0.0-beta-1" } sourceSets { diff --git a/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt b/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt index 06944832d..447217085 100644 --- a/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt +++ b/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt @@ -22,6 +22,7 @@ import io.ktor.http.HttpHeaders.ETag import io.ktor.http.HttpHeaders.LastModified import io.ktor.http.HttpHeaders.Range import io.ktor.http.HttpStatusCode.Companion.PartialContent +import io.ktor.http.Url import io.ktor.http.contentLength import io.ktor.util.InternalAPI import io.ktor.util.encodeBase64 @@ -127,8 +128,8 @@ public open class HttpManager @JvmOverloads constructor( request: DownloadRequest, skipFirstBytes: Long? = null, receiver: suspend (ByteArray) -> Unit, - ) { - get(request, skipFirstBytes).execute { response -> + ): Unit = mirrorChooser.mirrorRequest(request) { mirror, url -> + getHttpStatement(request, mirror, url, skipFirstBytes).execute { response -> if (skipFirstBytes != null && response.status != PartialContent) { throw NoResumeException() } @@ -143,22 +144,22 @@ public open class HttpManager @JvmOverloads constructor( } } - internal suspend fun get( + private suspend fun getHttpStatement( request: DownloadRequest, + mirror: Mirror, + url: Url, skipFirstBytes: Long? = null, ): HttpStatement { val authString = constructBasicAuthValue(request) - return mirrorChooser.mirrorRequest(request) { mirror, url -> - resetProxyIfNeeded(request.proxy, mirror) - log.debug { "GET $url" } - httpClient.get(url) { - // add authorization header from username / password if set - if (authString != null) header(Authorization, authString) - // increase connect timeout if using Tor mirror - if (mirror.isOnion()) timeout { connectTimeoutMillis = 20_000 } - // add range header if set - if (skipFirstBytes != null) header(Range, "bytes=$skipFirstBytes-") - } + resetProxyIfNeeded(request.proxy, mirror) + log.debug { "GET $url" } + return httpClient.get(url) { + // add authorization header from username / password if set + if (authString != null) header(Authorization, authString) + // increase connect timeout if using Tor mirror + if (mirror.isOnion()) timeout { connectTimeoutMillis = 20_000 } + // add range header if set + if (skipFirstBytes != null) header(Range, "bytes=$skipFirstBytes-") } } @@ -169,7 +170,9 @@ public open class HttpManager @JvmOverloads constructor( request: DownloadRequest, skipFirstBytes: Long? = null, ): ByteReadChannel { - return get(request, skipFirstBytes).receive() + return mirrorChooser.mirrorRequest(request) { mirror, url -> + getHttpStatement(request, mirror, url, skipFirstBytes).receive() + } } /** diff --git a/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt b/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt index d5e6340d5..e8204cd1c 100644 --- a/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt +++ b/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt @@ -3,6 +3,7 @@ package org.fdroid.download import io.ktor.client.features.ResponseException import io.ktor.http.HttpStatusCode.Companion.Forbidden import io.ktor.http.Url +import io.ktor.utils.io.errors.IOException import mu.KotlinLogging public interface MirrorChooser { @@ -43,16 +44,21 @@ internal abstract class MirrorChooserImpl : MirrorChooser { // don't try other mirrors if we got Forbidden response, but supplied credentials if (downloadRequest.hasCredentials && e.response.status == Forbidden) throw e // also throw if this is the last mirror to try, otherwise try next - val wasLastMirror = index == downloadRequest.mirrors.size - 1 - log.warn(e) { - if (wasLastMirror) "Last mirror, rethrowing..." - else "Trying other mirror now..." - } - if (wasLastMirror) throw e + throwOnLastMirror(e, index == downloadRequest.mirrors.size - 1) + } catch (e: IOException) { + throwOnLastMirror(e, index == downloadRequest.mirrors.size - 1) } } error("Reached code that was thought to be unreachable.") } + + private fun throwOnLastMirror(e: Exception, wasLastMirror: Boolean) { + log.warn(e) { + if (wasLastMirror) "Last mirror, rethrowing..." + else "Trying other mirror now..." + } + if (wasLastMirror) throw e + } } internal class MirrorChooserRandom : MirrorChooserImpl() { diff --git a/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt b/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt index feb040055..b47c16b3a 100644 --- a/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt +++ b/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt @@ -211,8 +211,8 @@ class HttpManagerTest { httpManager.getBytes(downloadRequest) } - // HEAD tries another mirror, but GET throws, so no retry - assertEquals(3, mockEngine.requestHistory.size) + // HEAD and GET try another mirror, so 4 requests + assertEquals(4, mockEngine.requestHistory.size) mockEngine.responseHistory.forEach { response -> assertEquals(TemporaryRedirect, response.statusCode) } diff --git a/download/src/commonTest/kotlin/org/fdroid/download/MirrorChooserTest.kt b/download/src/commonTest/kotlin/org/fdroid/download/MirrorChooserTest.kt index 862edf35c..5ffad0045 100644 --- a/download/src/commonTest/kotlin/org/fdroid/download/MirrorChooserTest.kt +++ b/download/src/commonTest/kotlin/org/fdroid/download/MirrorChooserTest.kt @@ -1,5 +1,6 @@ package org.fdroid.download +import io.ktor.utils.io.errors.IOException import org.fdroid.runSuspend import kotlin.random.Random import kotlin.test.Test @@ -24,6 +25,20 @@ class MirrorChooserTest { assertEquals(expectedResult, result) } + @Test + fun testFallbackToNextMirror() = runSuspend { + val mirrorChooser = MirrorChooserRandom() + val expectedResult = Random.nextInt() + + val result = mirrorChooser.mirrorRequest(downloadRequest) { mirror, url -> + assertEquals(mirror.getUrl(downloadRequest.path), url) + // fails with all except last mirror + if (mirror != downloadRequest.mirrors.last()) throw IOException("foo") + expectedResult + } + assertEquals(expectedResult, result) + } + @Test fun testMirrorChooserRandom() { val mirrorChooser = MirrorChooserRandom() diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 3b60e0518..2741c6e79 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -878,11 +878,21 @@ + + + + + + + + + + @@ -1655,6 +1665,7 @@ + @@ -1854,6 +1865,7 @@ + @@ -2067,6 +2079,11 @@ + + + + + @@ -2296,6 +2313,11 @@ + + + + + @@ -2319,6 +2341,14 @@ + + + + + + + + @@ -2332,6 +2362,11 @@ + + + + + @@ -2342,6 +2377,11 @@ + + + + + @@ -2352,6 +2392,11 @@ + + + + + @@ -2365,6 +2410,14 @@ + + + + + + + + @@ -2378,21 +2431,41 @@ + + + + + + + + + + + + + + + + + + + + @@ -2430,6 +2503,14 @@ + + + + + + + + @@ -2446,6 +2527,14 @@ + + + + + + + + @@ -2456,6 +2545,11 @@ + + + + + @@ -2466,6 +2560,11 @@ + + + + + @@ -2476,6 +2575,11 @@ + + + + + @@ -2486,6 +2590,11 @@ + + + + + @@ -2499,6 +2608,14 @@ + + + + + + + + @@ -2512,6 +2629,11 @@ + + + + + @@ -2528,6 +2650,17 @@ + + + + + + + + + + + @@ -2538,6 +2671,11 @@ + + + + + @@ -2548,6 +2686,11 @@ + + + + + @@ -2558,6 +2701,11 @@ + + + + + @@ -2599,6 +2747,14 @@ + + + + + + + + @@ -2612,6 +2768,11 @@ + + + + + @@ -2625,6 +2786,14 @@ + + + + + + + + @@ -3303,6 +3472,7 @@ + @@ -3809,6 +3979,11 @@ + + + + + @@ -3825,6 +4000,14 @@ + + + + + + + + @@ -3905,6 +4088,11 @@ + + + + + @@ -3922,6 +4110,7 @@ + @@ -4155,6 +4344,12 @@ + + + + + +