From a830f1ef869c2dfeb90154aed9b7c58df009ba6e Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 18 May 2022 11:46:20 -0300 Subject: [PATCH] [download] Add new download method for v2 index that receives total file size and ensures that the downloaded file has the provided sha256 hash --- .../fdroid/net/BluetoothDownloader.java | 8 +++ .../fdroid/net/LocalFileDownloader.java | 8 +++ .../fdroid/fdroid/net/TreeUriDownloader.java | 10 +++- .../kotlin/org/fdroid/download/Downloader.kt | 53 +++++++++++++++++-- .../org/fdroid/download/HttpDownloader.kt | 38 +++++++------ .../kotlin/org/fdroid/fdroid/HashUtils.kt | 13 +++++ .../org/fdroid/download/HttpDownloaderTest.kt | 36 ++++++++++++- .../kotlin/org/fdroid/download/HttpManager.kt | 8 +-- .../kotlin/org/fdroid/download/Mirror.kt | 4 +- .../org/fdroid/download/MirrorChooser.kt | 3 ++ .../org/fdroid/fdroid/ProgressListener.kt | 2 +- .../org/fdroid/download/HttpManagerTest.kt | 27 +++++++++- 12 files changed, 181 insertions(+), 29 deletions(-) create mode 100644 download/src/androidMain/kotlin/org/fdroid/fdroid/HashUtils.kt diff --git a/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java index 968a3f976..511dadc75 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java @@ -91,10 +91,18 @@ public class BluetoothDownloader extends Downloader { @Override public long totalDownloadSize() { + if (getFileSize() != null) return getFileSize(); FileDetails details = getFileDetails(); return details != null ? details.getFileSize() : -1; } + @Override + public void download(long totalSize, @Nullable String sha256) throws IOException, InterruptedException { + setFileSize(totalSize); + setSha256(sha256); + download(); + } + @Override public void download() throws IOException, InterruptedException { downloadFromStream(false); diff --git a/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java index 44c3bc7da..351dcf275 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/LocalFileDownloader.java @@ -3,6 +3,7 @@ package org.fdroid.fdroid.net; import android.net.Uri; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; @@ -69,6 +70,13 @@ public class LocalFileDownloader extends Downloader { return sourceFile.length(); } + @Override + public void download(long totalSize, @Nullable String sha256) throws IOException, InterruptedException { + setFileSize(totalSize); + setSha256(sha256); + download(); + } + @Override public void download() throws IOException, InterruptedException { if (!sourceFile.exists()) { diff --git a/app/src/main/java/org/fdroid/fdroid/net/TreeUriDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/TreeUriDownloader.java index f1f0bc70f..a8b417a47 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/TreeUriDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/TreeUriDownloader.java @@ -15,6 +15,7 @@ import java.io.InputStream; import java.net.ProtocolException; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.documentfile.provider.DocumentFile; /** @@ -94,7 +95,14 @@ public class TreeUriDownloader extends Downloader { @Override protected long totalDownloadSize() { - return documentFile.length(); // TODO how should this actually be implemented? + return getFileSize() != null ? getFileSize() : documentFile.length(); + } + + @Override + public void download(long totalSize, @Nullable String sha256) throws IOException, InterruptedException { + setFileSize(totalSize); + setSha256(sha256); + downloadFromStream(false); } @Override diff --git a/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt b/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt index 555335307..332ff86f1 100644 --- a/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt +++ b/download/src/androidMain/kotlin/org/fdroid/download/Downloader.kt @@ -2,11 +2,13 @@ package org.fdroid.download import mu.KotlinLogging import org.fdroid.fdroid.ProgressListener +import org.fdroid.fdroid.isMatching import java.io.File import java.io.FileOutputStream import java.io.IOException import java.io.InputStream import java.io.OutputStream +import java.security.MessageDigest public abstract class Downloader constructor( @JvmField @@ -17,6 +19,13 @@ public abstract class Downloader constructor( private val log = KotlinLogging.logger {} } + protected var fileSize: Long? = null + + /** + * If not null, this is the expected sha256 hash of the [outputFile] after download. + */ + protected var sha256: String? = null + /** * If you ask for the cacheTag before calling download(), you will get the * same one you passed in (if any). If you call it after download(), you @@ -25,6 +34,7 @@ public abstract class Downloader constructor( * If this cacheTag matches that returned by the server, then no download will * take place, and a status code of 304 will be returned by download(). */ + @Deprecated("Used only for v1 repos") public var cacheTag: String? = null @Volatile @@ -36,13 +46,24 @@ 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! + * + * @totalSize must be set to what the index tells us the size will be + * @sha256 must be set to the sha256 hash from the index and only be null for `entry.jar`. */ @Throws(IOException::class, InterruptedException::class) + public abstract fun download(totalSize: Long, sha256: String? = null) + + /** + * Call this to start the download. + * Never call this more than once. Create a new [Downloader], if you need to download again! + */ + @Deprecated("Use only for v1 repos") + @Throws(IOException::class, InterruptedException::class) public abstract fun download() @Throws(IOException::class) protected abstract fun getInputStream(resumable: Boolean): InputStream - protected open suspend fun getBytes(resumable: Boolean, receiver: (ByteArray) -> Unit) { + protected open suspend fun getBytes(resumable: Boolean, receiver: BytesReceiver) { throw NotImplementedError() } @@ -57,6 +78,7 @@ public abstract class Downloader constructor( * After calling [download], this returns true if a new file was downloaded and * false if the file on the server has not changed and thus was not downloaded. */ + @Deprecated("Only for v1 repos") public abstract fun hasChanged(): Boolean public abstract fun close() @@ -88,17 +110,28 @@ public abstract class Downloader constructor( @Throws(InterruptedException::class, IOException::class, NoResumeException::class) protected suspend fun downloadFromBytesReceiver(isResume: Boolean) { try { + val messageDigest: MessageDigest? = if (sha256 == null) null else { + MessageDigest.getInstance("SHA-256") + } FileOutputStream(outputFile, isResume).use { outputStream -> var bytesCopied = outputFile.length() var lastTimeReported = 0L val bytesTotal = totalDownloadSize() - getBytes(isResume) { bytes -> + getBytes(isResume) { bytes, numTotalBytes -> // Getting the input stream is slow(ish) for HTTP downloads, so we'll check if // we were interrupted before proceeding to the download. throwExceptionIfInterrupted() outputStream.write(bytes) + messageDigest?.update(bytes) bytesCopied += bytes.size - lastTimeReported = reportProgress(lastTimeReported, bytesCopied, bytesTotal) + val total = if (bytesTotal == -1L) numTotalBytes ?: -1L else bytesTotal + lastTimeReported = reportProgress(lastTimeReported, bytesCopied, total) + } + // check if expected sha256 hash matches + sha256?.let { expectedHash -> + if (!messageDigest.isMatching(expectedHash)) { + throw IOException("Hash not matching") + } } // force progress reporting at the end reportProgress(0L, bytesCopied, bytesTotal) @@ -119,6 +152,9 @@ public abstract class Downloader constructor( */ @Throws(IOException::class, InterruptedException::class) private fun copyInputToOutputStream(input: InputStream, output: OutputStream) { + val messageDigest: MessageDigest? = if (sha256 == null) null else { + MessageDigest.getInstance("SHA-256") + } try { var bytesCopied = outputFile.length() var lastTimeReported = 0L @@ -128,10 +164,17 @@ public abstract class Downloader constructor( while (numBytes >= 0) { throwExceptionIfInterrupted() output.write(buffer, 0, numBytes) + messageDigest?.update(buffer, 0, numBytes) bytesCopied += numBytes lastTimeReported = reportProgress(lastTimeReported, bytesCopied, bytesTotal) numBytes = input.read(buffer) } + // check if expected sha256 hash matches + sha256?.let { expectedHash -> + if (!messageDigest.isMatching(expectedHash)) { + throw IOException("Hash not matching") + } + } // force progress reporting at the end reportProgress(0L, bytesCopied, bytesTotal) } finally { @@ -176,3 +219,7 @@ public abstract class Downloader constructor( } } + +public fun interface BytesReceiver { + public suspend fun receive(bytes: ByteArray, numTotalBytes: Long?) +} diff --git a/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt b/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt index 68621f2cc..bfcaef7e0 100644 --- a/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt +++ b/download/src/androidMain/kotlin/org/fdroid/download/HttpDownloader.kt @@ -21,10 +21,8 @@ */ package org.fdroid.download -import android.annotation.TargetApi -import android.os.Build.VERSION.SDK_INT import io.ktor.client.plugins.ResponseException -import kotlinx.coroutines.DelicateCoroutinesApi +import io.ktor.http.HttpStatusCode.Companion.NotFound import kotlinx.coroutines.runBlocking import mu.KotlinLogging import java.io.File @@ -45,23 +43,30 @@ public class HttpDownloader constructor( val log = KotlinLogging.logger {} } + @Deprecated("Only for v1 repos") private var hasChanged = false - private var fileSize = -1L override fun getInputStream(resumable: Boolean): InputStream { throw NotImplementedError("Use getInputStreamSuspend instead.") } @Throws(IOException::class, NoResumeException::class) - override suspend fun getBytes(resumable: Boolean, receiver: (ByteArray) -> Unit) { + protected override suspend fun getBytes(resumable: Boolean, receiver: BytesReceiver) { val skipBytes = if (resumable) outputFile.length() else null return try { httpManager.get(request, skipBytes, receiver) } catch (e: ResponseException) { - throw IOException(e) + if (e.response.status == NotFound) throw NotFoundException(e) + else throw IOException(e) } } + public override fun download(totalSize: Long, sha256: String?) { + this.fileSize = totalSize + this.sha256 = sha256 + downloadToFile() + } + /** * Get a remote file, checking the HTTP response code, if it has changed since * the last time a download was tried. @@ -98,9 +103,9 @@ public class HttpDownloader constructor( * * @see [Cookieless cookies](http://lucb1e.com/rp/cookielesscookies) */ - @OptIn(DelicateCoroutinesApi::class) + @Suppress("DEPRECATION") @Throws(IOException::class, InterruptedException::class) - override fun download() { + public override fun download() { val headInfo = runBlocking { httpManager.head(request, cacheTag) ?: throw IOException() } @@ -137,9 +142,13 @@ public class HttpDownloader constructor( } hasChanged = true + downloadToFile() + } + + private fun downloadToFile() { var resumable = false val fileLength = outputFile.length() - if (fileLength > fileSize) { + if (fileLength > fileSize ?: -1) { if (!outputFile.delete()) log.warn { "Warning: " + outputFile.absolutePath + " not deleted" } @@ -163,15 +172,10 @@ public class HttpDownloader constructor( } } - @TargetApi(24) - public override fun totalDownloadSize(): Long { - return if (SDK_INT < 24) { - fileSize.toInt().toLong() // TODO why? - } else { - fileSize - } - } + protected override fun totalDownloadSize(): Long = fileSize ?: -1L + @Suppress("DEPRECATION") + @Deprecated("Only for v1 repos") override fun hasChanged(): Boolean { return hasChanged } diff --git a/download/src/androidMain/kotlin/org/fdroid/fdroid/HashUtils.kt b/download/src/androidMain/kotlin/org/fdroid/fdroid/HashUtils.kt new file mode 100644 index 000000000..64e421b35 --- /dev/null +++ b/download/src/androidMain/kotlin/org/fdroid/fdroid/HashUtils.kt @@ -0,0 +1,13 @@ +package org.fdroid.fdroid + +import java.security.MessageDigest + +internal fun MessageDigest?.isMatching(sha256: String): Boolean { + if (this == null) return false + val hexDigest = digest().toHex() + return hexDigest.equals(sha256, ignoreCase = true) +} + +internal fun ByteArray.toHex(): String = joinToString(separator = "") { eachByte -> + "%02x".format(eachByte) +} diff --git a/download/src/androidTest/kotlin/org/fdroid/download/HttpDownloaderTest.kt b/download/src/androidTest/kotlin/org/fdroid/download/HttpDownloaderTest.kt index bca53c4c8..ab4b65ceb 100644 --- a/download/src/androidTest/kotlin/org/fdroid/download/HttpDownloaderTest.kt +++ b/download/src/androidTest/kotlin/org/fdroid/download/HttpDownloaderTest.kt @@ -19,6 +19,7 @@ import org.fdroid.runSuspend import org.junit.Assume.assumeTrue import org.junit.Rule import org.junit.rules.TemporaryFolder +import java.io.IOException import java.net.BindException import java.net.ServerSocket import kotlin.random.Random @@ -31,7 +32,7 @@ import kotlin.test.fail private const val TOR_SOCKS_PORT = 9050 -@Suppress("BlockingMethodInNonBlockingContext") +@Suppress("BlockingMethodInNonBlockingContext", "DEPRECATION") internal class HttpDownloaderTest { @get:Rule @@ -55,6 +56,39 @@ internal class HttpDownloaderTest { assertContentEquals(bytes, file.readBytes()) } + @Test + fun testDownloadWithCorrectHash() = runSuspend { + val file = folder.newFile() + val bytes = "We know the hash for this string".encodeToByteArray() + var progressReported = false + + val mockEngine = MockEngine { respond(bytes) } + val httpManager = HttpManager(userAgent, null, httpClientEngineFactory = get(mockEngine)) + val httpDownloader = HttpDownloader(httpManager, downloadRequest, file) + httpDownloader.setListener { _, totalBytes -> + assertEquals(bytes.size.toLong(), totalBytes) + progressReported = true + } + httpDownloader.download(bytes.size.toLong(), + "e3802e5f8ae3dc7bbf5f1f4f7fb825d9bce9d1ddce50ac564fcbcfdeb31f1b90") + + assertContentEquals(bytes, file.readBytes()) + assertTrue(progressReported) + } + + @Test(expected = IOException::class) + fun testDownloadWithWrongHash() = runSuspend { + val file = folder.newFile() + val bytes = "We know the hash for this string".encodeToByteArray() + + val mockEngine = MockEngine { respond(bytes) } + val httpManager = HttpManager(userAgent, null, httpClientEngineFactory = get(mockEngine)) + val httpDownloader = HttpDownloader(httpManager, downloadRequest, file) + httpDownloader.download(bytes.size.toLong(), "This is not the right hash") + + assertContentEquals(bytes, file.readBytes()) + } + @Test fun testResumeSuccess() = runSuspend { val file = folder.newFile() diff --git a/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt b/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt index 5d488c3e3..88f3ef6a3 100644 --- a/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt +++ b/download/src/commonMain/kotlin/org/fdroid/download/HttpManager.kt @@ -119,9 +119,10 @@ public open class HttpManager @JvmOverloads constructor( public suspend fun get( request: DownloadRequest, skipFirstBytes: Long? = null, - receiver: suspend (ByteArray) -> Unit, + receiver: BytesReceiver, ): Unit = mirrorChooser.mirrorRequest(request) { mirror, url -> getHttpStatement(request, mirror, url, skipFirstBytes).execute { response -> + val contentLength = response.contentLength() if (skipFirstBytes != null && response.status != PartialContent) { throw NoResumeException() } @@ -130,7 +131,7 @@ public open class HttpManager @JvmOverloads constructor( while (!channel.isClosedForRead) { val packet = channel.readRemaining(limit) while (!packet.isEmpty) { - receiver(packet.readBytes()) + receiver.receive(packet.readBytes(), contentLength) } } } @@ -179,7 +180,7 @@ public open class HttpManager @JvmOverloads constructor( skipFirstBytes: Long? = null, ): ByteArray { val channel = ByteChannel() - get(request, skipFirstBytes) { bytes -> + get(request, skipFirstBytes) { bytes, _ -> channel.writeFully(bytes) } channel.close() @@ -225,3 +226,4 @@ public open class HttpManager @JvmOverloads constructor( } public class NoResumeException : Exception() +public class NotFoundException(e: Throwable? = null) : Exception(e) diff --git a/download/src/commonMain/kotlin/org/fdroid/download/Mirror.kt b/download/src/commonMain/kotlin/org/fdroid/download/Mirror.kt index 79d03d3ee..4235cf13a 100644 --- a/download/src/commonMain/kotlin/org/fdroid/download/Mirror.kt +++ b/download/src/commonMain/kotlin/org/fdroid/download/Mirror.kt @@ -7,7 +7,7 @@ import io.ktor.http.appendPathSegments import mu.KotlinLogging public data class Mirror @JvmOverloads constructor( - private val baseUrl: String, + val baseUrl: String, val location: String? = null, ) { public val url: Url by lazy { @@ -34,6 +34,8 @@ public data class Mirror @JvmOverloads constructor( public fun isLocal(): Boolean = url.isLocal() + public fun isHttp(): Boolean = url.protocol.name.startsWith("http") + public companion object { @JvmStatic public fun fromStrings(list: List): List = list.map { Mirror(it) } diff --git a/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt b/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt index fad2a438b..18b67b25a 100644 --- a/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt +++ b/download/src/commonMain/kotlin/org/fdroid/download/MirrorChooser.kt @@ -2,6 +2,7 @@ package org.fdroid.download import io.ktor.client.plugins.ResponseException import io.ktor.http.HttpStatusCode.Companion.Forbidden +import io.ktor.http.HttpStatusCode.Companion.NotFound import io.ktor.http.Url import io.ktor.utils.io.errors.IOException import mu.KotlinLogging @@ -43,6 +44,8 @@ internal abstract class MirrorChooserImpl : MirrorChooser { } catch (e: ResponseException) { // don't try other mirrors if we got Forbidden response, but supplied credentials if (downloadRequest.hasCredentials && e.response.status == Forbidden) throw e + // don't try other mirrors if we got NotFount response and downloaded a repo + if (downloadRequest.tryFirstMirror != null && e.response.status == NotFound) throw e // also throw if this is the last mirror to try, otherwise try next throwOnLastMirror(e, index == downloadRequest.mirrors.size - 1) } catch (e: IOException) { diff --git a/download/src/commonMain/kotlin/org/fdroid/fdroid/ProgressListener.kt b/download/src/commonMain/kotlin/org/fdroid/fdroid/ProgressListener.kt index 8d988c6c3..b4a47b3b8 100644 --- a/download/src/commonMain/kotlin/org/fdroid/fdroid/ProgressListener.kt +++ b/download/src/commonMain/kotlin/org/fdroid/fdroid/ProgressListener.kt @@ -16,6 +16,6 @@ package org.fdroid.fdroid * * `int`s, i.e. [String.hashCode] * */ -public interface ProgressListener { +public fun interface ProgressListener { public fun onProgress(bytesRead: Long, totalBytes: Long) } diff --git a/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt b/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt index 38d8adf29..3d64b5435 100644 --- a/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt +++ b/download/src/commonTest/kotlin/org/fdroid/download/HttpManagerTest.kt @@ -18,6 +18,7 @@ import io.ktor.http.HttpHeaders.Range import io.ktor.http.HttpHeaders.UserAgent import io.ktor.http.HttpStatusCode.Companion.Forbidden import io.ktor.http.HttpStatusCode.Companion.InternalServerError +import io.ktor.http.HttpStatusCode.Companion.NotFound import io.ktor.http.HttpStatusCode.Companion.OK import io.ktor.http.HttpStatusCode.Companion.PartialContent import io.ktor.http.HttpStatusCode.Companion.TemporaryRedirect @@ -37,7 +38,7 @@ import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.test.fail -class HttpManagerTest { +internal class HttpManagerTest { private val userAgent = getRandomString() private val mirrors = listOf(Mirror("http://example.org"), Mirror("http://example.net/")) @@ -195,7 +196,29 @@ class HttpManagerTest { // assert there is only one request per API call using one of the mirrors assertEquals(2, mockEngine.requestHistory.size) mockEngine.requestHistory.forEach { request -> - println(mockEngine.requestHistory) + val url = request.url.toString() + assertTrue(url == "http://example.org/foo" || url == "http://example.net/foo") + } + } + + @Test + fun testNoMoreMirrorsWhenRepoDownloadNotFound() = runSuspend { + val downloadRequest = downloadRequest.copy(tryFirstMirror = mirrors[0]) + val mockEngine = MockEngine { respond("", NotFound) } + val httpManager = HttpManager(userAgent, null, httpClientEngineFactory = get(mockEngine)) + + assertTrue(downloadRequest.tryFirstMirror != null) + + assertNull(httpManager.head(downloadRequest)) + val e = assertFailsWith { + httpManager.getBytes(downloadRequest) + } + + // assert that the exception reflects the NotFound error + assertEquals(NotFound, e.response.status) + // assert there is only one request per API call using one of the mirrors + assertEquals(2, mockEngine.requestHistory.size) + mockEngine.requestHistory.forEach { request -> val url = request.url.toString() assertTrue(url == "http://example.org/foo" || url == "http://example.net/foo") }