From a6335e7fb0bf48f493c0cf187d84a158c75f098e Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Mon, 16 Nov 2020 12:09:03 +0100
Subject: [PATCH] Handle failure to persist GSON serialized JSON data
(EXPOSUREAPP-3777) (#1604)
* Flush the json data after writing it, otherwise if the app process dies, we end up with an empty file.
GSON deserializes this into a null object that looks like a non-null object until it is evaluated some time later.
Force an evaluation via `require(it.size >= 0)` and delete the corrupt data file when catching the exception.
* Show a visible error dialog if we fail to save data.
* Used buffered versions of readers and writer.
---
.../CalculationTrackerStorage.kt | 5 +++++
.../coronawarnapp/util/gson/GsonExtensions.kt | 5 +++--
.../CalculationTrackerStorageTest.kt | 16 ++++++++++++++++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorage.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorage.kt
index 1aaf4d63b..1d907fcc6 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorage.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorage.kt
@@ -2,6 +2,8 @@ package de.rki.coronawarnapp.nearby.modules.calculationtracker
import android.content.Context
import com.google.gson.Gson
+import de.rki.coronawarnapp.exception.ExceptionCategory
+import de.rki.coronawarnapp.exception.reporting.report
import de.rki.coronawarnapp.util.di.AppContext
import de.rki.coronawarnapp.util.gson.fromJson
import de.rki.coronawarnapp.util.gson.toJson
@@ -36,11 +38,13 @@ class CalculationTrackerStorage @Inject constructor(
if (!storageFile.exists()) return@withLock emptyMap()
gson.fromJson<Map<String, Calculation>>(storageFile).also {
+ require(it.size >= 0)
Timber.v("Loaded calculation data: %s", it)
lastCalcuationData = it
}
} catch (e: Exception) {
Timber.e(e, "Failed to load tracked calculations.")
+ if (storageFile.delete()) Timber.w("Storage file was deleted.")
emptyMap()
}
}
@@ -55,6 +59,7 @@ class CalculationTrackerStorage @Inject constructor(
gson.toJson(data, storageFile)
} catch (e: Exception) {
Timber.e(e, "Failed to save tracked calculations.")
+ e.report(ExceptionCategory.INTERNAL)
}
}
}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/gson/GsonExtensions.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/gson/GsonExtensions.kt
index b79e725c5..79f055e93 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/gson/GsonExtensions.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/gson/GsonExtensions.kt
@@ -9,10 +9,11 @@ inline fun <reified T> Gson.fromJson(json: String): T = fromJson(
object : TypeToken<T>() {}.type
)
-inline fun <reified T> Gson.fromJson(file: File): T = file.reader().use {
+inline fun <reified T> Gson.fromJson(file: File): T = file.bufferedReader().use {
fromJson(it, object : TypeToken<T>() {}.type)
}
-inline fun <reified T> Gson.toJson(data: T, file: File) = file.writer().use { writer ->
+inline fun <reified T> Gson.toJson(data: T, file: File) = file.bufferedWriter().use { writer ->
toJson(data, writer)
+ writer.flush()
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorageTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorageTest.kt
index 5b1be36e8..4ab6acc1f 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorageTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/calculationtracker/CalculationTrackerStorageTest.kt
@@ -130,4 +130,20 @@ class CalculationTrackerStorageTest : BaseIOTest() {
storedData.getValue("b2b98400-058d-43e6-b952-529a5255248b").isCalculating shouldBe true
storedData.getValue("aeb15509-fb34-42ce-8795-7a9ae0c2f389").isCalculating shouldBe false
}
+
+ @Test
+ fun `we catch empty json data and prevent unsafely initialized maps`() = runBlockingTest {
+ storageDir.mkdirs()
+ storageFile.writeText("")
+
+ storageFile.exists() shouldBe true
+
+ createStorage().apply {
+ val value = load()
+ value.size shouldBe 0
+ value shouldBe emptyMap()
+
+ storageFile.exists() shouldBe false
+ }
+ }
}
--
GitLab