diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt index 8cc0869936a2c50288b8afb435c344174a1c151d..b0ddc5c2e45245be57eccfd373d000541ae2269c 100644 --- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt +++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/files/FileManager.kt @@ -21,6 +21,7 @@ package org.briarproject.mailbox.core.files import io.ktor.application.ApplicationCall import io.ktor.auth.principal +import io.ktor.features.BadRequestException import io.ktor.http.HttpStatusCode import io.ktor.request.receiveStream import io.ktor.response.respond @@ -37,9 +38,12 @@ import org.briarproject.mailbox.core.setup.WipeManager import org.briarproject.mailbox.core.system.InvalidIdException import org.briarproject.mailbox.core.system.RandomIdManager import org.slf4j.LoggerFactory.getLogger +import java.io.InputStream +import java.io.OutputStream import javax.inject.Inject private val LOG = getLogger(FileManager::class.java) +internal const val MAX_FILE_SIZE = 1024 * 1024 class FileManager @Inject constructor( private val fileProvider: FileProvider, @@ -86,7 +90,7 @@ class FileRouteManager @Inject constructor( * (no 201 as the uploader doesn't need to know the $fileId) * The mailbox chooses a random ID string for the file ID. */ - @Throws(AuthException::class, InvalidIdException::class) + @Throws(AuthException::class, InvalidIdException::class, BadRequestException::class) suspend fun postFile(call: ApplicationCall, folderId: String) { val principal: MailboxPrincipal? = call.principal() randomIdManager.assertIsRandomId(folderId) @@ -97,7 +101,12 @@ class FileRouteManager @Inject constructor( val tmpFile = fileProvider.getTemporaryFile(fileId) tmpFile.outputStream().use { outputStream -> call.receiveStream().use { inputStream -> - inputStream.copyTo(outputStream) + try { + copyFile(inputStream, outputStream) + } catch (e: Exception) { + tmpFile.delete() + throw e + } } } val file = fileProvider.getFile(folderId, fileId) @@ -107,6 +116,20 @@ class FileRouteManager @Inject constructor( call.respond(HttpStatusCode.OK) } + private fun copyFile(inputStream: InputStream, outputStream: OutputStream) { + var bytesCopied: Long = 0 + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + var bytes = inputStream.read(buffer) + while (bytes >= 0) { + outputStream.write(buffer, 0, bytes) + bytesCopied += bytes + if (bytesCopied > MAX_FILE_SIZE) { + throw BadRequestException("File larger than allowed.") + } + bytes = inputStream.read(buffer) + } + } + /** * Used by owner and contacts to list their files to retrieve. * diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileManagerIntegrationTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileManagerIntegrationTest.kt index cd92398a51cfbb33d007f1e377cbac9b4e55260a..22c7723608cc93fd323d3e42f2a93119e9f30a53 100644 --- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileManagerIntegrationTest.kt +++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileManagerIntegrationTest.kt @@ -15,8 +15,10 @@ import org.briarproject.mailbox.core.server.IntegrationTest import org.junit.jupiter.api.Assertions.assertArrayEquals import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import java.io.File import kotlin.random.Random import kotlin.test.assertEquals +import kotlin.test.assertTrue class FileManagerIntegrationTest : IntegrationTest() { @@ -58,6 +60,19 @@ class FileManagerIntegrationTest : IntegrationTest() { assertEquals("Malformed ID: foo", response.readText()) } + @Test + fun `post new file rejects large file`(): Unit = runBlocking { + val maxBytes = Random.nextBytes(MAX_FILE_SIZE + 1) + // owner uploads a file above limit + val response: HttpResponse = httpClient.post("$baseUrl/files/${contact1.inboxId}") { + authenticateWithToken(ownerToken) + body = maxBytes + } + assertEquals(HttpStatusCode.BadRequest, response.status) + assertEquals("Bad request: File larger than allowed.", response.readText()) + assertNoTmpFiles() + } + @Test fun `post new file, list, download and delete it`(): Unit = runBlocking { assertEquals(0L, metadataManager.ownerConnectionTime.value) @@ -67,6 +82,7 @@ class FileManagerIntegrationTest : IntegrationTest() { body = bytes } assertEquals(HttpStatusCode.OK, response.status) + assertNoTmpFiles() // owner connection got registered assertTimestampRecent(metadataManager.ownerConnectionTime.value) @@ -100,6 +116,7 @@ class FileManagerIntegrationTest : IntegrationTest() { authenticateWithToken(contact1.token) } assertEquals(0, emptyFileListResponse.files.size) + assertNoTmpFiles() } @Test @@ -323,4 +340,11 @@ class FileManagerIntegrationTest : IntegrationTest() { // owner connection got registered assertTimestampRecent(metadataManager.ownerConnectionTime.value) } + + private fun assertNoTmpFiles() { + val dir = requireNotNull(this.tempDir) + val tmp = File(dir, "tmp") + assertTrue(tmp.isDirectory) + assertEquals(0, tmp.listFiles()?.size) + } } diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileRouteManagerTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileRouteManagerTest.kt index 4a89977ea39b6c4cc51dccf8c948bfb30538a505..0f40cf73168fa440c02ff9b7329d557cabc08534 100644 --- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileRouteManagerTest.kt +++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/files/FileRouteManagerTest.kt @@ -2,6 +2,7 @@ package org.briarproject.mailbox.core.files import io.ktor.application.ApplicationCall import io.ktor.auth.principal +import io.ktor.features.BadRequestException import io.ktor.http.HttpStatusCode import io.ktor.request.receiveStream import io.ktor.response.respond @@ -24,6 +25,7 @@ import org.junit.jupiter.api.io.TempDir import java.io.ByteArrayInputStream import java.io.File import kotlin.random.Random +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -48,6 +50,25 @@ class FileRouteManagerTest { @Test fun `post new file stores file correctly`(@TempDir tempDir: File) = runBlocking { + postFile(tempDir, bytes) + } + + @Test + fun `post a new file with max size gets accepted`(@TempDir tempDir: File) = runBlocking { + val maxBytes = Random.nextBytes(MAX_FILE_SIZE) + postFile(tempDir, maxBytes) + } + + @Test + fun `post file above max size gets rejected`(@TempDir tempDir: File) = runBlocking { + val maxBytes = Random.nextBytes(MAX_FILE_SIZE + 1) + assertFailsWith<BadRequestException> { + postFile(tempDir, maxBytes) + } + Unit + } + + private suspend fun postFile(tempDir: File, bytes: ByteArray) { val tmpFile = File(tempDir, "tmp") val finalFile = File(tempDir, "final") diff --git a/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt b/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt index 778f9ed08e08029b9afff25ad39e886b3fce8929..3eef276aee498676b0c1896b75b879b7e29da381 100644 --- a/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt +++ b/mailbox-core/src/test/java/org/briarproject/mailbox/core/server/IntegrationTest.kt @@ -61,6 +61,7 @@ abstract class IntegrationTest(private val installJsonFeature: Boolean = true) { protected val id = getNewRandomId() protected val contact1 = getNewRandomContact() protected val contact2 = getNewRandomContact() + protected var tempDir: File? = null @Volatile protected var exceptionInBackgroundThread: Throwable? = null @@ -76,6 +77,7 @@ abstract class IntegrationTest(private val installJsonFeature: Boolean = true) { @BeforeAll fun setUp(@TempDir tempDir: File) { + this.tempDir = tempDir testComponent = DaggerTestComponent.builder().testModule(TestModule(tempDir)).build() testComponent.injectCoreEagerSingletons() assertFalse(setupManager.hasDb)