diff --git a/commons-lib/src/androidTest/kotlin/ee/ria/DigiDoc/common/ConstantTest.kt b/commons-lib/src/androidTest/kotlin/ee/ria/DigiDoc/common/ConstantTest.kt index a12dc5565..c28d0657f 100644 --- a/commons-lib/src/androidTest/kotlin/ee/ria/DigiDoc/common/ConstantTest.kt +++ b/commons-lib/src/androidTest/kotlin/ee/ria/DigiDoc/common/ConstantTest.kt @@ -22,6 +22,7 @@ package ee.ria.DigiDoc.common import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test import java.time.ZoneOffset @@ -70,8 +71,11 @@ class ConstantTest { @Test fun testFilenameRestrictions() { - assertTrue(Constant.RESTRICTED_FILENAME_CHARACTERS_AND_RTL_CHARACTERS_AS_STRING.contains('@')) - assertTrue(Constant.RESTRICTED_FILENAME_CHARACTERS_AND_RTL_CHARACTERS_AS_STRING.contains('\u202E')) + assertTrue(Constant.FORBIDDEN_FILENAME_CHARACTERS.contains('/')) + assertTrue(Constant.FORBIDDEN_FILENAME_CHARACTERS.contains('*')) + assertFalse(Constant.FORBIDDEN_FILENAME_CHARACTERS.contains('@')) + // The app adds " (99)", an extension and "-data-files" to a name later + assertTrue(Constant.MAX_FILENAME_BYTES + 25 <= 255) } @Test diff --git a/commons-lib/src/main/kotlin/ee/ria/DigiDoc/common/Constant.kt b/commons-lib/src/main/kotlin/ee/ria/DigiDoc/common/Constant.kt index cd6dfc642..b9ac502c5 100644 --- a/commons-lib/src/main/kotlin/ee/ria/DigiDoc/common/Constant.kt +++ b/commons-lib/src/main/kotlin/ee/ria/DigiDoc/common/Constant.kt @@ -115,10 +115,14 @@ object Constant { const val TSL_SEQUENCE_NUMBER_ELEMENT: String = "TSLSequenceNumber" const val KEY_LOCALE = "locale" - private const val RESTRICTED_FILENAME_CHARACTERS_AS_STRING = "@%:^?[]\\'\"”’{}#&`\\\\~«»/´" - private const val RTL_CHARACTERS_AS_STRING = "" + '\u200E' + '\u200F' + '\u202E' + '\u202A' + '\u202B' - const val RESTRICTED_FILENAME_CHARACTERS_AND_RTL_CHARACTERS_AS_STRING = - RESTRICTED_FILENAME_CHARACTERS_AS_STRING + RTL_CHARACTERS_AS_STRING + + const val FORBIDDEN_FILENAME_CHARACTERS = "/\\<>:\"|?*" + + const val ZERO_WIDTH_JOINER_CODE = 0x200D + + // 255 bytes is the file system limit. The rest is room for the " (99)", + // extension and "-data-files" that the app adds on top of this name. + const val MAX_FILENAME_BYTES = 230 const val DEFAULT_FILENAME = "newFile" const val ALLOWED_URL_CHARACTERS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_,.:/%;+=@?&!()" diff --git a/crypto-lib/src/main/kotlin/ee/ria/DigiDoc/cryptolib/CryptoContainer.kt b/crypto-lib/src/main/kotlin/ee/ria/DigiDoc/cryptolib/CryptoContainer.kt index a53db026f..f07d8e25a 100644 --- a/crypto-lib/src/main/kotlin/ee/ria/DigiDoc/cryptolib/CryptoContainer.kt +++ b/crypto-lib/src/main/kotlin/ee/ria/DigiDoc/cryptolib/CryptoContainer.kt @@ -40,6 +40,7 @@ import ee.ria.DigiDoc.utilsLib.container.ContainerUtil import ee.ria.DigiDoc.utilsLib.extensions.isCryptoContainer import ee.ria.DigiDoc.utilsLib.extensions.saveAs import ee.ria.DigiDoc.utilsLib.file.FileUtil.sanitizeString +import ee.ria.DigiDoc.utilsLib.file.FileUtil.uniqueFileName import ee.ria.DigiDoc.utilsLib.logging.LoggingUtil.Companion.debugLog import ee.ria.DigiDoc.utilsLib.logging.LoggingUtil.Companion.errorLog import ee.ria.cdoc.CDoc @@ -65,7 +66,6 @@ import java.io.FileInputStream import java.io.FileOutputStream import java.io.IOException import java.io.InputStream -import java.io.OutputStream import java.util.Base64 import javax.inject.Inject import javax.inject.Singleton @@ -164,7 +164,9 @@ class CryptoContainer companion object { val logger = JavaLogger() - var loggingIsSet = false + + private val logLevelLock = Any() + private val libcdocLogLevel = LogLevel.LEVEL_TRACE @Throws(CryptoException::class) private suspend fun open( @@ -286,50 +288,59 @@ class CryptoContainer val cdocReader = CDocReader.createReader(file.path, conf, token, network) debugLog(LOG_TAG, "Reader created: (version ${cdocReader.version})") - val idx = cdocReader.getLockForCert(authCert) + try { + val idx = cdocReader.getLockForCert(authCert) - if (idx < 0) { - throw CryptoException("Failed to get lock for certificate") - } + if (idx < 0) { + throw CryptoException("Failed to get lock for certificate") + } - val fmk = cdocReader.getFMK(idx.toInt()) + val fmk = cdocReader.getFMK(idx.toInt()) - if (token.lastError != null) { - throw token.lastError as Throwable - } + if (token.lastError != null) { + throw token.lastError as Throwable + } - if (fmk.isEmpty()) { - throw CryptoException("Failed to get FMK") - } + if (fmk.isEmpty()) { + throw CryptoException("Failed to get FMK") + } - if (cdocReader.beginDecryption(fmk) != 0L) { - throw CryptoException("Failed to begin decryption") - } + if (cdocReader.beginDecryption(fmk) != 0L) { + throw CryptoException("Failed to begin decryption") + } - val fi = FileInfo() - var result: Long = cdocReader.nextFile(fi) - try { - while (result == CDoc.OK.toLong()) { - val ofile = File(fi.name) - val dir = - ContainerUtil.getContainerDataFilesDir( - context, - file, - ) - val tmp = sanitizeString(ofile.name, "") - val fileToSave = File(dir, tmp) - val ofs: OutputStream = FileOutputStream(fileToSave) - cdocReader.readFile(ofs) - dataFiles.add(fileToSave) - ofs.close() - result = cdocReader.nextFile(fi) + val fi = FileInfo() + val savedNames = mutableSetOf() + val dir = ContainerUtil.getContainerDataFilesDir(context, file) + + synchronized(logLevelLock) { + // Some file names crash the app if libcdoc logs them while reading + logger.setMinLogLevel(LogLevel.LEVEL_INFO) + try { + var result: Long = cdocReader.nextFile(fi) + while (result == CDoc.OK.toLong()) { + val ofile = File(fi.name) + val tmp = uniqueFileName(sanitizeString(ofile.name, ""), savedNames) + savedNames.add(tmp) + val fileToSave = File(dir, tmp) + FileOutputStream(fileToSave).use { ofs -> + cdocReader.readFile(ofs) + } + dataFiles.add(fileToSave) + result = cdocReader.nextFile(fi) + } + + if (cdocReader.finishDecryption() != 0L) { + throw CryptoException("Failed to finish decryption") + } + } finally { + logger.setMinLogLevel(libcdocLogLevel) + } } } catch (exc: IOException) { throw CryptoException("IO Exception: ${exc.message}", exc) - } - - if (cdocReader.finishDecryption() != 0L) { - throw CryptoException("Failed to finish decryption") + } finally { + cdocReader.delete() } return create( @@ -499,11 +510,8 @@ class CryptoContainer fun setLogging(isLoggingEnabled: Boolean) { if (isLoggingEnabled) { - logger.setMinLogLevel(LogLevel.LEVEL_TRACE) - if (!loggingIsSet) { - CDoc.setLogger(logger) - loggingIsSet = true - } + logger.setMinLogLevel(libcdocLogLevel) + CDoc.setLogger(logger) CDoc.log(LogLevel.LEVEL_DEBUG, "CryptoContainer", 450, "Set libcdoc logging: true") } } diff --git a/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/container/ContainerUtil.kt b/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/container/ContainerUtil.kt index 224324582..65a3b6940 100644 --- a/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/container/ContainerUtil.kt +++ b/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/container/ContainerUtil.kt @@ -207,6 +207,7 @@ object ContainerUtil { if (i > 0) { name.append(i) } + dir = File(directory, name.toString()) if (dir.isDirectory || !dir.exists()) { break @@ -220,6 +221,8 @@ object ContainerUtil { if (directory != null) { debugLog(LOG_TAG, "Directories created or already exist for " + directory.path) } + } else if (!dir.isDirectory) { + errorLog(LOG_TAG, "Unable to create data file directory, name is ${dir.name.toByteArray().size} bytes") } return dir diff --git a/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtil.kt b/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtil.kt index 5b923d642..fc9de1817 100644 --- a/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtil.kt +++ b/utils-lib/src/main/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtil.kt @@ -28,7 +28,9 @@ import android.webkit.URLUtil import androidx.core.net.toUri import ee.ria.DigiDoc.common.Constant.ALLOWED_URL_CHARACTERS import ee.ria.DigiDoc.common.Constant.DEFAULT_FILENAME -import ee.ria.DigiDoc.common.Constant.RESTRICTED_FILENAME_CHARACTERS_AND_RTL_CHARACTERS_AS_STRING +import ee.ria.DigiDoc.common.Constant.FORBIDDEN_FILENAME_CHARACTERS +import ee.ria.DigiDoc.common.Constant.MAX_FILENAME_BYTES +import ee.ria.DigiDoc.common.Constant.ZERO_WIDTH_JOINER_CODE import ee.ria.DigiDoc.utilsLib.logging.LoggingUtil.Companion.errorLog import ee.ria.DigiDoc.utilsLib.logging.LoggingUtil.Companion.infoLog import kotlinx.coroutines.Dispatchers @@ -55,6 +57,8 @@ import java.io.OutputStreamWriter import java.nio.charset.Charset import java.nio.charset.StandardCharsets import java.nio.file.Files +import java.text.BreakIterator +import java.text.Normalizer import javax.xml.parsers.DocumentBuilderFactory import javax.xml.parsers.SAXParserFactory @@ -124,26 +128,85 @@ object FileUtil { } else if (trimmed.startsWith(".")) { trimmed = DEFAULT_FILENAME + trimmed } + if (isRawUrl(trimmed)) { + return FilenameUtils.getName(FilenameUtils.normalize(trimmed)) ?: DEFAULT_FILENAME + } + if (URLUtil.isValidUrl(trimmed)) { + return FilenameUtils.getName(normalizeUri(trimmed.toUri()).toString()) ?: DEFAULT_FILENAME + } val sb = StringBuilder(trimmed.length) - if (!URLUtil.isValidUrl(trimmed) && !isRawUrl(trimmed)) { - for (element in trimmed) { - if (RESTRICTED_FILENAME_CHARACTERS_AND_RTL_CHARACTERS_AS_STRING.indexOf(element) != -1) { - sb.append(replacement) - } else { - sb.append(element) - } + for (element in trimmed) { + if (isForbiddenInFileName(element)) { + sb.append(replacement) + } else { + sb.append(element) } - } else if (!isRawUrl(trimmed)) { - return normalizeUri(trimmed.toUri()).toString() } - return if (sb.toString().isNotEmpty()) { + val name: String = FilenameUtils.getName( FilenameUtils.normalize( - sb.toString(), + sb.toString().trim { it <= ' ' }, ), - ) + ) ?: "" + return if (name.isEmpty() || name.all { it == '.' }) { + DEFAULT_FILENAME } else { - FilenameUtils.normalize(trimmed) + truncateFileName(Normalizer.normalize(name, Normalizer.Form.NFC), MAX_FILENAME_BYTES) + } + } + + private fun isForbiddenInFileName(character: Char): Boolean { + if (character.code == ZERO_WIDTH_JOINER_CODE) { + return false + } + return FORBIDDEN_FILENAME_CHARACTERS.indexOf(character) != -1 || + character.category == CharCategory.CONTROL || + character.category == CharCategory.FORMAT + } + + fun truncateFileName( + fileName: String, + maxBytes: Int, + ): String { + if (fileName.toByteArray().size <= maxBytes) { + return fileName + } + val extension = FilenameUtils.getExtension(fileName) + val suffix = if (extension.isEmpty()) "" else ".$extension" + val baseName = FilenameUtils.getBaseName(fileName) + val truncated = truncateToBytes(baseName, maxBytes - suffix.toByteArray().size) + + val fitted = if (truncated.isEmpty()) truncateToBytes(fileName, maxBytes) else truncated + suffix + + return fitted.ifEmpty { truncateToBytes(DEFAULT_FILENAME + suffix, maxBytes) } + } + + private fun truncateToBytes( + text: String, + maxBytes: Int, + ): String { + val characters = BreakIterator.getCharacterInstance() + characters.setText(text) + val cuts = generateSequence(characters.first()) { characters.next().takeIf { it != BreakIterator.DONE } } + return text.substring(0, cuts.lastOrNull { text.substring(0, it).toByteArray().size <= maxBytes } ?: 0) + } + + fun uniqueFileName( + fileName: String, + taken: Set, + ): String { + if (!taken.contains(fileName)) { + return fileName + } + val baseName = FilenameUtils.getBaseName(fileName).ifEmpty { DEFAULT_FILENAME } + val extension = FilenameUtils.getExtension(fileName) + var counter = 1 + while (true) { + val candidate = if (extension.isEmpty()) "$baseName ($counter)" else "$baseName ($counter).$extension" + if (!taken.contains(candidate)) { + return candidate + } + counter++ } } diff --git a/utils-lib/src/test/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtilTest.kt b/utils-lib/src/test/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtilTest.kt index 57af028b3..ff1a12f4b 100644 --- a/utils-lib/src/test/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtilTest.kt +++ b/utils-lib/src/test/kotlin/ee/ria/DigiDoc/utilsLib/file/FileUtilTest.kt @@ -24,11 +24,14 @@ package ee.ria.DigiDoc.utilsLib.file import android.content.ClipData import android.content.Intent import android.net.Uri +import android.webkit.URLUtil import kotlinx.coroutines.runBlocking import org.apache.commons.io.FilenameUtils import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +import org.mockito.ArgumentMatchers.anyString import org.mockito.Mock import org.mockito.Mockito.mock import org.mockito.Mockito.mockStatic @@ -49,12 +52,13 @@ class FileUtilTest { fun fileUtil_getNameFromFileName_success() { val fileName = "test.txt" - val mockedFilenameUtils = mockStatic(FilenameUtils::class.java) - mockedFilenameUtils.`when` { FilenameUtils.getName(fileName) }.thenReturn(expectedName) + mockStatic(FilenameUtils::class.java).use { mockedFilenameUtils -> + mockedFilenameUtils.`when` { FilenameUtils.getName(fileName) }.thenReturn(expectedName) - val name = FileUtil.getNameFromFileName(fileName) + val name = FileUtil.getNameFromFileName(fileName) - assertEquals(expectedName, name) + assertEquals(expectedName, name) + } } @Test @@ -244,4 +248,194 @@ class FileUtilTest { assertEquals(0, externalFileUris.size) assertEquals(listOf(), externalFileUris) } + + @Test + fun fileUtil_sanitizeString_keepsCharactersThatFileSystemsAllow() { + mockStatic(URLUtil::class.java).use { urlUtil -> + urlUtil.`when` { URLUtil.isValidUrl(anyString()) }.thenReturn(false) + + val fileName = "pikk nimi2 !#\u00a4%&=`@\u00a3\${[]}\u00bd';,\u00a7^\u00d6\u00f6.txt" + + assertEquals(fileName, FileUtil.sanitizeString(fileName, "")) + } + } + + @Test + fun fileUtil_sanitizeString_removesCharactersFileSystemsRejectAndKeepsZeroWidthJoiner() { + mockStatic(URLUtil::class.java).use { urlUtil -> + urlUtil.`when` { URLUtil.isValidUrl(anyString()) }.thenReturn(false) + + val rightToLeftOverride = Char(0x202E) + val zeroWidthJoiner = Char(0x200D) + val fileName = "nae:wi\"th|ba?d*chars" + rightToLeftOverride + zeroWidthJoiner + ".txt" + + assertEquals("namewithbadchars" + zeroWidthJoiner + ".txt", FileUtil.sanitizeString(fileName, "")) + } + } + + @Test + fun fileUtil_sanitizeString_returnsDefaultNameWhenEverythingIsRemoved() { + mockStatic(URLUtil::class.java).use { urlUtil -> + urlUtil.`when` { URLUtil.isValidUrl(anyString()) }.thenReturn(false) + + assertEquals("newFile", FileUtil.sanitizeString("<>:|?*", "")) + } + } + + @Test + fun fileUtil_sanitizeString_shortensTooLongNameAndKeepsExtension() { + mockStatic(URLUtil::class.java).use { urlUtil -> + urlUtil.`when` { URLUtil.isValidUrl(anyString()) }.thenReturn(false) + + val fileName = "\u00f5".repeat(200) + ".txt" + + val name = FileUtil.sanitizeString(fileName, "") + + assertTrue(name.toByteArray().size <= 230) + assertTrue(name.endsWith(".txt")) + assertTrue(fileName.startsWith(name.removeSuffix(".txt"))) + } + } + + @Test + fun fileUtil_sanitizeString_keepsOnlyTheNameOfARawUrl() { + assertEquals( + "test.txt", + FileUtil.sanitizeString("raw:/storage/emulated/0/Download/test.txt", ""), + ) + } + + @Test + fun fileUtil_sanitizeString_doesNotLetARawUrlEscapeTheDirectory() { + assertEquals( + "test.txt", + FileUtil.sanitizeString("raw:/storage/emulated/0/../../test.txt", ""), + ) + } + + @Test + fun fileUtil_sanitizeString_removesSpaceLeftBehindByARemovedCharacter() { + mockStatic(URLUtil::class.java).use { urlUtil -> + urlUtil.`when` { URLUtil.isValidUrl(anyString()) }.thenReturn(false) + + assertEquals("report", FileUtil.sanitizeString("report *", "")) + } + } + + @Test + fun fileUtil_sanitizeString_leavesRoomForTheNamesTheAppBuildsFromIt() { + mockStatic(URLUtil::class.java).use { urlUtil -> + urlUtil.`when` { URLUtil.isValidUrl(anyString()) }.thenReturn(false) + + val sanitized = FileUtil.sanitizeString("\u00f5".repeat(200) + ".cdoc2", "") + val duplicate = FilenameUtils.getBaseName(sanitized) + " (99)." + FilenameUtils.getExtension(sanitized) + val dataFileDirectory = "$duplicate-data-files9" + + assertTrue(sanitized.toByteArray().size <= 230) + assertTrue(duplicate.toByteArray().size <= 255) + assertTrue( + "directory name is ${dataFileDirectory.toByteArray().size} bytes", + dataFileDirectory.toByteArray().size <= 255, + ) + } + } + + @Test + fun fileUtil_truncateFileName_returnsSameNameWhenItFits() { + assertEquals("test.txt", FileUtil.truncateFileName("test.txt", 240)) + } + + @Test + fun fileUtil_truncateFileName_cutsBetweenCharactersNotBytes() { + val fileName = "\u03b1".repeat(20) + ".txt" + + val name = FileUtil.truncateFileName(fileName, 20) + + assertEquals("\u03b1\u03b1\u03b1\u03b1\u03b1\u03b1\u03b1\u03b1.txt", name) + assertEquals(20, name.toByteArray().size) + } + + @Test + fun fileUtil_truncateFileName_dropsExtensionThatDoesNotFit() { + val name = FileUtil.truncateFileName("name.extensionthatistoolong", 10) + + assertEquals("name.exten", name) + } + + @Test + fun fileUtil_truncateFileName_dropsExtensionThatWouldLeaveNoName() { + val name = FileUtil.truncateFileName("name." + "test".repeat(30), 20) + + assertTrue(name.isNotEmpty()) + assertTrue(!name.startsWith(".")) + assertEquals(20, name.toByteArray().size) + } + + @Test + fun fileUtil_truncateFileName_keepsStartOfNameWhenOnlyExtensionWouldFit() { + val name = FileUtil.truncateFileName("\u00e4\u00e4\u00e4." + "test".repeat(17), 20) + + assertTrue(name.isNotEmpty()) + assertTrue(!name.startsWith(".")) + assertTrue(name.toByteArray().size <= 20) + } + + @Test + fun fileUtil_truncateFileName_staysWithinBudgetSmallerThanTheExtension() { + val name = FileUtil.truncateFileName("\u00e4.\u0424\u0430\u0439\u043b\u0420\u0430\u0441\u0448", 5) + + assertTrue(name.toByteArray().size <= 5) + } + + @Test + fun fileUtil_truncateFileName_fallsBackToDefaultNameWhenNoCharacterFits() { + val family = "\uD83D\uDC68\u200D\uD83D\uDC69\u200D\uD83D\uDC67" + + val name = FileUtil.truncateFileName("$family.txt", 12) + + assertEquals("newFile.txt", name) + assertTrue(name.toByteArray().size <= 12) + } + + @Test + fun fileUtil_truncateFileName_doesNotSplitCharacterMadeOfSeveralCodeUnits() { + val emoji = "\uD83D\uDE00" + + val name = FileUtil.truncateFileName(emoji + emoji + ".txt", 9) + + assertEquals(emoji + ".txt", name) + } + + @Test + fun fileUtil_truncateFileName_doesNotSplitAccentFromItsLetter() { + val letterWithAccent = "e\u0301" + + val name = FileUtil.truncateFileName(letterWithAccent + letterWithAccent + ".txt", 8) + + assertEquals(letterWithAccent + ".txt", name) + } + + @Test + fun fileUtil_uniqueFileName_returnsSameNameWhenNotTaken() { + assertEquals("test.txt", FileUtil.uniqueFileName("test.txt", setOf("other.txt"))) + } + + @Test + fun fileUtil_uniqueFileName_addsCounterToTakenName() { + assertEquals("test (1).txt", FileUtil.uniqueFileName("test.txt", setOf("test.txt"))) + assertEquals( + "test (2).txt", + FileUtil.uniqueFileName("test.txt", setOf("test.txt", "test (1).txt")), + ) + } + + @Test + fun fileUtil_uniqueFileName_usesDefaultNameWhenThereIsNoBaseName() { + assertEquals("newFile (1).txt", FileUtil.uniqueFileName(".txt", setOf(".txt"))) + } + + @Test + fun fileUtil_uniqueFileName_addsCounterToNameWithoutExtension() { + assertEquals("test (1)", FileUtil.uniqueFileName("test", setOf("test"))) + } }