From f8b2bc053020e51b077f0b6800298d822398fabf Mon Sep 17 00:00:00 2001 From: Christian Rowlands Date: Tue, 15 Oct 2024 20:00:06 -0400 Subject: [PATCH 1/5] #6449 Adds support for additional character scripts in file names --- changelog.d/6449.bugfix | 1 + .../internal/session/DefaultFileService.kt | 25 +--- .../sdk/internal/util/file/FileUtil.kt | 49 ++++++++ .../android/sdk/internal/util/FileUtilTest.kt | 113 ++++++++++++++++++ 4 files changed, 165 insertions(+), 23 deletions(-) create mode 100644 changelog.d/6449.bugfix create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt diff --git a/changelog.d/6449.bugfix b/changelog.d/6449.bugfix new file mode 100644 index 00000000000..203b01a0eb3 --- /dev/null +++ b/changelog.d/6449.bugfix @@ -0,0 +1 @@ +Extended file name support to include characters from multiple languages, including Cyrillic and Han scripts. ([#6449](https://github.com/element-hq/element-android/issues/6449)) 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 217ef438211..cc11f0e1b92 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 @@ -41,6 +41,7 @@ import org.matrix.android.sdk.internal.network.httpclient.addAuthenticationHeade import org.matrix.android.sdk.internal.network.token.AccessTokenProvider import org.matrix.android.sdk.internal.session.download.DownloadProgressInterceptor.Companion.DOWNLOAD_PROGRESS_INTERCEPTOR_HEADER import org.matrix.android.sdk.internal.util.file.AtomicFileCreator +import org.matrix.android.sdk.internal.util.file.safeFileName import org.matrix.android.sdk.internal.util.time.Clock import org.matrix.android.sdk.internal.util.writeToFile import timber.log.Timber @@ -247,28 +248,6 @@ internal class DefaultFileService @Inject constructor( } } - private fun safeFileName(fileName: String?, mimeType: String?): String { - return buildString { - // filename has to be safe for the Android System - val result = fileName - ?.replace("[^a-z A-Z0-9\\\\.\\-]".toRegex(), "_") - ?.takeIf { it.isNotEmpty() } - ?: DEFAULT_FILENAME - append(result) - // Check that the extension is correct regarding the mimeType - val extensionFromMime = mimeType?.let { MimeTypeMap.getSingleton().getExtensionFromMimeType(mimeType) } - if (extensionFromMime != null) { - // Compare - val fileExtension = result.substringAfterLast(delimiter = ".", missingDelimiterValue = "") - if (fileExtension.isEmpty() || fileExtension != extensionFromMime) { - // Missing extension, or diff in extension, add the one provided by the mimetype - append(".") - append(extensionFromMime) - } - } - } - } - override fun isFileInCache( mxcUrl: String?, fileName: String, @@ -368,6 +347,6 @@ internal class DefaultFileService @Inject constructor( private const val ENCRYPTED_FILENAME = "encrypted.bin" // The extension would be added from the mimetype - private const val DEFAULT_FILENAME = "file" + const val DEFAULT_FILENAME = "file" } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt new file mode 100644 index 00000000000..49bdbdd0a96 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * 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 android.webkit.MimeTypeMap +import org.matrix.android.sdk.internal.session.DefaultFileService.Companion.DEFAULT_FILENAME +import timber.log.Timber + +/** + * Remove any characters from the file name that are not supported by the Android OS, + * and update the file extension to match the mimeType. + */ +fun safeFileName(fileName: String?, mimeType: String?): String { + return buildString { + // filename has to be safe for the Android System + Timber.i("ISSUE: FileService: original fileName $fileName") + val result = fileName + ?.replace("[^\\p{sc=Cyrillic}\\p{sc=Han}a-z A-Z0-9\\\\.\\-]".toRegex(), "_") + ?.takeIf { it.isNotEmpty() } + ?: DEFAULT_FILENAME + Timber.i("ISSUE: FileService: safeFileName $result") + append(result) + // Check that the extension is correct regarding the mimeType + val extensionFromMime = mimeType?.let { MimeTypeMap.getSingleton().getExtensionFromMimeType(mimeType) } + if (extensionFromMime != null) { + // Compare + val fileExtension = result.substringAfterLast(delimiter = ".", missingDelimiterValue = "") + if (fileExtension.isEmpty() || fileExtension != extensionFromMime) { + // Missing extension, or diff in extension, add the one provided by the mimetype + append(".") + append(extensionFromMime) + } + } + } +} diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt new file mode 100644 index 00000000000..fa88833b751 --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt @@ -0,0 +1,113 @@ +/* + * Copyright 2020 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 + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.matrix.android.sdk.internal.session.DefaultFileService.Companion.DEFAULT_FILENAME +import org.matrix.android.sdk.internal.util.file.safeFileName + +class FileUtilTest { + + @Test + fun `should return original filename when valid characters are used`() { + val fileName = "validFileName.txt" + val mimeType = "text/plain" + val result = safeFileName(fileName, mimeType) + assertEquals("validFileName.txt", result) + } + + @Test + fun `should replace invalid characters with underscores`() { + val fileName = "invalid/filename:with*chars?.txt" + val mimeType = "text/plain" + val result = safeFileName(fileName, mimeType) + assertEquals("invalid_filename_with_chars_.txt", result) + } + + @Test + fun `should allow Cyrillic characters in the filename`() { + val fileName = "тестовыйФайл.txt" + val mimeType = "text/plain" + val result = safeFileName(fileName, mimeType) + assertEquals("тестовыйФайл.txt", result) + } + + @Test + fun `should allow Han characters in the filename`() { + val fileName = "测试文件.txt" + val mimeType = "text/plain" + val result = safeFileName(fileName, mimeType) + assertEquals("测试文件.txt", result) + } + + @Test + fun `should return default filename when input is null`() { + val fileName = null + val mimeType = "text/plain" + val result = safeFileName(fileName, mimeType) + assertEquals(DEFAULT_FILENAME, result) + } + + @Test + fun `should add the correct extension when missing`() { + val fileName = "myDocument" + val mimeType = "application/pdf" + val result = safeFileName(fileName, mimeType) + assertEquals("myDocument.pdf", result) + } + + @Test + fun `should replace invalid characters and add the correct extension`() { + val fileName = "my*docu/ment" + val mimeType = "application/pdf" + val result = safeFileName(fileName, mimeType) + assertEquals("my_docu_ment.pdf", result) + } + + @Test + fun `should not modify the extension if it matches the mimeType`() { + val fileName = "report.pdf" + val mimeType = "application/pdf" + val result = safeFileName(fileName, mimeType) + assertEquals("report.pdf", result) + } + + @Test + fun `should replace spaces with underscores`() { + val fileName = "my report.doc" + val mimeType = "application/msword" + val result = safeFileName(fileName, mimeType) + assertEquals("my_report.doc", result) + } + + @Test + fun `should append extension if file name has none and mimeType is valid`() { + val fileName = "newfile" + val mimeType = "image/jpeg" + val result = safeFileName(fileName, mimeType) + assertEquals("newfile.jpg", result) + } + + @Test + fun `should keep hyphenated names intact`() { + val fileName = "my-file-name" + val mimeType = "application/octet-stream" + val result = safeFileName(fileName, mimeType) + assertEquals("my-file-name", result) + } +} From 686ca0512f830de64527f2e7e891144de155b5c4 Mon Sep 17 00:00:00 2001 From: Christian Rowlands Date: Wed, 16 Oct 2024 09:30:31 -0400 Subject: [PATCH 2/5] #6449 Switch to removing specific invalid characters instead of including different character scripts for file names --- .../android/sdk/internal/util/FileUtilTest.kt | 42 +++++++++++-------- .../sdk/internal/util/file/FileUtil.kt | 2 +- 2 files changed, 26 insertions(+), 18 deletions(-) rename matrix-sdk-android/src/{test => androidTest}/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt (69%) diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt similarity index 69% rename from matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt rename to matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt index fa88833b751..edcd3095fa8 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Matrix.org Foundation C.I.C. + * Copyright (c) 2024 New Vector Ltd * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,15 +16,23 @@ package org.matrix.android.sdk.internal.util +import androidx.test.ext.junit.runners.AndroidJUnit4 import org.junit.Assert.assertEquals import org.junit.Test +import org.junit.runner.RunWith +import org.matrix.android.sdk.InstrumentedTest import org.matrix.android.sdk.internal.session.DefaultFileService.Companion.DEFAULT_FILENAME import org.matrix.android.sdk.internal.util.file.safeFileName -class FileUtilTest { +/** + * These tests are run on an Android device because they need to use the static + * MimeTypeMap#getSingleton() method, which was failing in the unit test directory. + */ +@RunWith(AndroidJUnit4::class) +class FileUtilTest : InstrumentedTest { @Test - fun `should return original filename when valid characters are used`() { + fun shouldReturnOriginalFilenameWhenValidCharactersAreUsed() { val fileName = "validFileName.txt" val mimeType = "text/plain" val result = safeFileName(fileName, mimeType) @@ -32,15 +40,15 @@ class FileUtilTest { } @Test - fun `should replace invalid characters with underscores`() { + fun shouldReplaceInvalidCharactersWithUnderscores() { val fileName = "invalid/filename:with*chars?.txt" val mimeType = "text/plain" val result = safeFileName(fileName, mimeType) - assertEquals("invalid_filename_with_chars_.txt", result) + assertEquals("invalid/filename_with_chars_.txt", result) } @Test - fun `should allow Cyrillic characters in the filename`() { + fun shouldAllowCyrillicCharactersInTheFilename() { val fileName = "тестовыйФайл.txt" val mimeType = "text/plain" val result = safeFileName(fileName, mimeType) @@ -48,7 +56,7 @@ class FileUtilTest { } @Test - fun `should allow Han characters in the filename`() { + fun shouldAllowHanCharactersInTheFilename() { val fileName = "测试文件.txt" val mimeType = "text/plain" val result = safeFileName(fileName, mimeType) @@ -56,15 +64,15 @@ class FileUtilTest { } @Test - fun `should return default filename when input is null`() { + fun shouldReturnDefaultFilenameWhenInputIsNull() { val fileName = null val mimeType = "text/plain" val result = safeFileName(fileName, mimeType) - assertEquals(DEFAULT_FILENAME, result) + assertEquals("$DEFAULT_FILENAME.txt", result) } @Test - fun `should add the correct extension when missing`() { + fun shouldAddTheCorrectExtensionWhenMissing() { val fileName = "myDocument" val mimeType = "application/pdf" val result = safeFileName(fileName, mimeType) @@ -72,15 +80,15 @@ class FileUtilTest { } @Test - fun `should replace invalid characters and add the correct extension`() { + fun shouldReplaceInvalidCharactersAndAddTheCorrectExtension() { val fileName = "my*docu/ment" val mimeType = "application/pdf" val result = safeFileName(fileName, mimeType) - assertEquals("my_docu_ment.pdf", result) + assertEquals("my_docu/ment.pdf", result) } @Test - fun `should not modify the extension if it matches the mimeType`() { + fun shouldNotModifyTheExtensionIfItMatchesTheMimeType() { val fileName = "report.pdf" val mimeType = "application/pdf" val result = safeFileName(fileName, mimeType) @@ -88,7 +96,7 @@ class FileUtilTest { } @Test - fun `should replace spaces with underscores`() { + fun shouldReplaceSpacesWithUnderscores() { val fileName = "my report.doc" val mimeType = "application/msword" val result = safeFileName(fileName, mimeType) @@ -96,7 +104,7 @@ class FileUtilTest { } @Test - fun `should append extension if file name has none and mimeType is valid`() { + fun shouldAppendExtensionIfFileNameHasNoneAndMimeTypeIsValid() { val fileName = "newfile" val mimeType = "image/jpeg" val result = safeFileName(fileName, mimeType) @@ -104,10 +112,10 @@ class FileUtilTest { } @Test - fun `should keep hyphenated names intact`() { + fun shouldKeepHyphenatedNamesIntact() { val fileName = "my-file-name" val mimeType = "application/octet-stream" val result = safeFileName(fileName, mimeType) - assertEquals("my-file-name", result) + assertEquals("my-file-name.bin", result) } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt index 49bdbdd0a96..68d2d397759 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt @@ -29,7 +29,7 @@ fun safeFileName(fileName: String?, mimeType: String?): String { // filename has to be safe for the Android System Timber.i("ISSUE: FileService: original fileName $fileName") val result = fileName - ?.replace("[^\\p{sc=Cyrillic}\\p{sc=Han}a-z A-Z0-9\\\\.\\-]".toRegex(), "_") + ?.replace("[\\\\?%*:|\"<>\\s]".toRegex(), "_") ?.takeIf { it.isNotEmpty() } ?: DEFAULT_FILENAME Timber.i("ISSUE: FileService: safeFileName $result") From 11f6987a9871a0904c92b6380b66a94300da064b Mon Sep 17 00:00:00 2001 From: Christian Rowlands Date: Wed, 16 Oct 2024 10:17:49 -0400 Subject: [PATCH 3/5] #6449 Remove test logging for file name --- .../java/org/matrix/android/sdk/internal/util/file/FileUtil.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt index 68d2d397759..494f55b2dc0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt @@ -27,12 +27,10 @@ import timber.log.Timber fun safeFileName(fileName: String?, mimeType: String?): String { return buildString { // filename has to be safe for the Android System - Timber.i("ISSUE: FileService: original fileName $fileName") val result = fileName ?.replace("[\\\\?%*:|\"<>\\s]".toRegex(), "_") ?.takeIf { it.isNotEmpty() } ?: DEFAULT_FILENAME - Timber.i("ISSUE: FileService: safeFileName $result") append(result) // Check that the extension is correct regarding the mimeType val extensionFromMime = mimeType?.let { MimeTypeMap.getSingleton().getExtensionFromMimeType(mimeType) } From 2cfc230255b375c36b5e2dce2fee20eb010fc0fe Mon Sep 17 00:00:00 2001 From: Christian Rowlands Date: Tue, 12 Nov 2024 10:15:23 -0500 Subject: [PATCH 4/5] #6449 Use the correct name in the file headers --- .../java/org/matrix/android/sdk/internal/util/FileUtilTest.kt | 2 +- .../java/org/matrix/android/sdk/internal/util/file/FileUtil.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt index edcd3095fa8..687f4ef16d8 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/util/FileUtilTest.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024 New Vector Ltd + * Copyright (c) 2024 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. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt index 494f55b2dc0..d911f967c1f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024 New Vector Ltd + * Copyright (c) 2024 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. From 36e8b7b25a35bc69cec8698492f24af6eb8c7293 Mon Sep 17 00:00:00 2001 From: Christian Rowlands Date: Tue, 12 Nov 2024 10:21:06 -0500 Subject: [PATCH 5/5] #6449 Remove unused imports --- .../matrix/android/sdk/internal/session/DefaultFileService.kt | 1 - .../java/org/matrix/android/sdk/internal/util/file/FileUtil.kt | 1 - 2 files changed, 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 cc11f0e1b92..ca534f5a651 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 @@ -18,7 +18,6 @@ package org.matrix.android.sdk.internal.session import android.content.Context import android.net.Uri -import android.webkit.MimeTypeMap import androidx.core.content.FileProvider import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.completeWith diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt index d911f967c1f..5baf63a887b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/file/FileUtil.kt @@ -18,7 +18,6 @@ package org.matrix.android.sdk.internal.util.file import android.webkit.MimeTypeMap import org.matrix.android.sdk.internal.session.DefaultFileService.Companion.DEFAULT_FILENAME -import timber.log.Timber /** * Remove any characters from the file name that are not supported by the Android OS,