From bcb7ff01bb9c4616e649929ffdc2cfab12df3e55 Mon Sep 17 00:00:00 2001
From: SpiritCroc <dev@spiritcroc.de>
Date: Sat, 10 Jul 2021 10:51:27 +0200
Subject: [PATCH] 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.

Change-Id: Ic4fb58853f04f8239c639814031e9ef00c091995
---
 .../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")
                     }