From 84f66c81ad2beff9ae5dac1a8b94f0ed53e6be4e Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Fri, 23 Apr 2021 10:06:13 +0200 Subject: [PATCH] Remove GSON footguns. (#2921) GSON uses unsafeinit and will initialize a data class without any "val some = thing" being initialized that are in the class body. Co-authored-by: Alex Paulescu <alex.paulescu@gmail.com> --- .../coronatest/storage/CoronaTestStorage.kt | 4 +- .../coronatest/type/pcr/PCRCoronaTest.kt | 42 +++++++++---------- .../type/rapidantigen/RACoronaTest.kt | 33 ++++++++------- .../datadonation/OneTimePassword.kt | 12 +++--- .../diagnosiskeys/server/LocationCode.kt | 4 +- .../diagnosiskeys/storage/CachedKeyInfo.kt | 4 +- .../storage/TraceWarningPackageMetadata.kt | 4 +- .../storage/CoronaTestStorageTest.kt | 23 ++++++---- 8 files changed, 70 insertions(+), 56 deletions(-) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorage.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorage.kt index 0fc19f03b..a0417a641 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorage.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorage.kt @@ -47,14 +47,16 @@ class CoronaTestStorage @Inject constructor( gson.fromJson<Set<PCRCoronaTest>>(raw, typeTokenPCR).onEach { Timber.tag(TAG).v("PCR loaded: %s", it) requireNotNull(it.identifier) + requireNotNull(it.type) { "PCR type should not be null, GSON footgun." } } } val raTests: Set<RACoronaTest> = run { val raw = prefs.getString(PKEY_DATA_RA, null) ?: return@run emptySet() gson.fromJson<Set<RACoronaTest>>(raw, typeTokenRA).onEach { - Timber.tag(TAG).v("PCR loaded: %s", it) + Timber.tag(TAG).v("RA loaded: %s", it) requireNotNull(it.identifier) + requireNotNull(it.type) { "RA type should not be null, GSON footgun." } } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt index 0f260941b..6c0314767 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRCoronaTest.kt @@ -42,27 +42,27 @@ data class PCRCoronaTest( @Transient override val lastError: Throwable? = null, ) : CoronaTest { - @Transient - override val type: CoronaTest.Type = CoronaTest.Type.PCR - - @Transient - override val isPositive: Boolean = testResult == CoronaTestResult.PCR_POSITIVE - - @Transient - override val isPending: Boolean = testResult == CoronaTestResult.PCR_OR_RAT_PENDING - - @Transient - override val isSubmissionAllowed: Boolean = isPositive && !isSubmitted - - @Transient - val state: State = when (testResult) { - CoronaTestResult.PCR_OR_RAT_PENDING -> State.PENDING - CoronaTestResult.PCR_NEGATIVE -> State.NEGATIVE - CoronaTestResult.PCR_POSITIVE -> State.POSITIVE - CoronaTestResult.PCR_INVALID -> State.INVALID - CoronaTestResult.PCR_REDEEMED -> State.REDEEMED - else -> throw IllegalArgumentException("Invalid PCR test state $testResult") - } + override val type: CoronaTest.Type + get() = CoronaTest.Type.PCR + + override val isPositive: Boolean + get() = testResult == CoronaTestResult.PCR_POSITIVE + + override val isPending: Boolean + get() = testResult == CoronaTestResult.PCR_OR_RAT_PENDING + + override val isSubmissionAllowed: Boolean + get() = isPositive && !isSubmitted + + val state: State + get() = when (testResult) { + CoronaTestResult.PCR_OR_RAT_PENDING -> State.PENDING + CoronaTestResult.PCR_NEGATIVE -> State.NEGATIVE + CoronaTestResult.PCR_POSITIVE -> State.POSITIVE + CoronaTestResult.PCR_INVALID -> State.INVALID + CoronaTestResult.PCR_REDEEMED -> State.REDEEMED + else -> throw IllegalArgumentException("Invalid PCR test state $testResult") + } enum class State { PENDING, diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt index ca4c97568..a8f6b3f5d 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/rapidantigen/RACoronaTest.kt @@ -2,6 +2,12 @@ package de.rki.coronawarnapp.coronatest.type.rapidantigen import com.google.gson.annotations.SerializedName import de.rki.coronawarnapp.coronatest.server.CoronaTestResult +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.PCR_OR_RAT_PENDING +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_INVALID +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_NEGATIVE +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_PENDING +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_POSITIVE +import de.rki.coronawarnapp.coronatest.server.CoronaTestResult.RAT_REDEEMED import de.rki.coronawarnapp.coronatest.type.CoronaTest import de.rki.coronawarnapp.coronatest.type.RegistrationToken import de.rki.coronawarnapp.coronatest.type.TestIdentifier @@ -55,27 +61,26 @@ data class RACoronaTest( @Transient override val lastError: Throwable? = null, ) : CoronaTest { - @Transient - override val type: CoronaTest.Type = CoronaTest.Type.RAPID_ANTIGEN + override val type: CoronaTest.Type + get() = CoronaTest.Type.RAPID_ANTIGEN fun getState(nowUTC: Instant) = when (testResult) { - CoronaTestResult.PCR_OR_RAT_PENDING -> State.PENDING - CoronaTestResult.RAT_NEGATIVE -> State.NEGATIVE - CoronaTestResult.RAT_POSITIVE -> State.POSITIVE - CoronaTestResult.RAT_INVALID -> State.INVALID - CoronaTestResult.RAT_REDEEMED -> State.REDEEMED + PCR_OR_RAT_PENDING -> State.PENDING + RAT_NEGATIVE -> State.NEGATIVE + RAT_POSITIVE -> State.POSITIVE + RAT_INVALID -> State.INVALID + RAT_REDEEMED -> State.REDEEMED else -> throw IllegalArgumentException("Invalid RAT test state $testResult") } - @Transient - override val isPositive: Boolean = testResult == CoronaTestResult.RAT_POSITIVE + override val isPositive: Boolean + get() = testResult == RAT_POSITIVE - @Transient - override val isPending: Boolean = - testResult == CoronaTestResult.PCR_OR_RAT_PENDING || testResult == CoronaTestResult.RAT_PENDING + override val isPending: Boolean + get() = setOf(PCR_OR_RAT_PENDING, RAT_PENDING).contains(testResult) - @Transient - override val isSubmissionAllowed: Boolean = isPositive && !isSubmitted + override val isSubmissionAllowed: Boolean + get() = isPositive && !isSubmitted enum class State { PENDING, diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/OneTimePassword.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/OneTimePassword.kt index 8bf382bdc..1692cb7d4 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/OneTimePassword.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/OneTimePassword.kt @@ -14,11 +14,11 @@ data class OneTimePassword( val time: Instant = Instant.now() ) { - @Transient - val edusOneTimePassword: EdusOtp.EDUSOneTimePassword = EdusOtp.EDUSOneTimePassword.newBuilder() - .setOtp(uuid.toString()) - .build() + val edusOneTimePassword: EdusOtp.EDUSOneTimePassword + get() = EdusOtp.EDUSOneTimePassword.newBuilder() + .setOtp(uuid.toString()) + .build() - @Transient - val payloadForRequest: ByteArray = edusOneTimePassword.toByteArray() + val payloadForRequest: ByteArray + get() = edusOneTimePassword.toByteArray() } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/server/LocationCode.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/server/LocationCode.kt index 8201571a4..2a3404e55 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/server/LocationCode.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/server/LocationCode.kt @@ -5,6 +5,6 @@ import java.util.Locale data class LocationCode( private val rawIdentifier: String ) { - @Transient - val identifier: String = rawIdentifier.toUpperCase(Locale.ROOT) + val identifier: String + get() = rawIdentifier.toUpperCase(Locale.ROOT) } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/storage/CachedKeyInfo.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/storage/CachedKeyInfo.kt index 9dd0b8ada..ada80369a 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/storage/CachedKeyInfo.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/storage/CachedKeyInfo.kt @@ -41,8 +41,8 @@ data class CachedKeyInfo( isDownloadComplete = false ) - @Transient - val fileName: String = "$id.zip" + val fileName: String + get() = "$id.zip" fun toDownloadUpdate(etag: String): DownloadUpdate = DownloadUpdate( id = id, diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/warning/storage/TraceWarningPackageMetadata.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/warning/storage/TraceWarningPackageMetadata.kt index 54725874a..6fa72600b 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/warning/storage/TraceWarningPackageMetadata.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/presencetracing/warning/storage/TraceWarningPackageMetadata.kt @@ -31,8 +31,8 @@ data class TraceWarningPackageMetadata( createdAt = createdAt, ) - @Transient - val fileName: String = "$packageId.bin" + val fileName: String + get() = "$packageId.bin" companion object { fun calcluateId( diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt index fd88abd78..4f026681c 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/storage/CoronaTestStorageTest.kt @@ -3,6 +3,7 @@ package de.rki.coronawarnapp.coronatest.storage import android.content.Context import androidx.core.content.edit import de.rki.coronawarnapp.coronatest.server.CoronaTestResult +import de.rki.coronawarnapp.coronatest.type.CoronaTest import de.rki.coronawarnapp.coronatest.type.pcr.PCRCoronaTest import de.rki.coronawarnapp.coronatest.type.rapidantigen.RACoronaTest import de.rki.coronawarnapp.util.serialization.SerializationModule @@ -114,10 +115,13 @@ class CoronaTestStorageTest : BaseTest() { ] """.toComparableJsonPretty() - instance.coronaTests.single() shouldBe pcrTest.copy( - lastError = null, - isProcessing = false - ) + instance.coronaTests.single().apply { + this shouldBe pcrTest.copy( + lastError = null, + isProcessing = false + ) + type shouldBe CoronaTest.Type.PCR + } } @Test @@ -153,10 +157,13 @@ class CoronaTestStorageTest : BaseTest() { ] """.toComparableJsonPretty() - instance.coronaTests.single() shouldBe raTest.copy( - lastError = null, - isProcessing = false - ) + instance.coronaTests.single().apply { + this shouldBe raTest.copy( + lastError = null, + isProcessing = false + ) + type shouldBe CoronaTest.Type.RAPID_ANTIGEN + } } @Test -- GitLab