diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt
index 3979f1b6..606e8793 100644
--- a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt
+++ b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ChunkWriter.kt
@@ -17,7 +17,6 @@ import java.io.OutputStream
import java.nio.file.attribute.FileTime
import java.security.GeneralSecurityException
import java.util.zip.ZipEntry
-import java.util.zip.ZipOutputStream
import kotlin.math.min
internal data class ChunkWriterResult(
@@ -121,9 +120,17 @@ internal class ChunkWriter(
}
@Throws(IOException::class)
- fun writeNewZipEntry(zipOutputStream: ZipOutputStream, counter: Int, inputStream: InputStream) {
- val entry = createNewZipEntry(counter)
- zipOutputStream.putNextEntry(entry)
+ fun writeNewZipEntry(
+ zipOutputStream: NameZipOutputStream,
+ counter: Int,
+ inputStream: InputStream,
+ ) {
+ // If copying below throws an exception, we'd be adding a new entry with the same counter,
+ // so we check if we have added an entry for that counter already to prevent duplicates.
+ if ((zipOutputStream.lastName?.toIntOrNull() ?: 0) != counter) {
+ val entry = createNewZipEntry(counter)
+ zipOutputStream.putNextEntry(entry)
+ }
inputStream.copyTo(zipOutputStream)
zipOutputStream.closeEntry()
}
diff --git a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ZipChunker.kt b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ZipChunker.kt
index f98f9327..88efa1be 100644
--- a/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ZipChunker.kt
+++ b/storage/lib/src/main/java/org/calyxos/backup/storage/backup/ZipChunker.kt
@@ -11,7 +11,9 @@ import org.calyxos.backup.storage.toHexString
import java.io.ByteArrayOutputStream
import java.io.IOException
import java.io.InputStream
+import java.io.OutputStream
import java.security.GeneralSecurityException
+import java.util.zip.ZipEntry
import java.util.zip.ZipOutputStream
import javax.crypto.Mac
@@ -36,7 +38,7 @@ internal class ZipChunker(
private val files = ArrayList<ContentFile>()
private val outputStream = ByteArrayOutputStream(chunkSizeMax)
- private var zipOutputStream = ZipOutputStream(outputStream)
+ private var zipOutputStream = NameZipOutputStream(outputStream)
// we start with 1, because 0 is the default value in protobuf 3
private var counter = 1
@@ -77,8 +79,21 @@ internal class ZipChunker(
private fun reset() {
files.clear()
outputStream.reset()
- zipOutputStream = ZipOutputStream(outputStream)
+ zipOutputStream = NameZipOutputStream(outputStream)
counter = 1
}
}
+
+/**
+ * A wrapper for [ZipOutputStream] that remembers the name of the last [ZipEntry] that was added.
+ */
+internal class NameZipOutputStream(outputStream: OutputStream) : ZipOutputStream(outputStream) {
+ internal var lastName: String? = null
+ private set
+
+ override fun putNextEntry(e: ZipEntry) {
+ super.putNextEntry(e)
+ lastName = e.name
+ }
+}
diff --git a/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupIntegrationTest.kt b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupIntegrationTest.kt
new file mode 100644
index 00000000..4d8214e4
--- /dev/null
+++ b/storage/lib/src/test/java/org/calyxos/backup/storage/backup/SmallFileBackupIntegrationTest.kt
@@ -0,0 +1,115 @@
+/*
+ * SPDX-FileCopyrightText: 2021 The Calyx Institute
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.calyxos.backup.storage.backup
+
+import android.content.ContentResolver
+import io.mockk.Runs
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.just
+import io.mockk.mockk
+import kotlinx.coroutines.runBlocking
+import org.calyxos.backup.storage.api.BackupObserver
+import org.calyxos.backup.storage.api.StoragePlugin
+import org.calyxos.backup.storage.crypto.Hkdf.KEY_SIZE_BYTES
+import org.calyxos.backup.storage.crypto.StreamCrypto
+import org.calyxos.backup.storage.db.CachedChunk
+import org.calyxos.backup.storage.db.ChunksCache
+import org.calyxos.backup.storage.db.FilesCache
+import org.calyxos.backup.storage.getRandomDocFile
+import org.calyxos.backup.storage.getRandomString
+import org.calyxos.backup.storage.mockLog
+import org.calyxos.backup.storage.toHexString
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import java.io.ByteArrayInputStream
+import java.io.ByteArrayOutputStream
+import java.io.IOException
+import java.io.InputStream
+import javax.crypto.Mac
+import kotlin.random.Random
+
+internal class SmallFileBackupIntegrationTest {
+
+ private val contentResolver: ContentResolver = mockk()
+ private val filesCache: FilesCache = mockk()
+ private val mac: Mac = mockk()
+ private val chunksCache: ChunksCache = mockk()
+ private val storagePlugin: StoragePlugin = mockk()
+
+ private val chunkWriter = ChunkWriter(
+ streamCrypto = StreamCrypto,
+ streamKey = Random.nextBytes(KEY_SIZE_BYTES),
+ chunksCache = chunksCache,
+ storagePlugin = storagePlugin,
+ )
+ private val zipChunker = ZipChunker(
+ mac = mac,
+ chunkWriter = chunkWriter,
+ )
+
+ private val smallFileBackup = SmallFileBackup(contentResolver, filesCache, zipChunker, true)
+
+ init {
+ mockLog()
+ }
+
+ /**
+ * This tests that if writing out one ZIP entry throws an exception,
+ * the subsequent entries can still be written.
+ * Previously, we'd start a new ZipEntry with the same counter value
+ * which is not allowed, so all subsequent files would also not get backed up.
+ */
+ @Test
+ fun `first of two new files throws and gets ignored`(): Unit = runBlocking {
+ val file1 = getRandomDocFile()
+ val file2 = getRandomDocFile()
+ val files = listOf(file1, file2)
+ val availableChunkIds = hashSetOf(getRandomString(6))
+ val observer: BackupObserver = mockk()
+
+ val inputStream1: InputStream = mockk()
+ val inputStream2: InputStream = ByteArrayInputStream(Random.nextBytes(42))
+ val outputStream2 = ByteArrayOutputStream()
+
+ val chunkId = Random.nextBytes(KEY_SIZE_BYTES)
+ val cachedChunk = CachedChunk(chunkId.toHexString(), 0, 181, 0)
+ val cachedFile2 = file2.toCachedFile(listOf(chunkId.toHexString()), 1)
+ val backupFile = file2.toBackupFile(cachedFile2.chunks, cachedFile2.zipIndex)
+
+ every { filesCache.getByUri(file1.uri) } returns null
+ every { filesCache.getByUri(file2.uri) } returns null
+
+ every { contentResolver.openInputStream(file1.uri) } returns inputStream1
+ every { contentResolver.openInputStream(file2.uri) } returns inputStream2
+
+ every { inputStream1.read(any<ByteArray>()) } throws IOException()
+ coEvery { observer.onFileBackupError(file1, "S") } just Runs
+
+ every { mac.doFinal(any<ByteArray>()) } returns chunkId
+ every { chunksCache.get(any()) } returns null
+ every { storagePlugin.getChunkOutputStream(any()) } returns outputStream2
+ every { chunksCache.insert(cachedChunk) } just Runs
+ every {
+ filesCache.upsert(match {
+ it.copy(lastSeen = cachedFile2.lastSeen) == cachedFile2
+ })
+ } just Runs
+ coEvery { observer.onFileBackedUp(file2, true, 0, 181, "S") } just Runs
+
+ val result = smallFileBackup.backupFiles(files, availableChunkIds, observer)
+ assertEquals(setOf(chunkId.toHexString()), result.chunkIds)
+ assertEquals(1, result.backupDocumentFiles.size)
+ assertEquals(backupFile, result.backupDocumentFiles[0])
+ assertEquals(0, result.backupMediaFiles.size)
+
+ coVerify {
+ observer.onFileBackedUp(file2, true, 0, 181, "S")
+ }
+ }
+
+}