From b9b5cab7722a8b648b80b62fffc19cf27b92c375 Mon Sep 17 00:00:00 2001 From: cketti Date: Tue, 29 Mar 2022 16:12:12 +0200 Subject: [PATCH] Use truncate mode to replace the contents of existing files `ContentResolver.openOutputStream(Uri)` does not truncate existing files. If the amount of data written is smaller than the file size, you end up with new data at the beginning of the file followed by old data at the end of the file. --- changelog.d/5663.bugfix | 1 + .../im/vector/app/features/crypto/keys/KeysExporter.kt | 2 +- .../keysbackup/setup/KeysBackupSetupStep3Fragment.kt | 2 +- .../crypto/recover/BootstrapSaveRecoveryKeyFragment.kt | 2 +- .../app/features/settings/devtools/KeyRequestsFragment.kt | 2 +- .../vector/app/features/crypto/keys/KeysExporterTest.kt | 8 ++++---- .../src/test/java/im/vector/app/test/fakes/FakeContext.kt | 8 ++++---- 7 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 changelog.d/5663.bugfix diff --git a/changelog.d/5663.bugfix b/changelog.d/5663.bugfix new file mode 100644 index 0000000000..5086407e8e --- /dev/null +++ b/changelog.d/5663.bugfix @@ -0,0 +1 @@ +Fixed key export when overwriting existing files diff --git a/vector/src/main/java/im/vector/app/features/crypto/keys/KeysExporter.kt b/vector/src/main/java/im/vector/app/features/crypto/keys/KeysExporter.kt index 3db67df8e1..27d8d4842a 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/keys/KeysExporter.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/keys/KeysExporter.kt @@ -34,7 +34,7 @@ class KeysExporter @Inject constructor( suspend fun export(password: String, uri: Uri) { withContext(dispatchers.io) { val data = session.cryptoService().exportRoomKeys(password) - context.contentResolver.openOutputStream(uri) + context.contentResolver.openOutputStream(uri, "wt") ?.use { it.write(data) } ?: throw IllegalStateException("Unable to open file for writing") verifyExportedKeysOutputFileSize(uri, expectedSize = data.size.toLong()) diff --git a/vector/src/main/java/im/vector/app/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt b/vector/src/main/java/im/vector/app/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt index c1cd87b4c8..42ff4ac183 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt @@ -165,7 +165,7 @@ class KeysBackupSetupStep3Fragment @Inject constructor() : VectorBaseFragment os.write(data.toByteArray()) os.flush() diff --git a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSaveRecoveryKeyFragment.kt b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSaveRecoveryKeyFragment.kt index 8a41c7ce4d..746ed48c94 100644 --- a/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSaveRecoveryKeyFragment.kt +++ b/vector/src/main/java/im/vector/app/features/crypto/recover/BootstrapSaveRecoveryKeyFragment.kt @@ -81,7 +81,7 @@ class BootstrapSaveRecoveryKeyFragment @Inject constructor( val uri = activityResult.data?.data ?: return@registerStartForActivityResult lifecycleScope.launch(Dispatchers.IO) { try { - sharedViewModel.handle(BootstrapActions.SaveKeyToUri(requireContext().contentResolver!!.openOutputStream(uri)!!)) + sharedViewModel.handle(BootstrapActions.SaveKeyToUri(requireContext().contentResolver!!.openOutputStream(uri, "wt")!!)) } catch (failure: Throwable) { sharedViewModel.handle(BootstrapActions.SaveReqFailed) } diff --git a/vector/src/main/java/im/vector/app/features/settings/devtools/KeyRequestsFragment.kt b/vector/src/main/java/im/vector/app/features/settings/devtools/KeyRequestsFragment.kt index db2d07feef..6748fec1bc 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devtools/KeyRequestsFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devtools/KeyRequestsFragment.kt @@ -106,7 +106,7 @@ class KeyRequestsFragment @Inject constructor() : VectorBaseFragment { tryOrNull { - requireContext().contentResolver?.openOutputStream(it.uri) + requireContext().contentResolver?.openOutputStream(it.uri, "wt") ?.use { os -> os.write(it.raw.toByteArray()) } } } diff --git a/vector/src/test/java/im/vector/app/features/crypto/keys/KeysExporterTest.kt b/vector/src/test/java/im/vector/app/features/crypto/keys/KeysExporterTest.kt index 5d0317592d..3bec3fad88 100644 --- a/vector/src/test/java/im/vector/app/features/crypto/keys/KeysExporterTest.kt +++ b/vector/src/test/java/im/vector/app/features/crypto/keys/KeysExporterTest.kt @@ -53,7 +53,7 @@ class KeysExporterTest { @Test fun `when exporting then writes exported keys to context output stream`() { givenFileDescriptorWithSize(size = A_ROOM_KEYS_EXPORT.size.toLong()) - val outputStream = context.givenOutputStreamFor(A_URI) + val outputStream = context.givenOutputStreamFor(A_URI, mode = "wt") runTest { keysExporter.export(A_PASSWORD, A_URI) } @@ -63,7 +63,7 @@ class KeysExporterTest { @Test fun `given different file size returned for export when exporting then throws UnexpectedExportKeysFileSizeException`() { givenFileDescriptorWithSize(size = 110) - context.givenOutputStreamFor(A_URI) + context.givenOutputStreamFor(A_URI, mode = "wt") assertFailsWith { runTest { keysExporter.export(A_PASSWORD, A_URI) } @@ -72,7 +72,7 @@ class KeysExporterTest { @Test fun `given output stream is unavailable for exporting to when exporting then throws IllegalStateException`() { - context.givenMissingOutputStreamFor(A_URI) + context.givenMissingOutputStreamFor(A_URI, mode = "wt") assertFailsWith(message = "Unable to open file for writing") { runTest { keysExporter.export(A_PASSWORD, A_URI) } @@ -82,7 +82,7 @@ class KeysExporterTest { @Test fun `given exported file is missing after export when exporting then throws IllegalStateException`() { context.givenFileDescriptor(A_URI, mode = "r") { null } - context.givenOutputStreamFor(A_URI) + context.givenOutputStreamFor(A_URI, mode = "wt") assertFailsWith(message = "Exported file not found") { runTest { keysExporter.export(A_PASSWORD, A_URI) } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt index 8382e54253..de1a7956b8 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeContext.kt @@ -39,13 +39,13 @@ class FakeContext( every { contentResolver.openFileDescriptor(uri, mode, null) } returns fileDescriptor } - fun givenOutputStreamFor(uri: Uri): OutputStream { + fun givenOutputStreamFor(uri: Uri, mode: String): OutputStream { val outputStream = mockk(relaxed = true) - every { contentResolver.openOutputStream(uri) } returns outputStream + every { contentResolver.openOutputStream(uri, mode) } returns outputStream return outputStream } - fun givenMissingOutputStreamFor(uri: Uri) { - every { contentResolver.openOutputStream(uri) } returns null + fun givenMissingOutputStreamFor(uri: Uri, mode: String) { + every { contentResolver.openOutputStream(uri, mode) } returns null } }