Add decodeFromStringOrNull for safer disk JSON parsing (#873)

This commit is contained in:
Brian Yencho 2024-01-30 13:16:12 -06:00 committed by Álison Fernandes
parent 4807005428
commit 94f532b9d2
7 changed files with 92 additions and 17 deletions

View file

@ -7,6 +7,7 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.BaseEncryptedDiskSource
import com.x8bit.bitwarden.data.platform.datasource.disk.BaseEncryptedDiskSource.Companion.ENCRYPTED_BASE_KEY
import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacySecureStorageMigrator
import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
import com.x8bit.bitwarden.data.platform.util.decodeFromStringOrNull
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
@ -82,7 +83,7 @@ class AuthDiskSourceImpl(
}
override var userState: UserStateJson?
get() = getString(key = STATE_KEY)?.let { json.decodeFromString(it) }
get() = getString(key = STATE_KEY)?.let { json.decodeFromStringOrNull(it) }
set(value) {
putString(
key = STATE_KEY,
@ -229,7 +230,7 @@ class AuthDiskSourceImpl(
override fun getOrganizationKeys(userId: String): Map<String, String>? =
getString(key = "${ORGANIZATION_KEYS_KEY}_$userId")
?.let { json.decodeFromString(it) }
?.let { json.decodeFromStringOrNull(it) }
override fun storeOrganizationKeys(
userId: String,
@ -247,9 +248,9 @@ class AuthDiskSourceImpl(
getString(key = "${ORGANIZATIONS_KEY}_$userId")
?.let {
// The organizations are stored as a map
val organizationMap: Map<String, SyncResponseJson.Profile.Organization> =
json.decodeFromString(it)
organizationMap.values.toList()
val organizationMap: Map<String, SyncResponseJson.Profile.Organization>? =
json.decodeFromStringOrNull(it)
organizationMap?.values?.toList()
}
override fun getOrganizationsFlow(
@ -284,9 +285,9 @@ class AuthDiskSourceImpl(
getString(key = "${POLICIES_KEY}_$userId")
?.let {
// The policies are stored as a map.
val policiesMap: Map<String, SyncResponseJson.Policy> =
json.decodeFromString(it)
policiesMap.values.toList()
val policiesMap: Map<String, SyncResponseJson.Policy>? =
json.decodeFromStringOrNull(it)
policiesMap?.values?.toList()
}
override fun getPoliciesFlow(

View file

@ -4,6 +4,7 @@ import android.content.SharedPreferences
import com.x8bit.bitwarden.data.auth.datasource.disk.model.EnvironmentUrlDataJson
import com.x8bit.bitwarden.data.platform.datasource.disk.BaseDiskSource.Companion.BASE_KEY
import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
import com.x8bit.bitwarden.data.platform.util.decodeFromStringOrNull
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.onSubscription
import kotlinx.serialization.encodeToString
@ -20,7 +21,7 @@ class EnvironmentDiskSourceImpl(
) : BaseDiskSource(sharedPreferences = sharedPreferences),
EnvironmentDiskSource {
override var preAuthEnvironmentUrlData: EnvironmentUrlDataJson?
get() = getString(key = PRE_AUTH_URLS_KEY)?.let { json.decodeFromString(it) }
get() = getString(key = PRE_AUTH_URLS_KEY)?.let { json.decodeFromStringOrNull(it) }
set(value) {
putString(
key = PRE_AUTH_URLS_KEY,

View file

@ -5,6 +5,7 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.BaseDiskSource.Companio
import com.x8bit.bitwarden.data.platform.repository.model.UriMatchType
import com.x8bit.bitwarden.data.platform.repository.model.VaultTimeoutAction
import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
import com.x8bit.bitwarden.data.platform.util.decodeFromStringOrNull
import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppLanguage
import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppTheme
import kotlinx.coroutines.flow.Flow
@ -265,7 +266,7 @@ class SettingsDiskSourceImpl(
override fun getBlockedAutofillUris(userId: String): List<String>? =
getString(key = "${BLOCKED_AUTOFILL_URIS_KEY}_$userId")?.let {
json.decodeFromString(it)
json.decodeFromStringOrNull(it)
}
override fun storeBlockedAutofillUris(

View file

@ -1,6 +1,7 @@
package com.x8bit.bitwarden.data.platform.datasource.network.util
import com.x8bit.bitwarden.data.platform.datasource.network.model.BitwardenError
import com.x8bit.bitwarden.data.platform.util.decodeFromStringOrNull
import kotlinx.serialization.json.Json
import retrofit2.HttpException
@ -21,11 +22,7 @@ inline fun <reified T> BitwardenError.parseErrorBodyOrNull(codes: List<Int>, jso
?.takeIf { codes.any { it == this.code } }
?.responseBodyString
?.let { responseBody ->
try {
json.decodeFromString(responseBody)
} catch (_: Exception) {
null
}
json.decodeFromStringOrNull(responseBody)
}
/**

View file

@ -0,0 +1,19 @@
package com.x8bit.bitwarden.data.platform.util
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
/**
* Attempts to decode the given JSON [string] into the given type [T]. If there is an error in
* processing the JSON or deserializing it to an instance of [T], `null` will be returned.
*/
inline fun <reified T> Json.decodeFromStringOrNull(
string: String,
): T? =
try {
decodeFromString(string = string)
} catch (e: SerializationException) {
null
} catch (e: IllegalStateException) {
null
}

View file

@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.tools.generator.datasource.disk
import android.content.SharedPreferences
import com.x8bit.bitwarden.data.platform.datasource.disk.BaseDiskSource
import com.x8bit.bitwarden.data.platform.util.decodeFromStringOrNull
import com.x8bit.bitwarden.data.tools.generator.repository.model.PasscodeGenerationOptions
import com.x8bit.bitwarden.data.tools.generator.repository.model.UsernameGenerationOptions
import kotlinx.serialization.encodeToString
@ -26,7 +27,7 @@ class GeneratorDiskSourceImpl(
override fun getPasscodeGenerationOptions(userId: String): PasscodeGenerationOptions? {
val key = getPasswordGenerationOptionsKey(userId)
return getString(key)?.let { json.decodeFromString(it) }
return getString(key)?.let { json.decodeFromStringOrNull(it) }
}
override fun storePasscodeGenerationOptions(
@ -45,7 +46,7 @@ class GeneratorDiskSourceImpl(
override fun getUsernameGenerationOptions(userId: String): UsernameGenerationOptions? {
val key = getUsernameGenerationOptionsKey(userId)
return getString(key)?.let { json.decodeFromString(it) }
return getString(key)?.let { json.decodeFromStringOrNull(it) }
}
override fun storeUsernameGenerationOptions(

View file

@ -0,0 +1,55 @@
package com.x8bit.bitwarden.data.platform.util
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Test
class JsonExtensionsTest {
private val json = Json
@Test
fun `decodeFromStringOrNull for invalid JSON should return null`() {
assertNull(
json.decodeFromStringOrNull<TestData>(
"""
{]
""",
),
)
}
@Test
fun `decodeFromStringOrNull for valid JSON but an incorrect model should return null`() {
assertNull(
json.decodeFromStringOrNull<TestData>(
"""
{}
""",
),
)
}
@Test
fun `decodeFromStringOrNull for valid JSON and a correct model should parse correctly`() {
assertEquals(
TestData(
data = "test",
),
json.decodeFromStringOrNull<TestData>(
"""
{
"data": "test"
}
""",
),
)
}
}
@Serializable
private data class TestData(
@SerialName("data") val data: String,
)