From 87a31ec2f23df2a2cf3ab8941fc8e0720b21c9ec Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Mon, 5 Oct 2020 11:14:38 -0300
Subject: [PATCH] Ensure that metadata cache streams get closed

---
 .../seedvault/metadata/MetadataManager.kt     | 12 ++++--
 .../seedvault/metadata/MetadataManagerTest.kt | 42 +++++++++++++++++++
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt b/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt
index 830c4e8f..83d9d4ba 100644
--- a/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt
+++ b/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt
@@ -66,6 +66,8 @@ class MetadataManager(
      *
      * It updates the packages' metadata
      * and writes it encrypted to the given [OutputStream] as well as the internal cache.
+     *
+     * Closing the [OutputStream] is the responsibility of the caller.
      */
     @Synchronized
     @Throws(IOException::class)
@@ -112,6 +114,8 @@ class MetadataManager(
      *
      * It updates the packages' metadata
      * and writes it encrypted to the given [OutputStream] as well as the internal cache.
+     *
+     * Closing the [OutputStream] is the responsibility of the caller.
      */
     @Synchronized
     @Throws(IOException::class)
@@ -221,8 +225,8 @@ class MetadataManager(
     @VisibleForTesting
     private fun getMetadataFromCache(): BackupMetadata? {
         try {
-            with(context.openFileInput(METADATA_CACHE_FILE)) {
-                return metadataReader.decode(readBytes())
+            context.openFileInput(METADATA_CACHE_FILE).use { stream ->
+                return metadataReader.decode(stream.readBytes())
             }
         } catch (e: SecurityException) {
             Log.e(TAG, "Error parsing cached metadata", e)
@@ -237,8 +241,8 @@ class MetadataManager(
     @VisibleForTesting
     @Throws(IOException::class)
     private fun writeMetadataToCache() {
-        with(context.openFileOutput(METADATA_CACHE_FILE, MODE_PRIVATE)) {
-            write(metadataWriter.encode(metadata))
+        context.openFileOutput(METADATA_CACHE_FILE, MODE_PRIVATE).use { stream ->
+            stream.write(metadataWriter.encode(metadata))
         }
     }
 
diff --git a/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt b/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt
index 0d941449..6b356016 100644
--- a/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt
+++ b/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt
@@ -20,6 +20,7 @@ import io.mockk.Runs
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
+import io.mockk.verify
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.fail
@@ -74,6 +75,11 @@ class MetadataManagerTest {
 
         assertEquals(token, manager.getBackupToken())
         assertEquals(0L, manager.getLastBackupTime())
+
+        verify {
+            cacheInputStream.close()
+            cacheOutputStream.close()
+        }
     }
 
     @Test
@@ -91,6 +97,11 @@ class MetadataManagerTest {
         manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
 
         assertEquals(packageMetadata, manager.getPackageMetadata(packageName))
+
+        verify {
+            cacheInputStream.close()
+            cacheOutputStream.close()
+        }
     }
 
     @Test
@@ -109,6 +120,11 @@ class MetadataManagerTest {
         manager.onApkBackedUp(packageInfo, packageMetadata, storageOutputStream)
 
         assertEquals(packageMetadata.copy(system = true), manager.getPackageMetadata(packageName))
+
+        verify {
+            cacheInputStream.close()
+            cacheOutputStream.close()
+        }
     }
 
     @Test
@@ -133,6 +149,11 @@ class MetadataManagerTest {
         manager.onApkBackedUp(packageInfo, updatedPackageMetadata, storageOutputStream)
 
         assertEquals(updatedPackageMetadata, manager.getPackageMetadata(packageName))
+
+        verify {
+            cacheInputStream.close()
+            cacheOutputStream.close()
+        }
     }
 
     @Test
@@ -187,6 +208,11 @@ class MetadataManagerTest {
             packageMetadata.copy(state = WAS_STOPPED),
             manager.getPackageMetadata(packageName)
         )
+
+        verify {
+            cacheInputStream.close()
+            cacheOutputStream.close()
+        }
     }
 
     @Test
@@ -210,6 +236,11 @@ class MetadataManagerTest {
             manager.getPackageMetadata(packageName)
         )
         assertEquals(time, manager.getLastBackupTime())
+
+        verify {
+            cacheInputStream.close()
+            cacheOutputStream.close()
+        }
     }
 
     @Test
@@ -237,6 +268,8 @@ class MetadataManagerTest {
             initialMetadata.packageMetadataMap[packageName],
             manager.getPackageMetadata(packageName)
         )
+
+        verify { cacheInputStream.close() }
     }
 
     @Test
@@ -265,6 +298,11 @@ class MetadataManagerTest {
             updatedMetadata.packageMetadataMap[packageName],
             manager.getPackageMetadata(packageName)
         )
+
+        verify {
+            cacheInputStream.close()
+            cacheOutputStream.close()
+        }
     }
 
     @Test
@@ -289,6 +327,8 @@ class MetadataManagerTest {
 
         assertEquals(initialMetadata.time, manager.getLastBackupTime())
         assertEquals(initialMetadata.token, manager.getBackupToken())
+
+        verify { cacheInputStream.close() }
     }
 
     private fun expectModifyMetadata(metadata: BackupMetadata) {
@@ -301,6 +341,7 @@ class MetadataManagerTest {
             )
         } returns cacheOutputStream
         every { cacheOutputStream.write(encodedMetadata) } just Runs
+        every { cacheOutputStream.close() } just Runs
     }
 
     private fun expectReadFromCache() {
@@ -309,6 +350,7 @@ class MetadataManagerTest {
         every { cacheInputStream.available() } returns byteArray.size andThen 0
         every { cacheInputStream.read(byteArray) } returns -1
         every { metadataReader.decode(ByteArray(0)) } returns initialMetadata
+        every { cacheInputStream.close() } just Runs
     }
 
 }