From 32af8a186067bb06bba8f55ba15e9bb347720a95 Mon Sep 17 00:00:00 2001 From: Patrick Honkonen <1883101+SaintPatrck@users.noreply.github.com> Date: Mon, 22 Apr 2024 15:28:14 -0400 Subject: [PATCH] BIT-1624 Fix OutOfMemoryException when uploading large files (#1293) --- .../platform/util/InputStreamExtensions.kt | 48 +++++++++++++ .../network/model/SendJsonRequest.kt | 2 +- .../network/service/SendsService.kt | 3 +- .../network/service/SendsServiceImpl.kt | 9 +-- .../vault/datasource/sdk/VaultSdkSource.kt | 15 ++++ .../datasource/sdk/VaultSdkSourceImpl.kt | 19 +++++ .../data/vault/manager/FileManager.kt | 11 +++ .../data/vault/manager/FileManagerImpl.kt | 23 +++++++ .../vault/repository/VaultRepositoryImpl.kt | 15 ++-- .../repository/util/VaultSdkSendExtensions.kt | 2 +- .../network/service/SendsServiceTest.kt | 5 +- .../vault/repository/VaultRepositoryTest.kt | 69 +++++++++++++------ 12 files changed, 186 insertions(+), 35 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/util/InputStreamExtensions.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/util/InputStreamExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/InputStreamExtensions.kt new file mode 100644 index 0000000000..9cb053763c --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/InputStreamExtensions.kt @@ -0,0 +1,48 @@ +package com.x8bit.bitwarden.data.platform.util + +import android.os.Build +import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage +import java.io.IOException +import java.io.InputStream +import java.io.OutputStream + +private const val DEFAULT_BUFFER_SIZE = 8192 + +/** + * Reads all bytes from this input stream and writes the bytes to the + * given output stream in the order that they are read. On return, this + * input stream will be at end of stream. This method does not close either + * stream. + * + * This method may block indefinitely reading from the input stream, or + * writing to the output stream. The behavior for the case where the input + * and/or output stream is asynchronously closed, or the thread + * interrupted during the transfer, is highly input and output stream + * specific, and therefore not specified. + * + * If an I/O error occurs reading from the input stream or writing to the + * output stream, then it may do so after some bytes have been read or + * written. Consequently the input stream may not be at end of stream and + * one, or both, streams may be in an inconsistent state. It is strongly + * recommended that both streams be promptly closed if an I/O error occurs. + * + * @param outputStream the output stream, non-null + * @return the number of bytes transferred + * @throws IOException if an I/O error occurs when reading or writing + * @throws NullPointerException if {@code out} is {@code null} + */ +@OmitFromCoverage +fun InputStream.sdkAgnosticTransferTo(outputStream: OutputStream): Long { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + transferTo(outputStream) + } else { + var transferred: Long = 0 + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + var read: Int + while (this.read(buffer, 0, DEFAULT_BUFFER_SIZE).also { read = it } >= 0) { + outputStream.write(buffer, 0, read) + transferred += read.toLong() + } + transferred + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/SendJsonRequest.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/SendJsonRequest.kt index 9d37a0cc28..e041cae77a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/SendJsonRequest.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/SendJsonRequest.kt @@ -48,7 +48,7 @@ data class SendJsonRequest( val deletionDate: ZonedDateTime, @SerialName("fileLength") - val fileLength: Int?, + val fileLength: Long?, @SerialName("file") val file: SyncResponseJson.Send.File?, diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsService.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsService.kt index 9902173085..6cce0b1e61 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsService.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsService.kt @@ -6,6 +6,7 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateSendJsonRes import com.x8bit.bitwarden.data.vault.datasource.network.model.SendJsonRequest import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateSendResponseJson +import java.io.File /** * Provides an API for querying sends endpoints. @@ -30,7 +31,7 @@ interface SendsService { */ suspend fun uploadFile( sendFileResponse: CreateFileSendResponseJson, - encryptedFile: ByteArray, + encryptedFile: File, ): Result /** diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceImpl.kt index 5e6091b15e..b62b8d35c3 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceImpl.kt @@ -15,7 +15,8 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateSendRespons import kotlinx.serialization.json.Json import okhttp3.MediaType.Companion.toMediaType import okhttp3.MultipartBody -import okhttp3.RequestBody.Companion.toRequestBody +import okhttp3.RequestBody.Companion.asRequestBody +import java.io.File import java.time.Clock import java.time.ZoneOffset import java.time.ZonedDateTime @@ -80,7 +81,7 @@ class SendsServiceImpl( override suspend fun uploadFile( sendFileResponse: CreateFileSendResponseJson, - encryptedFile: ByteArray, + encryptedFile: File, ): Result { val send = sendFileResponse.sendResponse return when (sendFileResponse.fileUploadType) { @@ -94,7 +95,7 @@ class SendsServiceImpl( ) .addPart( part = MultipartBody.Part.createFormData( - body = encryptedFile.toRequestBody( + body = encryptedFile.asRequestBody( contentType = "application/octet-stream".toMediaType(), ), name = "data", @@ -112,7 +113,7 @@ class SendsServiceImpl( .RFC_1123_DATE_TIME .format(ZonedDateTime.ofInstant(clock.instant(), ZoneOffset.UTC)), version = sendFileResponse.url.toUri().getQueryParameter("sv"), - body = encryptedFile.toRequestBody(), + body = encryptedFile.asRequestBody(), ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt index 39bdb79e06..518994bf29 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSource.kt @@ -24,6 +24,7 @@ import com.bitwarden.core.TotpResponse import com.bitwarden.core.UpdatePasswordResponse import com.bitwarden.crypto.TrustDeviceResponse import com.x8bit.bitwarden.data.vault.datasource.sdk.model.InitializeCryptoResult +import java.io.File /** * Source of vault information and functionality from the Bitwarden SDK. @@ -236,6 +237,20 @@ interface VaultSdkSource { fileBuffer: ByteArray, ): Result + /** + * Encrypts a file at [path] for the user with the given [userId], returning the + * encrypted [File] as a [Result]. + * + * This should only be called after a successful call to [initializeCrypto] for the associated + * user. + */ + suspend fun encryptFile( + userId: String, + send: Send, + path: String, + destinationFilePath: String, + ): Result + /** * Decrypts a [Send] for the user with the given [userId], returning a [SendView] wrapped in a * [Result]. diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt index f73e1f6b82..3836a94917 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/VaultSdkSourceImpl.kt @@ -27,6 +27,7 @@ import com.bitwarden.sdk.Client import com.bitwarden.sdk.ClientVault import com.x8bit.bitwarden.data.platform.manager.SdkClientManager import com.x8bit.bitwarden.data.vault.datasource.sdk.model.InitializeCryptoResult +import java.io.File /** * Primary implementation of [VaultSdkSource] that serves as a convenience wrapper around a @@ -163,6 +164,24 @@ class VaultSdkSourceImpl( ) } + override suspend fun encryptFile( + userId: String, + send: Send, + path: String, + destinationFilePath: String, + ): Result = + runCatching { + getClient(userId = userId) + .vault() + .sends() + .encryptFile( + send = send, + decryptedFilePath = path, + encryptedFilePath = destinationFilePath, + ) + File(destinationFilePath) + } + override suspend fun encryptAttachment( userId: String, cipher: Cipher, 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 aef5095899..07dcb53515 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 @@ -11,6 +11,11 @@ import java.io.File @OmitFromCoverage interface FileManager { + /** + * Absolute path to the private file storage directory. + */ + val filesDirectory: String + /** * Deletes a [file] from the system. */ @@ -38,4 +43,10 @@ interface FileManager { * Reads the [fileUri] into memory. A successful result will contain the raw [ByteArray]. */ suspend fun uriToByteArray(fileUri: Uri): Result + + /** + * Reads the [fileUri] into a file on disk. A successful result will contain the [File] + * reference. + */ + suspend fun writeUriToCache(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 f59eb3d0ff..56faf1771e 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 @@ -4,6 +4,7 @@ import android.content.Context import android.net.Uri import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager +import com.x8bit.bitwarden.data.platform.util.sdkAgnosticTransferTo import com.x8bit.bitwarden.data.vault.datasource.network.service.DownloadService import com.x8bit.bitwarden.data.vault.manager.model.DownloadResult import kotlinx.coroutines.withContext @@ -28,6 +29,9 @@ class FileManagerImpl( private val dispatcherManager: DispatcherManager, ) : FileManager { + override val filesDirectory: String + get() = context.filesDir.absolutePath + override suspend fun deleteFile(file: File) { withContext(dispatcherManager.io) { file.delete() @@ -131,4 +135,23 @@ class FileManagerImpl( ?: throw IllegalStateException("Stream has crashed") } } + + override suspend fun writeUriToCache(fileUri: Uri): Result = + runCatching { + withContext(dispatcherManager.io) { + val tempFileName = "temp_send_file.bw" + context + .contentResolver + .openInputStream(fileUri) + ?.use { inputStream -> + context.openFileOutput(tempFileName, Context.MODE_PRIVATE) + .use { outputStream -> + inputStream.sdkAgnosticTransferTo(outputStream) + } + } + ?: throw IllegalStateException("Stream has crashed") + + File(context.filesDir, tempFileName) + } + } } 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 360a8e89bd..800010d3a4 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 @@ -1589,19 +1589,20 @@ class VaultRepositoryImpl( .asFailure() return fileManager - .uriToByteArray(fileUri = uri) - .flatMap { - vaultSdkSource.encryptBuffer( + .writeUriToCache(uri) + .flatMap { file -> + vaultSdkSource.encryptFile( userId = userId, send = send, - fileBuffer = it, + path = file.absolutePath, + destinationFilePath = file.absolutePath, ) } .flatMap { encryptedFile -> sendsService .createFileSend( body = send.toEncryptedNetworkSend( - fileLength = encryptedFile.size, + fileLength = encryptedFile.length(), ), ) .flatMap { sendFileResponse -> @@ -1621,6 +1622,10 @@ class VaultRepositoryImpl( sendFileResponse = sendFileResponse.createFileJsonResponse, encryptedFile = encryptedFile, ) + .also { + // Delete encrypted file once it has been uploaded. + fileManager.deleteFile(encryptedFile) + } .map { CreateSendJsonResponse.Success(it) } } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt index fe7ce899a4..d99982d770 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt @@ -13,7 +13,7 @@ import java.time.ZonedDateTime /** * Converts a Bitwarden SDK [Send] object to a corresponding [SyncResponseJson.Send] object. */ -fun Send.toEncryptedNetworkSend(fileLength: Int? = null): SendJsonRequest = +fun Send.toEncryptedNetworkSend(fileLength: Long? = null): SendJsonRequest = SendJsonRequest( type = type.toNetworkSendType(), name = name, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceTest.kt index 3fdd73e513..25a626130e 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceTest.kt @@ -22,6 +22,7 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import retrofit2.create +import java.io.File import java.time.Clock import java.time.Instant import java.time.ZoneOffset @@ -117,7 +118,7 @@ class SendsServiceTest : BaseServiceTest() { val url = "www.test.com" setupMockUri(url = url, queryParams = mapOf("sv" to "2024-04-03")) val sendFileResponse = createMockFileSendResponseJson(number = 1) - val encryptedFile = byteArrayOf() + val encryptedFile = File.createTempFile("mockFile", "temp") server.enqueue(MockResponse().setResponseCode(201)) @@ -134,7 +135,7 @@ class SendsServiceTest : BaseServiceTest() { val url = "www.test.com" setupMockUri(url = url, queryParams = mapOf("sv" to "2024-04-03")) val sendFileResponse = createMockFileSendResponseJson(number = 1) - val encryptedFile = byteArrayOf() + val encryptedFile = File.createTempFile("mockFile", "temp") server.enqueue(MockResponse().setResponseCode(201)) val result = sendsService.uploadFile( 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 31ea744848..3dcdaf2dd4 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 @@ -2512,19 +2512,26 @@ class VaultRepositoryTest { val uri = setupMockUri(url = "www.test.com") val mockSendView = createMockSendView(number = 1) val mockSdkSend = createMockSdkSend(number = 1) - val byteArray = byteArrayOf(1) - val encryptedByteArray = byteArrayOf(2) + val decryptedFile = mockk { + every { length() } returns 1 + every { absolutePath } returns "mockAbsolutePath" + } + val encryptedFile = mockk { + every { length() } returns 1 + every { absolutePath } returns "mockAbsolutePath" + } coEvery { vaultSdkSource.encryptSend(userId = userId, sendView = mockSendView) } returns mockSdkSend.asSuccess() - coEvery { fileManager.uriToByteArray(any()) } returns byteArray.asSuccess() + coEvery { fileManager.writeUriToCache(any()) } returns decryptedFile.asSuccess() coEvery { - vaultSdkSource.encryptBuffer( + vaultSdkSource.encryptFile( userId = userId, send = mockSdkSend, - fileBuffer = byteArray, + path = "mockAbsolutePath", + destinationFilePath = "mockAbsolutePath", ) - } returns encryptedByteArray.asSuccess() + } returns encryptedFile.asSuccess() coEvery { sendsService.createFileSend(body = createMockSendJsonRequest(number = 1)) } returns IllegalStateException().asFailure() @@ -2544,8 +2551,16 @@ class VaultRepositoryTest { val uri = setupMockUri(url = url) val mockSendView = createMockSendView(number = 1) val mockSdkSend = createMockSdkSend(number = 1) - val byteArray = byteArrayOf(1) - val encryptedByteArray = byteArrayOf(2) + val decryptedFile = mockk { + every { name } returns "mockFileName" + every { absolutePath } returns "mockAbsolutePath" + every { length() } returns 1 + } + val encryptedFile = mockk { + every { name } returns "mockFileName" + every { absolutePath } returns "mockAbsolutePath" + every { length() } returns 1 + } val sendFileResponse = CreateFileSendResponse.Success( createMockFileSendResponseJson( number = 1, @@ -2558,14 +2573,17 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.decryptSend(userId, mockSdkSend) } returns mockSendView.asSuccess() - coEvery { fileManager.uriToByteArray(any()) } returns byteArray.asSuccess() + every { fileManager.filesDirectory } returns "mockFilesDirectory" + coEvery { fileManager.writeUriToCache(any()) } returns decryptedFile.asSuccess() + coEvery { fileManager.deleteFile(any()) } returns Unit coEvery { - vaultSdkSource.encryptBuffer( + vaultSdkSource.encryptFile( userId = userId, send = mockSdkSend, - fileBuffer = byteArray, + path = "mockAbsolutePath", + destinationFilePath = "mockAbsolutePath", ) - } returns encryptedByteArray.asSuccess() + } returns encryptedFile.asSuccess() coEvery { vaultDiskSource.saveSend( userId, @@ -2578,7 +2596,7 @@ class VaultRepositoryTest { coEvery { sendsService.uploadFile( sendFileResponse = sendFileResponse.createFileJsonResponse, - encryptedFile = encryptedByteArray, + encryptedFile = encryptedFile, ) } returns Throwable().asFailure() @@ -2600,7 +2618,7 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptSend(userId = userId, sendView = mockSendView) } returns mockSdkSend.asSuccess() - coEvery { fileManager.uriToByteArray(any()) } returns Throwable().asFailure() + coEvery { fileManager.writeUriToCache(any()) } returns Throwable().asFailure() val result = vaultRepository.createSend(sendView = mockSendView, fileUri = uri) @@ -2617,8 +2635,13 @@ class VaultRepositoryTest { val uri = setupMockUri(url = url) val mockSendView = createMockSendView(number = 1) val mockSdkSend = createMockSdkSend(number = 1) - val byteArray = byteArrayOf(1) - val encryptedByteArray = byteArrayOf(2) + val decryptedFile = mockk { + every { name } returns "mockFileName" + every { absolutePath } returns "mockAbsolutePath" + } + val encryptedFile = mockk { + every { length() } returns 1 + } val sendFileResponse = CreateFileSendResponse.Success( createMockFileSendResponseJson(number = 1), ) @@ -2626,21 +2649,25 @@ class VaultRepositoryTest { coEvery { vaultSdkSource.encryptSend(userId = userId, sendView = mockSendView) } returns mockSdkSend.asSuccess() - coEvery { fileManager.uriToByteArray(any()) } returns byteArray.asSuccess() + + every { fileManager.filesDirectory } returns "mockFilesDirectory" + coEvery { fileManager.deleteFile(any()) } returns Unit + coEvery { fileManager.writeUriToCache(any()) } returns decryptedFile.asSuccess() coEvery { - vaultSdkSource.encryptBuffer( + vaultSdkSource.encryptFile( userId = userId, send = mockSdkSend, - fileBuffer = byteArray, + path = "mockAbsolutePath", + destinationFilePath = "mockAbsolutePath", ) - } returns encryptedByteArray.asSuccess() + } returns encryptedFile.asSuccess() coEvery { sendsService.createFileSend(body = createMockSendJsonRequest(number = 1)) } returns sendFileResponse.asSuccess() coEvery { sendsService.uploadFile( sendFileResponse = sendFileResponse.createFileJsonResponse, - encryptedFile = encryptedByteArray, + encryptedFile = encryptedFile, ) } returns sendFileResponse.createFileJsonResponse.sendResponse.asSuccess() coEvery {