From 1b93d78c9eec4ad7f7b063e45a07235265685a58 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Fri, 15 Dec 2023 18:54:46 +0100 Subject: [PATCH 1/2] Do not try to coerce input values for properties that do not have default values Trying so leads to confusing errors about missing values despite a json key actually present in the input. Fixes #2529 --- .../json/JsonCoerceInputValuesTest.kt | 30 +++++++++++++++++++ .../src/kotlinx/serialization/json/Json.kt | 2 +- .../json/internal/JsonNamesMap.kt | 5 +++- .../json/internal/StreamingJsonDecoder.kt | 2 +- .../json/internal/TreeJsonDecoder.kt | 2 +- .../json/internal/lexer/AbstractJsonLexer.kt | 2 +- .../json/internal/DynamicDecoders.kt | 2 +- 7 files changed, 39 insertions(+), 6 deletions(-) diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonCoerceInputValuesTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonCoerceInputValuesTest.kt index ecb946cb72..3d7c332271 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonCoerceInputValuesTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonCoerceInputValuesTest.kt @@ -29,6 +29,16 @@ class JsonCoerceInputValuesTest : JsonTestBase() { val enum: SampleEnum? ) + @Serializable + class Uncoercable( + val s: String + ) + + @Serializable + class UncoercableEnum( + val e: SampleEnum + ) + val json = Json { coerceInputValues = true isLenient = true @@ -112,4 +122,24 @@ class JsonCoerceInputValuesTest : JsonTestBase() { decoded = decodeFromString("""{"enum": OptionA}""") assertEquals(SampleEnum.OptionA, decoded.enum) } + + @Test + fun propertiesWithoutDefaultValuesDoNotChangeErrorMsg() { + val json2 = Json(json) { coerceInputValues = false } + parametrizedTest { mode -> + val e1 = assertFailsWith() { json.decodeFromString("""{"s":null}""", mode) } + val e2 = assertFailsWith() { json2.decodeFromString("""{"s":null}""", mode) } + assertEquals(e2.message, e1.message) + } + } + + @Test + fun propertiesWithoutDefaultValuesDoNotChangeErrorMsgEnum() { + val json2 = Json(json) { coerceInputValues = false } + parametrizedTest { mode -> + val e1 = assertFailsWith { json.decodeFromString("""{"e":"UNEXPECTED"}""", mode) } + val e2 = assertFailsWith { json2.decodeFromString("""{"e":"UNEXPECTED"}""", mode) } + assertEquals(e2.message, e1.message) + } + } } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt index 26c376ef67..a510e8a30c 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt @@ -287,7 +287,7 @@ public class JsonBuilder internal constructor(json: Json) { public var prettyPrintIndent: String = json.configuration.prettyPrintIndent /** - * Enables coercing incorrect JSON values to the default property value in the following cases: + * Enables coercing incorrect JSON values to the default property value (if exists) in the following cases: * 1. JSON value is `null` but the property type is non-nullable. * 2. Property type is an enum type, but JSON value contains unknown enum member. * diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt index 8acd8fc477..9128f3a276 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt @@ -110,11 +110,14 @@ internal fun SerialDescriptor.getJsonNameIndexOrThrow(json: Json, name: String, @OptIn(ExperimentalSerializationApi::class) internal inline fun Json.tryCoerceValue( - elementDescriptor: SerialDescriptor, + descriptor: SerialDescriptor, + index: Int, peekNull: (consume: Boolean) -> Boolean, peekString: () -> String?, onEnumCoercing: () -> Unit = {} ): Boolean { + if (!descriptor.isElementOptional(index)) return false + val elementDescriptor = descriptor.getElementDescriptor(index) if (!elementDescriptor.isNullable && peekNull(true)) return true if (elementDescriptor.kind == SerialKind.ENUM) { if (elementDescriptor.isNullable && peekNull(false)) { diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt index 0018fce107..caa1f4a52d 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt @@ -213,7 +213,7 @@ internal open class StreamingJsonDecoder( * Checks whether JSON has `null` value for non-null property or unknown enum value for enum property */ private fun coerceInputValue(descriptor: SerialDescriptor, index: Int): Boolean = json.tryCoerceValue( - descriptor.getElementDescriptor(index), + descriptor, index, { lexer.tryConsumeNull(it) }, { lexer.peekString(configuration.isLenient) }, { lexer.consumeString() /* skip unknown enum string*/ } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt index aedfb95f3e..690b35e1f7 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt @@ -190,7 +190,7 @@ private open class JsonTreeDecoder( */ private fun coerceInputValue(descriptor: SerialDescriptor, index: Int, tag: String): Boolean = json.tryCoerceValue( - descriptor.getElementDescriptor(index), + descriptor, index, { currentElement(tag) is JsonNull }, { (currentElement(tag) as? JsonPrimitive)?.contentOrNull } ) diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/lexer/AbstractJsonLexer.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/lexer/AbstractJsonLexer.kt index c83bdef978..f90ee1a0e3 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/lexer/AbstractJsonLexer.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/lexer/AbstractJsonLexer.kt @@ -11,7 +11,7 @@ import kotlin.jvm.* import kotlin.math.* internal const val lenientHint = "Use 'isLenient = true' in 'Json {}' builder to accept non-compliant JSON." -internal const val coerceInputValuesHint = "Use 'coerceInputValues = true' in 'Json {}' builder to coerce nulls to default values." +internal const val coerceInputValuesHint = "Use 'coerceInputValues = true' in 'Json {}' builder to coerce nulls if property has a default value." internal const val specialFlowingValuesHint = "It is possible to deserialize them using 'JsonBuilder.allowSpecialFloatingPointValues = true'" internal const val ignoreUnknownKeysHint = "Use 'ignoreUnknownKeys = true' in 'Json {}' builder to ignore unknown keys." diff --git a/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt b/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt index 86c7a85cbc..1ff1e40daa 100644 --- a/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt +++ b/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt @@ -73,7 +73,7 @@ private open class DynamicInput( private fun coerceInputValue(descriptor: SerialDescriptor, index: Int, tag: String): Boolean = json.tryCoerceValue( - descriptor.getElementDescriptor(index), + descriptor, index, { getByTag(tag) == null }, { getByTag(tag) as? String } ) From 3afaebc497e1e03cb5364feabcf7ec3d0c3f3f53 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Mon, 18 Dec 2023 12:06:48 +0100 Subject: [PATCH 2/2] fixup! Do not try to coerce input values for properties that do not have default values --- docs/basic-serialization.md | 2 +- guide/test/BasicSerializationTest.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/basic-serialization.md b/docs/basic-serialization.md index 3853376eee..96e709817d 100644 --- a/docs/basic-serialization.md +++ b/docs/basic-serialization.md @@ -534,7 +534,7 @@ the `null` value to it. ```text Exception in thread "main" kotlinx.serialization.json.internal.JsonDecodingException: Unexpected JSON token at offset 52: Expected string literal but 'null' literal was found at path: $.language -Use 'coerceInputValues = true' in 'Json {}' builder to coerce nulls to default values. +Use 'coerceInputValues = true' in 'Json {}' builder to coerce nulls if property has a default value. ``` diff --git a/guide/test/BasicSerializationTest.kt b/guide/test/BasicSerializationTest.kt index dc89feb69c..11f9e9f28f 100644 --- a/guide/test/BasicSerializationTest.kt +++ b/guide/test/BasicSerializationTest.kt @@ -110,7 +110,7 @@ class BasicSerializationTest { fun testExampleClasses12() { captureOutput("ExampleClasses12") { example.exampleClasses12.main() }.verifyOutputLinesStart( "Exception in thread \"main\" kotlinx.serialization.json.internal.JsonDecodingException: Unexpected JSON token at offset 52: Expected string literal but 'null' literal was found at path: $.language", - "Use 'coerceInputValues = true' in 'Json {}' builder to coerce nulls to default values." + "Use 'coerceInputValues = true' in 'Json {}' builder to coerce nulls if property has a default value." ) }