From 9bca064881fd70ae4813f12b1958d2abb6edbe4f Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Wed, 25 Nov 2020 15:26:25 +0100
Subject: [PATCH] Improve Gson deserialization handling (DEV) (#1715)
* Check whether the local config file exists before parsing it.
* Force types deserialized via Gson to be non-null.
Perform sanity checks and empty file deletion directly when trying to load the json file.
* Check that empty json file is deleted.
* LINTs
* Test parsing exception.
Co-authored-by: Kolya Opahle <k.opahle@sap.com>
Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com>
Co-authored-by: Ralf Gehrer <ralfgehrer@users.noreply.github.com>
---
.../sources/local/AppConfigStorage.kt | 3 +-
.../sources/remote/RemoteAppConfigSource.kt | 2 +-
.../ExposureDetectionTrackerStorage.kt | 6 +-
.../util/serialization/GsonExtensions.kt | 25 +++++++-
.../ExposureDetectionTrackerStorageTest.kt | 3 +-
.../util/serialization/GsonExtensionsTest.kt | 62 +++++++++++++++++++
6 files changed, 91 insertions(+), 10 deletions(-)
create mode 100644 Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/serialization/GsonExtensionsTest.kt
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/local/AppConfigStorage.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/local/AppConfigStorage.kt
index dda995f9b..4f0db33f2 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/local/AppConfigStorage.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/local/AppConfigStorage.kt
@@ -61,8 +61,9 @@ class AppConfigStorage @Inject constructor(
}
return@withLock try {
- gson.fromJson<InternalConfigData>(configFile).also {
+ gson.fromJson<InternalConfigData>(configFile)?.also {
requireNotNull(it.rawData)
+ Timber.v("Loaded stored config, serverTime=%s", it.serverTime)
}
} catch (e: Exception) {
Timber.e(e, "Couldn't load config.")
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt
index e5006f027..f77c2a8de 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/appconfig/sources/remote/RemoteAppConfigSource.kt
@@ -19,7 +19,7 @@ class RemoteAppConfigSource @Inject constructor(
) {
suspend fun getConfigData(): ConfigData? = withContext(dispatcherProvider.IO) {
- Timber.tag(TAG).v("retrieveConfig()")
+ Timber.tag(TAG).v("getConfigData()")
val configDownload = try {
server.downloadAppConfig()
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorage.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorage.kt
index c4299cafe..8ca647c1d 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorage.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorage.kt
@@ -43,13 +43,11 @@ class ExposureDetectionTrackerStorage @Inject constructor(
suspend fun load(): Map<String, TrackedExposureDetection> = mutex.withLock {
return@withLock try {
- if (!storageFile.exists()) return@withLock emptyMap()
-
- gson.fromJson<Map<String, TrackedExposureDetection>>(storageFile).also {
+ gson.fromJson<Map<String, TrackedExposureDetection>>(storageFile)?.also {
require(it.size >= 0)
Timber.v("Loaded detection data: %s", it)
lastCalcuationData = it
- }
+ } ?: emptyMap()
} catch (e: Exception) {
Timber.e(e, "Failed to load tracked detections.")
if (storageFile.delete()) Timber.w("Storage file was deleted.")
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/serialization/GsonExtensions.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/serialization/GsonExtensions.kt
index f014dc54c..04234b4c5 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/serialization/GsonExtensions.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/serialization/GsonExtensions.kt
@@ -3,6 +3,7 @@ package de.rki.coronawarnapp.util.serialization
import com.google.gson.Gson
import com.google.gson.TypeAdapter
import com.google.gson.reflect.TypeToken
+import timber.log.Timber
import java.io.File
import kotlin.reflect.KClass
@@ -11,8 +12,28 @@ inline fun <reified T> Gson.fromJson(json: String): T = fromJson(
object : TypeToken<T>() {}.type
)
-inline fun <reified T> Gson.fromJson(file: File): T = file.bufferedReader().use {
- fromJson(it, object : TypeToken<T>() {}.type)
+/**
+ * Returns null if the file doesn't exist, otherwise returns the parsed object.
+ * Throws an exception if the object can't be parsed.
+ * An empty file, that was deserialized to a null value is deleted.
+ */
+inline fun <reified T : Any> Gson.fromJson(file: File): T? {
+ if (!file.exists()) {
+ Timber.v("fromJson(): File doesn't exist %s", file)
+ return null
+ }
+
+ return file.bufferedReader().use {
+ val value: T? = fromJson(it, object : TypeToken<T>() {}.type)
+ if (value != null) {
+ Timber.v("Json read from %s", file)
+ value
+ } else {
+ Timber.w("Tried to parse json from file that exists, but was empty: %s", file)
+ if (file.delete()) Timber.w("Deleted empty json file: %s", file)
+ null
+ }
+ }
}
inline fun <reified T> Gson.toJson(data: T, file: File) = file.bufferedWriter().use { writer ->
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorageTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorageTest.kt
index 702291cea..61e3bcab5 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorageTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/nearby/modules/detectiontracker/ExposureDetectionTrackerStorageTest.kt
@@ -113,11 +113,10 @@ class ExposureDetectionTrackerStorageTest : BaseIOTest() {
@Test
fun `saving data creates a json file`() = runBlockingTest {
-
createStorage().save(demoData)
storageFile.exists() shouldBe true
- val storedData: Map<String, TrackedExposureDetection> = gson.fromJson(storageFile)
+ val storedData: Map<String, TrackedExposureDetection> = gson.fromJson(storageFile)!!
storedData shouldBe demoData
gson.toJson(storedData) shouldBe demoJsonString
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/serialization/GsonExtensionsTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/serialization/GsonExtensionsTest.kt
new file mode 100644
index 000000000..72b6ed30a
--- /dev/null
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/serialization/GsonExtensionsTest.kt
@@ -0,0 +1,62 @@
+package de.rki.coronawarnapp.util.serialization
+
+import com.google.gson.Gson
+import com.google.gson.JsonSyntaxException
+import io.kotest.assertions.throwables.shouldThrow
+import io.kotest.matchers.shouldBe
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import testhelpers.BaseIOTest
+import java.io.File
+import java.util.UUID
+
+class GsonExtensionsTest : BaseIOTest() {
+
+ private val testDir = File(IO_TEST_BASEDIR, this::class.java.simpleName)
+ private val testFile = File(testDir, "testfile")
+ private val gson = Gson()
+
+ @BeforeEach
+ fun setup() {
+ testDir.mkdirs()
+ }
+
+ @AfterEach
+ fun teardown() {
+ testDir.deleteRecursively()
+ }
+
+ data class TestData(
+ val value: String
+ )
+
+ @Test
+ fun `serialize and deserialize`() {
+ val testData = TestData(value = UUID.randomUUID().toString())
+ gson.toJson(testData, testFile)
+
+ gson.fromJson<TestData>(testFile) shouldBe testData
+ }
+
+ @Test
+ fun `deserialize an empty file`() {
+ testFile.createNewFile()
+ testFile.exists() shouldBe true
+
+ val testData: TestData? = gson.fromJson(testFile)
+
+ testData shouldBe null
+
+ testFile.exists() shouldBe false
+ }
+
+ @Test
+ fun `deserialize a malformed file`() {
+ testFile.writeText("{")
+
+ shouldThrow<JsonSyntaxException> {
+ gson.fromJson(testFile)
+ }
+ }
+}
--
GitLab