[download] Add new download method for v2 index

that receives total file size and ensures that the downloaded file has the provided sha256 hash
This commit is contained in:
Torsten Grote
2022-05-18 11:46:20 -03:00
committed by Michael Pöhn
parent c92d64a36b
commit a830f1ef86
12 changed files with 181 additions and 29 deletions

View File

@@ -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);

View File

@@ -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()) {

View File

@@ -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

View File

@@ -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?)
}

View File

@@ -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
}

View File

@@ -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)
}

View File

@@ -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()

View File

@@ -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)

View File

@@ -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<String>): List<Mirror> = list.map { Mirror(it) }

View File

@@ -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) {

View File

@@ -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)
}

View File

@@ -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<ClientRequestException> {
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")
}