From 8cedd8ed33954de3a33b3d7411b1f3272689e605 Mon Sep 17 00:00:00 2001 From: David Perez Date: Wed, 6 Mar 2024 14:13:05 -0600 Subject: [PATCH] Catch exception caused by trying to process large files on devices with low memory (#1101) --- .../data/vault/manager/FileManager.kt | 4 +- .../data/vault/manager/FileManagerImpl.kt | 31 ++++---- .../vault/repository/VaultRepositoryImpl.kt | 32 +++++--- .../vault/repository/VaultRepositoryTest.kt | 76 +++++++++++++++++-- 4 files changed, 106 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManager.kt index e2a2d8f68..aef509589 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManager.kt @@ -35,7 +35,7 @@ interface FileManager { suspend fun stringToUri(fileUri: Uri, dataString: String): Boolean /** - * Reads the [fileUri] into memory and returns the raw [ByteArray] + * Reads the [fileUri] into memory. A successful result will contain the raw [ByteArray]. */ - suspend fun uriToByteArray(fileUri: Uri): ByteArray + suspend fun uriToByteArray(fileUri: Uri): Result } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManagerImpl.kt index 4d0d417d6..f59eb3d0f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/manager/FileManagerImpl.kt @@ -7,7 +7,6 @@ import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.vault.datasource.network.service.DownloadService import com.x8bit.bitwarden.data.vault.manager.model.DownloadResult import kotlinx.coroutines.withContext -import okio.use import java.io.ByteArrayOutputStream import java.io.File import java.io.FileInputStream @@ -113,21 +112,23 @@ class FileManagerImpl( } } - override suspend fun uriToByteArray(fileUri: Uri): ByteArray = - withContext(dispatcherManager.io) { - context - .contentResolver - .openInputStream(fileUri) - ?.use { inputStream -> - ByteArrayOutputStream().use { outputStream -> - val buffer = ByteArray(BUFFER_SIZE) - var length: Int - while (inputStream.read(buffer).also { length = it } != -1) { - outputStream.write(buffer, 0, length) + override suspend fun uriToByteArray(fileUri: Uri): Result = + runCatching { + withContext(dispatcherManager.io) { + context + .contentResolver + .openInputStream(fileUri) + ?.use { inputStream -> + ByteArrayOutputStream().use { outputStream -> + val buffer = ByteArray(BUFFER_SIZE) + var length: Int + while (inputStream.read(buffer).also { length = it } != -1) { + outputStream.write(buffer, 0, length) + } + outputStream.toByteArray() } - outputStream.toByteArray() } - } - ?: byteArrayOf() + ?: throw IllegalStateException("Stream has crashed") + } } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index 459f5c26d..2488d9153 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -868,12 +868,16 @@ class VaultRepositoryImpl( cipherView = cipherView, ) .flatMap { cipher -> - vaultSdkSource.encryptAttachment( - userId = userId, - cipher = cipher, - attachmentView = attachmentView, - fileBuffer = fileManager.uriToByteArray(fileUri = fileUri), - ) + fileManager + .uriToByteArray(fileUri = fileUri) + .flatMap { + vaultSdkSource.encryptAttachment( + userId = userId, + cipher = cipher, + attachmentView = attachmentView, + fileBuffer = it, + ) + } } .flatMap { attachmentEncryptResult -> ciphersService @@ -990,12 +994,16 @@ class VaultRepositoryImpl( "File URI must be present to create a File Send.", ) .asFailure() - vaultSdkSource - .encryptBuffer( - userId = userId, - send = send, - fileBuffer = fileManager.uriToByteArray(fileUri = uri), - ) + + fileManager + .uriToByteArray(fileUri = uri) + .flatMap { + vaultSdkSource.encryptBuffer( + userId = userId, + send = send, + fileBuffer = it, + ) + } .flatMap { encryptedFile -> sendsService .createFileSend( diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt index 199312c18..bb6af4002 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt @@ -2514,7 +2514,7 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptSend(userId = userId, sendView = mockSendView) } returns mockSdkSend.asSuccess() - coEvery { fileManager.uriToByteArray(any()) } returns byteArray + coEvery { fileManager.uriToByteArray(any()) } returns byteArray.asSuccess() coEvery { vaultSdkSource.encryptBuffer( userId = userId, @@ -2551,7 +2551,7 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptSend(userId = userId, sendView = mockSendView) } returns mockSdkSend.asSuccess() - coEvery { fileManager.uriToByteArray(any()) } returns byteArray + coEvery { fileManager.uriToByteArray(any()) } returns byteArray.asSuccess() coEvery { vaultSdkSource.encryptBuffer( userId = userId, @@ -2574,6 +2574,26 @@ class VaultRepositoryTest { assertEquals(CreateSendResult.Error, result) } + @Test + @Suppress("MaxLineLength") + fun `createSend with FILE and fileManager uriToByteArray failure should return CreateSendResult Error`() = + runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + val userId = "mockId-1" + val url = "www.test.com" + val uri = setupMockUri(url = url) + val mockSendView = createMockSendView(number = 1) + val mockSdkSend = createMockSdkSend(number = 1) + coEvery { + vaultSdkSource.encryptSend(userId = userId, sendView = mockSendView) + } returns mockSdkSend.asSuccess() + coEvery { fileManager.uriToByteArray(any()) } returns Throwable("Fail").asFailure() + + val result = vaultRepository.createSend(sendView = mockSendView, fileUri = uri) + + assertEquals(CreateSendResult.Error, result) + } + @Test @Suppress("MaxLineLength") fun `createSend with FILE and sendsService uploadFile success should return CreateSendResult success`() = @@ -2596,7 +2616,7 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptSend(userId = userId, sendView = mockSendView) } returns mockSdkSend.asSuccess() - coEvery { fileManager.uriToByteArray(any()) } returns byteArray + coEvery { fileManager.uriToByteArray(any()) } returns byteArray.asSuccess() coEvery { vaultSdkSource.encryptBuffer( userId = userId, @@ -3225,7 +3245,9 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptCipher(userId = userId, cipherView = mockCipherView) } returns mockCipher.asSuccess() - coEvery { fileManager.uriToByteArray(fileUri = mockUri) } returns mockByteArray + coEvery { + fileManager.uriToByteArray(fileUri = mockUri) + } returns mockByteArray.asSuccess() coEvery { vaultSdkSource.encryptAttachment( userId = userId, @@ -3246,6 +3268,36 @@ class VaultRepositoryTest { assertEquals(CreateAttachmentResult.Error, result) } + @Suppress("MaxLineLength") + @Test + fun `createAttachment with uriToByteArray failure should return CreateAttachmentResult Error`() = + runTest { + fakeAuthDiskSource.userState = MOCK_USER_STATE + val userId = "mockId-1" + val cipherId = "cipherId-1" + val mockUri = setupMockUri(url = "www.test.com") + val mockCipherView = createMockCipherView(number = 1) + val mockCipher = createMockSdkCipher(number = 1) + val mockFileName = "mockFileName-1" + val mockFileSize = "1" + coEvery { + vaultSdkSource.encryptCipher(userId = userId, cipherView = mockCipherView) + } returns mockCipher.asSuccess() + coEvery { + fileManager.uriToByteArray(fileUri = mockUri) + } returns Throwable("Fail").asFailure() + + val result = vaultRepository.createAttachment( + cipherId = cipherId, + cipherView = mockCipherView, + fileSizeBytes = mockFileSize, + fileName = mockFileName, + fileUri = mockUri, + ) + + assertEquals(CreateAttachmentResult.Error, result) + } + @Suppress("MaxLineLength") @Test fun `createAttachment with createAttachment failure should return CreateAttachmentResult Error`() = @@ -3269,7 +3321,9 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptCipher(userId = userId, cipherView = mockCipherView) } returns mockCipher.asSuccess() - coEvery { fileManager.uriToByteArray(fileUri = mockUri) } returns mockByteArray + coEvery { + fileManager.uriToByteArray(fileUri = mockUri) + } returns mockByteArray.asSuccess() coEvery { vaultSdkSource.encryptAttachment( userId = userId, @@ -3324,7 +3378,9 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptCipher(userId = userId, cipherView = mockCipherView) } returns mockCipher.asSuccess() - coEvery { fileManager.uriToByteArray(fileUri = mockUri) } returns mockByteArray + coEvery { + fileManager.uriToByteArray(fileUri = mockUri) + } returns mockByteArray.asSuccess() coEvery { vaultSdkSource.encryptAttachment( userId = userId, @@ -3389,7 +3445,9 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptCipher(userId = userId, cipherView = mockCipherView) } returns mockCipher.asSuccess() - coEvery { fileManager.uriToByteArray(fileUri = mockUri) } returns mockByteArray + coEvery { + fileManager.uriToByteArray(fileUri = mockUri) + } returns mockByteArray.asSuccess() coEvery { vaultSdkSource.encryptAttachment( userId = userId, @@ -3463,7 +3521,9 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptCipher(userId = userId, cipherView = mockCipherView) } returns mockCipher.asSuccess() - coEvery { fileManager.uriToByteArray(fileUri = mockUri) } returns mockByteArray + coEvery { + fileManager.uriToByteArray(fileUri = mockUri) + } returns mockByteArray.asSuccess() coEvery { vaultSdkSource.encryptAttachment( userId = userId,