Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not try to coerce input values for properties #2530

Merged
merged 2 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/basic-serialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
```

<!--- TEST LINES_START -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -112,4 +122,24 @@ class JsonCoerceInputValuesTest : JsonTestBase() {
decoded = decodeFromString<NullableEnumHolder>("""{"enum": OptionA}""")
assertEquals(SampleEnum.OptionA, decoded.enum)
}

@Test
fun propertiesWithoutDefaultValuesDoNotChangeErrorMsg() {
val json2 = Json(json) { coerceInputValues = false }
parametrizedTest { mode ->
val e1 = assertFailsWith<SerializationException>() { json.decodeFromString<Uncoercable>("""{"s":null}""", mode) }
val e2 = assertFailsWith<SerializationException>() { json2.decodeFromString<Uncoercable>("""{"s":null}""", mode) }
assertEquals(e2.message, e1.message)
}
}

@Test
fun propertiesWithoutDefaultValuesDoNotChangeErrorMsgEnum() {
val json2 = Json(json) { coerceInputValues = false }
parametrizedTest { mode ->
val e1 = assertFailsWith<SerializationException> { json.decodeFromString<UncoercableEnum>("""{"e":"UNEXPECTED"}""", mode) }
val e2 = assertFailsWith<SerializationException> { json2.decodeFromString<UncoercableEnum>("""{"e":"UNEXPECTED"}""", mode) }
assertEquals(e2.message, e1.message)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M.b. if specified?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if specified may be incorrectly read as if coerceInputValues = true is specified

* 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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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*/ }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
)
Expand Down
2 changes: 1 addition & 1 deletion guide/test/BasicSerializationTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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."
)
}

Expand Down