From 4ef1f5c90f62fc9f03367d412b0fa0af4f8afda1 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sat, 10 Jul 2021 10:51:27 +0200 Subject: [PATCH 1/2] Avoid incomplete downloads in cache Previously, when a download was aborted (e.g. due to a bad internet connection), a partly downloaded file was remaining in cache, which would then be delivered upon later requests. This can lead e.g. to chats where images aren't loading. To avoid this, first download files to a temporary file that is not the final cache file, and only rename/move it on finish. Note that if you already have broken downloads, you still need to clear cache once to get rid of them after this commit, but it should not occur anymore afterwards. --- .../sdk/internal/session/DefaultFileService.kt | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultFileService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultFileService.kt index a284d976d0..ba32bca6d6 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultFileService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultFileService.kt @@ -131,8 +131,14 @@ internal class DefaultFileService @Inject constructor( Timber.v("Response size ${response.body?.contentLength()} - Stream available: ${!source.exhausted()}") // Write the file to cache (encrypted version if the file is encrypted) - writeToFile(source.inputStream(), cachedFiles.file) + // Write to a tmp file first, so if we abort before done, we don't have a broken cached file + val tmpFile = File(cachedFiles.file.parentFile, "${cachedFiles.file.name}.tmp") + if (tmpFile.exists()) { + Timber.v("## FileService: discard aborted tmp file ${tmpFile.path}") + } + writeToFile(source.inputStream(), tmpFile) response.close() + tmpFile.renameTo(cachedFiles.file) } else { Timber.v("## FileService: cache hit for $url") } @@ -145,8 +151,13 @@ internal class DefaultFileService @Inject constructor( Timber.v("## FileService: decrypt file") // Ensure the parent folder exists cachedFiles.decryptedFile.parentFile?.mkdirs() + // Write to a tmp file first, so if we abort before done, we don't have a broken cached file + val tmpFile = File(cachedFiles.decryptedFile.parentFile, "${cachedFiles.decryptedFile.name}.tmp") + if (tmpFile.exists()) { + Timber.v("## FileService: discard aborted tmp file ${tmpFile.path}") + } val decryptSuccess = cachedFiles.file.inputStream().use { inputStream -> - cachedFiles.decryptedFile.outputStream().buffered().use { outputStream -> + tmpFile.outputStream().buffered().use { outputStream -> MXEncryptedAttachments.decryptAttachment( inputStream, elementToDecrypt, @@ -154,6 +165,7 @@ internal class DefaultFileService @Inject constructor( ) } } + tmpFile.renameTo(cachedFiles.decryptedFile) if (!decryptSuccess) { throw IllegalStateException("Decryption error") } From 512e1b339dcaef0c63a4d2a11de315725fc02c3d Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sat, 10 Jul 2021 11:07:21 +0200 Subject: [PATCH 2/2] Add changelog.d/3656.bugfix --- changelog.d/3656.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3656.bugfix diff --git a/changelog.d/3656.bugfix b/changelog.d/3656.bugfix new file mode 100644 index 0000000000..30d451558f --- /dev/null +++ b/changelog.d/3656.bugfix @@ -0,0 +1 @@ +Avoid incomplete downloads in cache