From 623151a5002c2be9249828083c4b9668d39fbb36 Mon Sep 17 00:00:00 2001 From: zerox80 Date: Thu, 4 Jun 2026 14:05:25 +0200 Subject: [PATCH] Fix truncated content URI upload cache --- .../workers/ContentUriUploadCacheValidator.kt | 27 ++++++ .../workers/UploadFileFromContentUriWorker.kt | 89 ++++++++++++++++--- .../ContentUriUploadCacheValidatorTest.kt | 88 ++++++++++++++++++ 3 files changed, 190 insertions(+), 14 deletions(-) create mode 100644 opencloudApp/src/main/java/eu/opencloud/android/workers/ContentUriUploadCacheValidator.kt create mode 100644 opencloudApp/src/test/java/eu/opencloud/android/workers/ContentUriUploadCacheValidatorTest.kt diff --git a/opencloudApp/src/main/java/eu/opencloud/android/workers/ContentUriUploadCacheValidator.kt b/opencloudApp/src/main/java/eu/opencloud/android/workers/ContentUriUploadCacheValidator.kt new file mode 100644 index 0000000000..3ffbf1cadc --- /dev/null +++ b/opencloudApp/src/main/java/eu/opencloud/android/workers/ContentUriUploadCacheValidator.kt @@ -0,0 +1,27 @@ +/** + * openCloud Android client application + * + * Copyright (C) 2026 OpenCloud GmbH. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package eu.opencloud.android.workers + +internal object ContentUriUploadCacheValidator { + fun isValidCacheSize( + actualSize: Long, + expectedSize: Long, + ): Boolean = + actualSize > 0 && (expectedSize <= 0 || actualSize == expectedSize) +} diff --git a/opencloudApp/src/main/java/eu/opencloud/android/workers/UploadFileFromContentUriWorker.kt b/opencloudApp/src/main/java/eu/opencloud/android/workers/UploadFileFromContentUriWorker.kt index 08eedd26e2..71782420c2 100644 --- a/opencloudApp/src/main/java/eu/opencloud/android/workers/UploadFileFromContentUriWorker.kt +++ b/opencloudApp/src/main/java/eu/opencloud/android/workers/UploadFileFromContentUriWorker.kt @@ -147,18 +147,37 @@ class UploadFileFromContentUriWorker( cachePath = localStorageProvider.getTemporalPath(account.name, ocTransfer.spaceId) + File.separator + flatCacheName - // Re-copy if the cache file is missing or empty. A previous run may have copied it - // and then had it removed (e.g. by removeCacheFile() at the end of a successful run - // that the OS killed before bookkeeping). Only the contentUri from worker params is - // authoritative. val cacheFile = File(cachePath) - if (!cacheFile.exists() || cacheFile.length() == 0L) { + if (!isCacheFileReadyForUpload(cacheFile)) { checkDocumentFileExists() checkPermissionsToReadDocumentAreGranted() copyFileToLocalStorage() } } + private fun isCacheFileReadyForUpload(cacheFile: File): Boolean { + if (!cacheFile.exists()) return false + + val cacheSize = cacheFile.length() + val isValidCacheSize = ContentUriUploadCacheValidator.isValidCacheSize( + actualSize = cacheSize, + expectedSize = ocTransfer.fileSize, + ) + if (isValidCacheSize) return true + + Timber.w( + "Cached upload file for %s has invalid size. expected=%d actual=%d. Deleting and recopying.", + contentUri, + ocTransfer.fileSize, + cacheSize, + ) + if (!cacheFile.delete()) { + Timber.w("Could not delete invalid cached upload file: %s", cacheFile.absolutePath) + } + clearTusState() + return false + } + private fun areParametersValid(): Boolean { val paramAccountName = workerParameters.inputData.getString(KEY_PARAM_ACCOUNT_NAME) val paramUploadPath = workerParameters.inputData.getString(KEY_PARAM_UPLOAD_PATH) @@ -208,11 +227,14 @@ class UploadFileFromContentUriWorker( private fun copyFileToLocalStorage() { val documentFile = DocumentFile.fromSingleUri(appContext, contentUri) val cacheFile = File(cachePath) + val partFile = File("$cachePath.part") val cacheDir = cacheFile.parentFile if (cacheDir != null && !cacheDir.exists()) { cacheDir.mkdirs() } - cacheFile.createNewFile() + if (partFile.exists() && !partFile.delete()) { + Timber.w("Could not delete stale partial cache file: %s", partFile.absolutePath) + } // openInputStream can return null if the content provider is unavailable or permissions were revoked. // Failing here avoids silently uploading a 0-byte file. @@ -221,18 +243,57 @@ class UploadFileFromContentUriWorker( Timber.e("Failed to open input stream for %s — content provider unavailable or permissions revoked", contentUri) throw LocalFileNotFoundException() } - val outputStream = FileOutputStream(cachePath) - inputStream.use { input -> - outputStream.use { output -> - input.copyTo(output) + try { + val outputStream = FileOutputStream(partFile) + inputStream.use { input -> + outputStream.use { output -> + input.copyTo(output) + } } + } catch (throwable: Throwable) { + partFile.delete() + throw throwable } - // Guard against a truncated or empty copy (e.g. file deleted mid-read). - if (cacheFile.length() == 0L) { - Timber.e("Cache file is 0 bytes after copy from %s — source may have been deleted mid-read", contentUri) + val copiedSize = partFile.length() + if (!ContentUriUploadCacheValidator.isValidCacheSize(copiedSize, ocTransfer.fileSize)) { + Timber.e( + "Partial cache copy from %s. expected=%d actual=%d", + contentUri, + ocTransfer.fileSize, + copiedSize, + ) + partFile.delete() + clearTusState() + throw IOException( + "Cache copy size mismatch for $contentUri: " + + "expected ${ocTransfer.fileSize} bytes, copied $copiedSize bytes" + ) + } + + if (cacheFile.exists() && !cacheFile.delete()) { + partFile.delete() + throw IOException("Could not replace cached upload file: ${cacheFile.absolutePath}") + } + if (!partFile.renameTo(cacheFile)) { + partFile.delete() + throw IOException("Could not finalize cached upload file: ${cacheFile.absolutePath}") + } + + val finalSize = cacheFile.length() + if (!ContentUriUploadCacheValidator.isValidCacheSize(finalSize, ocTransfer.fileSize)) { + Timber.e( + "Invalid finalized cache copy from %s. expected=%d actual=%d", + contentUri, + ocTransfer.fileSize, + finalSize, + ) cacheFile.delete() - throw LocalFileNotFoundException() + clearTusState() + throw IOException( + "Final cache copy size mismatch for $contentUri: " + + "expected ${ocTransfer.fileSize} bytes, copied $finalSize bytes" + ) } transferRepository.updateTransferSourcePath(uploadIdInStorageManager, contentUri.toString()) diff --git a/opencloudApp/src/test/java/eu/opencloud/android/workers/ContentUriUploadCacheValidatorTest.kt b/opencloudApp/src/test/java/eu/opencloud/android/workers/ContentUriUploadCacheValidatorTest.kt new file mode 100644 index 0000000000..0b2d88a9a1 --- /dev/null +++ b/opencloudApp/src/test/java/eu/opencloud/android/workers/ContentUriUploadCacheValidatorTest.kt @@ -0,0 +1,88 @@ +/** + * openCloud Android client application + * + * Copyright (C) 2026 OpenCloud GmbH. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package eu.opencloud.android.workers + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ContentUriUploadCacheValidatorTest { + + @Test + fun `exact large cache size is valid`() { + val fileSize = 5_832_800_958L + + val isValid = ContentUriUploadCacheValidator.isValidCacheSize( + actualSize = fileSize, + expectedSize = fileSize, + ) + + assertTrue(isValid) + } + + @Test + fun `partial non-zero cache size is invalid`() { + val isValid = ContentUriUploadCacheValidator.isValidCacheSize( + actualSize = 1_180_123_136L, + expectedSize = 5_832_800_958L, + ) + + assertFalse(isValid) + } + + @Test + fun `larger than expected cache size is invalid`() { + val isValid = ContentUriUploadCacheValidator.isValidCacheSize( + actualSize = 5_832_800_959L, + expectedSize = 5_832_800_958L, + ) + + assertFalse(isValid) + } + + @Test + fun `zero byte cache size is invalid`() { + val isValid = ContentUriUploadCacheValidator.isValidCacheSize( + actualSize = 0L, + expectedSize = 5_832_800_958L, + ) + + assertFalse(isValid) + } + + @Test + fun `unknown expected size keeps existing non-zero behavior`() { + val isValid = ContentUriUploadCacheValidator.isValidCacheSize( + actualSize = 42L, + expectedSize = -1L, + ) + + assertTrue(isValid) + } + + @Test + fun `non-positive expected size still rejects zero byte cache`() { + val isValid = ContentUriUploadCacheValidator.isValidCacheSize( + actualSize = 0L, + expectedSize = -1L, + ) + + assertFalse(isValid) + } +}