From 29c7ea11bd799ab1021131c88048bbcb654e5de3 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 30 Mar 2022 15:38:40 +0200 Subject: [PATCH] Create extension function `Context.safeOpenOutputStream` --- .../main/java/im/vector/app/core/extensions/Context.kt | 9 +++++++++ .../im/vector/app/features/crypto/keys/KeysExporter.kt | 3 ++- .../keysbackup/setup/KeysBackupSetupStep3Fragment.kt | 3 ++- .../crypto/recover/BootstrapSaveRecoveryKeyFragment.kt | 3 ++- .../features/settings/devtools/KeyRequestsFragment.kt | 3 ++- .../vector/app/features/crypto/keys/KeysExporterTest.kt | 8 ++++---- .../test/java/im/vector/app/test/fakes/FakeContext.kt | 8 ++++---- 7 files changed, 25 insertions(+), 12 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/extensions/Context.kt b/vector/src/main/java/im/vector/app/core/extensions/Context.kt index b1e24c9502..0f785e43a3 100644 --- a/vector/src/main/java/im/vector/app/core/extensions/Context.kt +++ b/vector/src/main/java/im/vector/app/core/extensions/Context.kt @@ -18,6 +18,7 @@ package im.vector.app.core.extensions import android.content.Context import android.graphics.drawable.Drawable +import android.net.Uri import android.text.Spannable import android.text.SpannableString import android.text.style.ImageSpan @@ -31,6 +32,7 @@ import androidx.datastore.preferences.core.Preferences import dagger.hilt.EntryPoints import im.vector.app.core.datastore.dataStoreProvider import im.vector.app.core.di.SingletonEntryPoint +import java.io.OutputStream import kotlin.math.roundToInt fun Context.singletonEntryPoint(): SingletonEntryPoint { @@ -68,3 +70,10 @@ private fun Float.toAndroidAlpha(): Int { } val Context.dataStoreProvider: (String) -> DataStore by dataStoreProvider() + +/** + * Open Uri in truncate mode to make sure we don't partially overwrite content when we get passed a Uri to an existing file. + */ +fun Context.safeOpenOutputStream(uri: Uri): OutputStream? { + return contentResolver.openOutputStream(uri, "wt") +} 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 27d8d4842a..f40f126d2c 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 @@ -19,6 +19,7 @@ package im.vector.app.features.crypto.keys import android.content.Context import android.net.Uri import im.vector.app.core.dispatchers.CoroutineDispatchers +import im.vector.app.core.extensions.safeOpenOutputStream import kotlinx.coroutines.withContext import org.matrix.android.sdk.api.session.Session import javax.inject.Inject @@ -34,7 +35,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, "wt") + context.safeOpenOutputStream(uri) ?.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 42ff4ac183..e5d7ade3ce 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 @@ -30,6 +30,7 @@ import com.google.android.material.bottomsheet.BottomSheetDialog import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R import im.vector.app.core.extensions.registerStartForActivityResult +import im.vector.app.core.extensions.safeOpenOutputStream import im.vector.app.core.platform.VectorBaseFragment import im.vector.app.core.utils.LiveEvent import im.vector.app.core.utils.copyToClipboard @@ -165,7 +166,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 746ed48c94..efabae2f3a 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 @@ -29,6 +29,7 @@ import com.airbnb.mvrx.parentFragmentViewModel import com.airbnb.mvrx.withState import im.vector.app.R import im.vector.app.core.extensions.registerStartForActivityResult +import im.vector.app.core.extensions.safeOpenOutputStream import im.vector.app.core.platform.VectorBaseFragment import im.vector.app.core.resources.ColorProvider import im.vector.app.core.utils.startSharePlainTextIntent @@ -81,7 +82,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, "wt")!!)) + sharedViewModel.handle(BootstrapActions.SaveKeyToUri(requireContext().safeOpenOutputStream(uri)!!)) } 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 6748fec1bc..cef68c01c1 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 @@ -34,6 +34,7 @@ import com.airbnb.mvrx.withState import com.google.android.material.tabs.TabLayoutMediator import im.vector.app.R import im.vector.app.core.extensions.registerStartForActivityResult +import im.vector.app.core.extensions.safeOpenOutputStream import im.vector.app.core.platform.VectorBaseFragment import im.vector.app.core.utils.selectTxtFileToWrite import im.vector.app.databinding.FragmentDevtoolKeyrequestsBinding @@ -106,7 +107,7 @@ class KeyRequestsFragment @Inject constructor() : VectorBaseFragment { tryOrNull { - requireContext().contentResolver?.openOutputStream(it.uri, "wt") + requireContext().safeOpenOutputStream(it.uri) ?.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 3bec3fad88..3cd797a7b1 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, mode = "wt") + val outputStream = context.givenSafeOutputStreamFor(A_URI) 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, mode = "wt") + context.givenSafeOutputStreamFor(A_URI) 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, mode = "wt") + context.givenMissingSafeOutputStreamFor(A_URI) 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, mode = "wt") + context.givenSafeOutputStreamFor(A_URI) 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 de1a7956b8..2a50c34ca3 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, mode: String): OutputStream { + fun givenSafeOutputStreamFor(uri: Uri): OutputStream { val outputStream = mockk(relaxed = true) - every { contentResolver.openOutputStream(uri, mode) } returns outputStream + every { contentResolver.openOutputStream(uri, "wt") } returns outputStream return outputStream } - fun givenMissingOutputStreamFor(uri: Uri, mode: String) { - every { contentResolver.openOutputStream(uri, mode) } returns null + fun givenMissingSafeOutputStreamFor(uri: Uri) { + every { contentResolver.openOutputStream(uri, "wt") } returns null } }