From ca62b6a71efdbe5a5cd61527f6b4e1b5558f8aa6 Mon Sep 17 00:00:00 2001 From: "Sergey.Shanshin" Date: Fri, 27 Jan 2023 18:36:40 +0100 Subject: [PATCH 1/3] Added support for null values for nullable enums in lanient mode Fixed #2170 --- .../json/JsonCoerceInputValuesTest.kt | 15 ++++++++++++++- .../serialization/json/internal/JsonNamesMap.kt | 5 +++++ .../json/internal/StreamingJsonDecoder.kt | 1 + .../json/internal/TreeJsonDecoder.kt | 1 + .../json/internal/lexer/AbstractJsonLexer.kt | 10 ++++++++-- .../json/internal/DynamicDecoders.kt | 1 + 6 files changed, 30 insertions(+), 3 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 fd8d516c78..ecb946cb72 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonCoerceInputValuesTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonCoerceInputValuesTest.kt @@ -5,7 +5,6 @@ package kotlinx.serialization.json import kotlinx.serialization.* -import kotlinx.serialization.json.internal.* import kotlinx.serialization.test.assertFailsWithSerial import kotlin.test.* @@ -25,6 +24,11 @@ class JsonCoerceInputValuesTest : JsonTestBase() { val foo: String ) + @Serializable + data class NullableEnumHolder( + val enum: SampleEnum? + ) + val json = Json { coerceInputValues = true isLenient = true @@ -99,4 +103,13 @@ class JsonCoerceInputValuesTest : JsonTestBase() { assertEquals(expected, json.decodeFromString(MultipleValues.serializer(), input), "Failed on input: $input") } } + + @Test + fun testNullSupportForEnums() = parametrizedTest(json) { + var decoded = decodeFromString("""{"enum": null}""") + assertNull(decoded.enum) + + decoded = decodeFromString("""{"enum": OptionA}""") + assertEquals(SampleEnum.OptionA, decoded.enum) + } } 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 bf616f98e3..5e3969855d 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt @@ -99,11 +99,16 @@ internal fun SerialDescriptor.getJsonNameIndexOrThrow(json: Json, name: String, internal inline fun Json.tryCoerceValue( elementDescriptor: SerialDescriptor, peekNull: () -> Boolean, + tryPeekNull: () -> Boolean, peekString: () -> String?, onEnumCoercing: () -> Unit = {} ): Boolean { if (!elementDescriptor.isNullable && peekNull()) return true if (elementDescriptor.kind == SerialKind.ENUM) { + if (elementDescriptor.isNullable && tryPeekNull()) { + return false + } + val enumValue = peekString() ?: return false // if value is not a string, decodeEnum() will throw correct exception val enumIndex = elementDescriptor.getJsonNameIndex(this, enumValue) 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 c7648ad6d2..6dd2277e08 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt @@ -209,6 +209,7 @@ internal open class StreamingJsonDecoder( private fun coerceInputValue(descriptor: SerialDescriptor, index: Int): Boolean = json.tryCoerceValue( descriptor.getElementDescriptor(index), { !lexer.tryConsumeNotNull() }, + { !lexer.tryConsumeNotNull(false) }, { 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 9e0cb2ad5f..cbb599ef95 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt @@ -196,6 +196,7 @@ private open class JsonTreeDecoder( json.tryCoerceValue( descriptor.getElementDescriptor(index), { currentElement(tag) is JsonNull }, + { 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 977347a55c..cfbb579fc0 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 @@ -246,8 +246,11 @@ internal abstract class AbstractJsonLexer { * Tries to consume `null` token from input. * Returns `true` if the next 4 chars in input are not `null`, * `false` otherwise and consumes it. + * + * If [moveForward] is true and if the next 4 chars in input are `null` then + * current position in lexer move forward on these 4 chars. */ - fun tryConsumeNotNull(): Boolean { + fun tryConsumeNotNull(moveForward: Boolean = true): Boolean { var current = skipWhitespaces() current = prefetchOrEof(current) // Cannot consume null due to EOF, maybe something else @@ -261,7 +264,10 @@ internal abstract class AbstractJsonLexer { * distinguish it from 'null' */ if (len > 4 && charToTokenClass(source[current + 4]) == TC_OTHER) return true - currentPosition = current + 4 + + if (moveForward) { + currentPosition = current + 4 + } return false } 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 1447881c16..847211cea8 100644 --- a/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt +++ b/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt @@ -75,6 +75,7 @@ private open class DynamicInput( json.tryCoerceValue( descriptor.getElementDescriptor(index), { getByTag(tag) == null }, + { getByTag(tag) == null }, { getByTag(tag) as? String } ) From b002e16cb915eb619f2c1f657062718d9fa29ea5 Mon Sep 17 00:00:00 2001 From: "Sergey.Shanshin" Date: Mon, 6 Feb 2023 14:20:37 +0100 Subject: [PATCH 2/3] ~review fixes --- .../serialization/json/internal/JsonNamesMap.kt | 7 +++---- .../json/internal/StreamingJsonDecoder.kt | 5 ++--- .../json/internal/TreeJsonDecoder.kt | 1 - .../json/internal/lexer/AbstractJsonLexer.kt | 17 +++++++---------- .../json/internal/DynamicDecoders.kt | 1 - 5 files changed, 12 insertions(+), 19 deletions(-) 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 5e3969855d..762bacd9ec 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt @@ -98,14 +98,13 @@ internal fun SerialDescriptor.getJsonNameIndexOrThrow(json: Json, name: String, @OptIn(ExperimentalSerializationApi::class) internal inline fun Json.tryCoerceValue( elementDescriptor: SerialDescriptor, - peekNull: () -> Boolean, - tryPeekNull: () -> Boolean, + peekNull: (consume: Boolean) -> Boolean, peekString: () -> String?, onEnumCoercing: () -> Unit = {} ): Boolean { - if (!elementDescriptor.isNullable && peekNull()) return true + if (!elementDescriptor.isNullable && peekNull(true)) return true if (elementDescriptor.kind == SerialKind.ENUM) { - if (elementDescriptor.isNullable && tryPeekNull()) { + if (elementDescriptor.isNullable && peekNull(false)) { return 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 6dd2277e08..627ee7bbf8 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt @@ -133,7 +133,7 @@ internal open class StreamingJsonDecoder( } override fun decodeNotNullMark(): Boolean { - return !(elementMarker?.isUnmarkedNull ?: false) && lexer.tryConsumeNotNull() + return !(elementMarker?.isUnmarkedNull ?: false) && !lexer.tryConsumeNull() } override fun decodeNull(): Nothing? { @@ -208,8 +208,7 @@ internal open class StreamingJsonDecoder( */ private fun coerceInputValue(descriptor: SerialDescriptor, index: Int): Boolean = json.tryCoerceValue( descriptor.getElementDescriptor(index), - { !lexer.tryConsumeNotNull() }, - { !lexer.tryConsumeNotNull(false) }, + { 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 cbb599ef95..9e0cb2ad5f 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt @@ -196,7 +196,6 @@ private open class JsonTreeDecoder( json.tryCoerceValue( descriptor.getElementDescriptor(index), { currentElement(tag) is JsonNull }, - { 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 cfbb579fc0..a5b4880ecd 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 @@ -245,30 +245,27 @@ internal abstract class AbstractJsonLexer { /** * Tries to consume `null` token from input. * Returns `true` if the next 4 chars in input are not `null`, - * `false` otherwise and consumes it. - * - * If [moveForward] is true and if the next 4 chars in input are `null` then - * current position in lexer move forward on these 4 chars. + * `false` otherwise and consumes it if [doConsume] is `true`. */ - fun tryConsumeNotNull(moveForward: Boolean = true): Boolean { + fun tryConsumeNull(doConsume: Boolean = true): Boolean { var current = skipWhitespaces() current = prefetchOrEof(current) // Cannot consume null due to EOF, maybe something else val len = source.length - current - if (len < 4 || current == -1) return true + if (len < 4 || current == -1) return false for (i in 0..3) { - if (NULL[i] != source[current + i]) return true + if (NULL[i] != source[current + i]) return false } /* * If we're in lenient mode, this might be the string with 'null' prefix, * distinguish it from 'null' */ - if (len > 4 && charToTokenClass(source[current + 4]) == TC_OTHER) return true + if (len > 4 && charToTokenClass(source[current + 4]) == TC_OTHER) return false - if (moveForward) { + if (doConsume) { currentPosition = current + 4 } - return false + return true } open fun skipWhitespaces(): Int { 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 847211cea8..1447881c16 100644 --- a/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt +++ b/formats/json/jsMain/src/kotlinx/serialization/json/internal/DynamicDecoders.kt @@ -75,7 +75,6 @@ private open class DynamicInput( json.tryCoerceValue( descriptor.getElementDescriptor(index), { getByTag(tag) == null }, - { getByTag(tag) == null }, { getByTag(tag) as? String } ) From dfe617e3bd9b7e6b8be7cf882af3be2d4eec8c8a Mon Sep 17 00:00:00 2001 From: Sergey Shanshin Date: Mon, 6 Feb 2023 16:43:35 +0300 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Leonid Startsev --- .../serialization/json/internal/lexer/AbstractJsonLexer.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 a5b4880ecd..ce81f19957 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 @@ -244,8 +244,8 @@ internal abstract class AbstractJsonLexer { /** * Tries to consume `null` token from input. - * Returns `true` if the next 4 chars in input are not `null`, - * `false` otherwise and consumes it if [doConsume] is `true`. + * Returns `false` if the next 4 chars in input are not `null`, + * `true` otherwise and consumes it if [doConsume] is `true`. */ fun tryConsumeNull(doConsume: Boolean = true): Boolean { var current = skipWhitespaces()