fix: crash when saving file - duplicate names [WPB-5026] (#2402)

Co-authored-by: GitHub Actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Michał Saleniuk <[email protected]> Co-authored-by: Michał Saleniuk <[email protected]>
wireapp · Nov 10, 2023 · 914723f · 914723f
1 parent a4b6f38
commit 914723f
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 7 deletions.
diff --git a/app/src/main/kotlin/com/wire/android/util/FileUtil.kt b/app/src/main/kotlin/com/wire/android/util/FileUtil.kt
@@ -132,19 +132,18 @@ suspend fun createPemFile(
 private fun Context.saveFileDataToDownloadsFolder(assetName: String, downloadedDataPath: Path, fileSize: Long): Uri? {
     val resolver = contentResolver
     val mimeType = Uri.parse(downloadedDataPath.toString()).getMimeType(this@saveFileDataToDownloadsFolder)
+    val downloadsDir = Environment.getExternalStoragePublicDirectory(DIRECTORY_DOWNLOADS)
+    // we need to find the next available name with copy counter by ourselves before copying
+    val availableAssetName = findFirstUniqueName(downloadsDir, assetName.ifEmpty { ATTACHMENT_FILENAME })
     return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
         val contentValues = ContentValues().apply {
-            // ContentResolver modifies the name if another file with the given name already exists, so we don't have to worry about it
-            put(DISPLAY_NAME, assetName.ifEmpty { ATTACHMENT_FILENAME })
+            put(DISPLAY_NAME, availableAssetName)
             put(MIME_TYPE, mimeType)
             put(SIZE, fileSize)
         }
         resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues)
     } else {
         val authority = getProviderAuthority()
-        val downloadsDir = Environment.getExternalStoragePublicDirectory(DIRECTORY_DOWNLOADS)
-        // we need to find the next available name with copy counter by ourselves before copying
-        val availableAssetName = findFirstUniqueName(downloadsDir, assetName.ifEmpty { ATTACHMENT_FILENAME })
         val destinationFile = File(downloadsDir, availableAssetName)
         val uri = FileProvider.getUriForFile(this, authority, destinationFile)
         if (mimeType?.isNotEmpty() == true) {
@@ -428,14 +427,22 @@ fun Context.getProviderAuthority() = "$packageName.provider"
 
 @VisibleForTesting
 fun findFirstUniqueName(dir: File, desiredName: String): String {
-    var currentName: String = desiredName
+    var currentName: String = desiredName.sanitizeFilename()
     while (File(dir, currentName).exists()) {
         val (nameWithoutCopyCounter, copyCounter, extension) = currentName.splitFileExtensionAndCopyCounter()
-        currentName = buildFileName(nameWithoutCopyCounter, extension, copyCounter + 1)
+        currentName = buildFileName(nameWithoutCopyCounter, extension, copyCounter + 1).sanitizeFilename()
     }
     return currentName
 }
 
+/**
+ * Removes disallowed characters and returns valid filename.
+ *
+ * Uses the same cases as in `isValidFatFilenameChar` and `isValidExtFilenameChar` from [android.os.FileUtils].
+ */
+@VisibleForTesting
+fun String.sanitizeFilename(): String = replace(Regex("[\u0000-\u001f\u007f\"*/:<>?\\\\|]"), "_")
+
 fun getAudioLengthInMs(dataPath: Path, mimeType: String): Long =
     if (isAudioMimeType(mimeType)) {
         val retriever = MediaMetadataRetriever()

diff --git a/app/src/test/kotlin/com/wire/android/util/FileUtilTest.kt b/app/src/test/kotlin/com/wire/android/util/FileUtilTest.kt
@@ -60,4 +60,13 @@ class FileUtilTest {
         val result = findFirstUniqueName(tempDir, desired)
         assertEquals(expected, result)
     }
+
+    @Test
+    fun `given file with invalid filename when finding unique name in directory then return name without disallowed characters`() {
+        val desired = "\u0020ab\u0008cd\u0000ef\u001fgh\u007Fij*kl/mn:op<qr>st?uv\\wx|yz.jpg"
+        val expected = "\u0020ab_cd_ef_gh_ij_kl_mn_op_qr_st_uv_wx_yz.jpg"
+
+        val result = desired.sanitizeFilename()
+        assertEquals(expected, result)
+    }
 }