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 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..1cbf621d36 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 @@ -33,6 +33,7 @@ import org.matrix.android.sdk.internal.di.SessionDownloadsDirectory import org.matrix.android.sdk.internal.di.UnauthenticatedWithCertificateWithProgress import org.matrix.android.sdk.internal.session.download.DownloadProgressInterceptor.Companion.DOWNLOAD_PROGRESS_INTERCEPTOR_HEADER import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers +import org.matrix.android.sdk.internal.util.file.AtomicFileCreator import org.matrix.android.sdk.internal.util.md5 import org.matrix.android.sdk.internal.util.writeToFile import timber.log.Timber @@ -96,6 +97,9 @@ internal class DefaultFileService @Inject constructor( } } + var atomicFileDownload: AtomicFileCreator? = null + var atomicFileDecrypt: AtomicFileCreator? = null + if (existingDownload != null) { // FIXME If the first downloader cancels then we'll unfortunately be cancelled too. return existingDownload.await() @@ -131,8 +135,11 @@ 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 part file first, so if we abort before done, we don't have a broken cached file + val atomicFileCreator = AtomicFileCreator(cachedFiles.file).also { atomicFileDownload = it } + writeToFile(source.inputStream(), atomicFileCreator.partFile) response.close() + atomicFileCreator.commit() } else { Timber.v("## FileService: cache hit for $url") } @@ -145,8 +152,10 @@ internal class DefaultFileService @Inject constructor( Timber.v("## FileService: decrypt file") // Ensure the parent folder exists cachedFiles.decryptedFile.parentFile?.mkdirs() + // Write to a part file first, so if we abort before done, we don't have a broken cached file + val atomicFileCreator = AtomicFileCreator(cachedFiles.decryptedFile).also { atomicFileDecrypt = it } val decryptSuccess = cachedFiles.file.inputStream().use { inputStream -> - cachedFiles.decryptedFile.outputStream().buffered().use { outputStream -> + atomicFileCreator.partFile.outputStream().buffered().use { outputStream -> MXEncryptedAttachments.decryptAttachment( inputStream, elementToDecrypt, @@ -154,6 +163,7 @@ internal class DefaultFileService @Inject constructor( ) } } + atomicFileCreator.commit() if (!decryptSuccess) { throw IllegalStateException("Decryption error") } @@ -174,6 +184,11 @@ internal class DefaultFileService @Inject constructor( } toNotify?.completeWith(result) + result.onFailure { + atomicFileDownload?.cancel() + atomicFileDecrypt?.cancel() + } + return result.getOrThrow() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/AtomicFileCreator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/AtomicFileCreator.kt new file mode 100644 index 0000000000..ca10c0ed0f --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/AtomicFileCreator.kt @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.util.file + +import timber.log.Timber +import java.io.File + +internal class AtomicFileCreator(private val file: File) { + val partFile = File(file.parentFile, "${file.name}.part") + + init { + if (file.exists()) { + Timber.w("## AtomicFileCreator: target file ${file.path} exists, it should not happen.") + } + if (partFile.exists()) { + Timber.d("## AtomicFileCreator: discard aborted part file ${partFile.path}") + // No need to delete the file, we will overwrite it + } + } + + fun cancel() { + partFile.delete() + } + + fun commit() { + partFile.renameTo(file) + } +}