Merge pull request #5663 from cketti/fix_openOutputStream

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

@@ -0,0 +1 @@
+Fixed key export when overwriting existing files

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<Preferences> 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")
+}

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)
+            context.safeOpenOutputStream(uri)
                     ?.use { it.write(data) }
                     ?: throw IllegalStateException("Unable to open file for writing")
             verifyExportedKeysOutputFileSize(uri, expectedSize = data.size.toLong())

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<Fr
         lifecycleScope.launch(Dispatchers.Main) {
             Try {
                 withContext(Dispatchers.IO) {
-                    requireContext().contentResolver.openOutputStream(uri)
+                    requireContext().safeOpenOutputStream(uri)
                             ?.use { os ->
                                 os.write(data.toByteArray())
                                 os.flush()

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)!!))
+                    sharedViewModel.handle(BootstrapActions.SaveKeyToUri(requireContext().safeOpenOutputStream(uri)!!))
                 } catch (failure: Throwable) {
                     sharedViewModel.handle(BootstrapActions.SaveReqFailed)
                 }

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<FragmentDev
             when (it) {
                 is KeyRequestEvents.SaveAudit -> {
                     tryOrNull {
-                        requireContext().contentResolver?.openOutputStream(it.uri)
+                        requireContext().safeOpenOutputStream(it.uri)
                                 ?.use { os -> os.write(it.raw.toByteArray()) }
                     }
                 }

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.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)
+        context.givenSafeOutputStreamFor(A_URI)
 
         assertFailsWith<UnexpectedExportKeysFileSizeException> {
             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.givenMissingSafeOutputStreamFor(A_URI)
 
         assertFailsWith<IllegalStateException>(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.givenSafeOutputStreamFor(A_URI)
 
         assertFailsWith<IllegalStateException>(message = "Exported file not found") {
             runTest { keysExporter.export(A_PASSWORD, A_URI) }

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 givenSafeOutputStreamFor(uri: Uri): OutputStream {
         val outputStream = mockk<OutputStream>(relaxed = true)
-        every { contentResolver.openOutputStream(uri) } returns outputStream
+        every { contentResolver.openOutputStream(uri, "wt") } returns outputStream
         return outputStream
     }
 
-    fun givenMissingOutputStreamFor(uri: Uri) {
+    fun givenMissingSafeOutputStreamFor(uri: Uri) {
-        every { contentResolver.openOutputStream(uri) } returns null
+        every { contentResolver.openOutputStream(uri, "wt") } returns null
     }
 }