From f79544035a2d95cd07fac3e1dc353230cc1153ba Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Mon, 16 Oct 2023 12:22:46 +0200 Subject: [PATCH] Add a flag to allow parser to accept trailing commas. This is one of the popular community requests and one of the main reasons people ask for Json5 support. Implementing this flag separately will allow alleviating large paint points quickly without a need to wait for full Json5 support. Fixes #1812 Relates to: #797, #2221 --- .../json/JsonErrorMessagesTest.kt | 8 +- .../serialization/json/JsonParserTest.kt | 2 +- .../serialization/json/TrailingCommaTest.kt | 128 ++++++++++++++++++ .../serialization/test/TestingFramework.kt | 6 + .../json/api/kotlinx-serialization-json.api | 3 + .../src/kotlinx/serialization/json/Json.kt | 12 +- .../serialization/json/JsonConfiguration.kt | 6 +- .../json/internal/JsonExceptions.kt | 7 + .../json/internal/JsonTreeReader.kt | 9 +- .../json/internal/StreamingJsonDecoder.kt | 9 +- .../json/internal/lexer/AbstractJsonLexer.kt | 2 +- 11 files changed, 173 insertions(+), 19 deletions(-) create mode 100644 formats/json-tests/commonTest/src/kotlinx/serialization/json/TrailingCommaTest.kt diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt index 8c16ac01f2..08d1eefd7d 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonErrorMessagesTest.kt @@ -6,6 +6,7 @@ package kotlinx.serialization.json import kotlinx.serialization.* +import kotlinx.serialization.test.* import kotlin.test.* @@ -155,11 +156,4 @@ class JsonErrorMessagesTest : JsonTestBase() { }) } - - private fun checkSerializationException(action: () -> Unit, assertions: SerializationException.(String) -> Unit) { - val e = assertFailsWith(SerializationException::class, action) - assertNotNull(e.message) - e.assertions(e.message!!) - } - } diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonParserTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonParserTest.kt index 7f9044f9de..94f7052cd4 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonParserTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonParserTest.kt @@ -83,7 +83,7 @@ class JsonParserTest : JsonTestBase() { } private fun testTrailingComma(content: String) { - assertFailsWithSerialMessage("JsonDecodingException", "Unexpected trailing") { Json.parseToJsonElement(content) } + assertFailsWithSerialMessage("JsonDecodingException", "Trailing comma before the end of JSON object") { Json.parseToJsonElement(content) } } @Test diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/TrailingCommaTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/TrailingCommaTest.kt new file mode 100644 index 0000000000..0916de5795 --- /dev/null +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/TrailingCommaTest.kt @@ -0,0 +1,128 @@ +/* + * Copyright 2017-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.serialization.json + +import kotlinx.serialization.* +import kotlinx.serialization.test.* +import kotlin.test.* + +class TrailingCommaTest : JsonTestBase() { + val tj = Json { allowTrailingComma = true } + + @Serializable + data class Optional(val data: String = "") + + @Serializable + data class MultipleFields(val a: String, val b: String, val c: String) + + private val multipleFields = MultipleFields("1", "2", "3") + + @Serializable + data class WithMap(val m: Map) + + private val withMap = WithMap(mapOf("a" to "1", "b" to "2", "c" to "3")) + + @Serializable + data class WithList(val l: List) + + private val withList = WithList(listOf(1, 2, 3)) + + @Test + fun basic() = parametrizedTest { mode -> + val sd = """{"data":"str",}""" + assertEquals(Optional("str"), tj.decodeFromString(sd, mode)) + } + + @Test + fun trailingCommaNotAllowedByDefaultForObjects() = parametrizedTest { mode -> + val sd = """{"data":"str",}""" + checkSerializationException({ + default.decodeFromString(sd, mode) + }, { message -> + assertContains( + message, + """Unexpected JSON token at offset 13: Trailing comma before the end of JSON object""" + ) + }) + } + + @Test + fun trailingCommaNotAllowedByDefaultForLists() = parametrizedTest { mode -> + val sd = """{"l":[1,]}""" + checkSerializationException({ + default.decodeFromString(sd, mode) + }, { message -> + assertContains( + message, + """Unexpected JSON token at offset 7: Trailing comma before the end of JSON array""" + ) + }) + } + + @Test + fun trailingCommaNotAllowedByDefaultForMaps() = parametrizedTest { mode -> + val sd = """{"m":{"a": "b",}}""" + checkSerializationException({ + default.decodeFromString(sd, mode) + }, { message -> + assertContains( + message, + """Unexpected JSON token at offset 14: Trailing comma before the end of JSON object""" + ) + }) + } + + @Test + fun emptyObjectNotAllowed() = parametrizedTest { mode -> + assertFailsWithMessage("Unexpected leading comma") { + tj.decodeFromString("""{,}""", mode) + } + } + + @Test + fun emptyListNotAllowed() = parametrizedTest { mode -> + assertFailsWithMessage("Unexpected leading comma") { + tj.decodeFromString("""{"l":[,]}""", mode) + } + } + + @Test + fun emptyMapNotAllowed() = parametrizedTest { mode -> + assertFailsWithMessage("Unexpected leading comma") { + tj.decodeFromString("""{"m":{,}}""", mode) + } + } + + @Test + fun testMultipleFields() = parametrizedTest { mode -> + val input = """{"a":"1","b":"2","c":"3", }""" + assertEquals(multipleFields, tj.decodeFromString(input, mode)) + } + + @Test + fun testWithMap() = parametrizedTest { mode -> + val input = """{"m":{"a":"1","b":"2","c":"3", }}""" + + assertEquals(withMap, tj.decodeFromString(input, mode)) + } + + @Test + fun testWithList() = parametrizedTest { mode -> + val input = """{"l":[1, 2, 3, ]}""" + assertEquals(withList, tj.decodeFromString(input, mode)) + } + + @Serializable + data class Mixed(val mf: MultipleFields, val wm: WithMap, val wl: WithList) + + @Test + fun testMixed() = parametrizedTest { mode -> + //language=JSON5 + val input = """{"mf":{"a":"1","b":"2","c":"3",}, + "wm":{"m":{"a":"1","b":"2","c":"3",},}, + "wl":{"l":[1, 2, 3,],},}""" + assertEquals(Mixed(multipleFields, withMap, withList), tj.decodeFromString(input, mode)) + } +} diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/test/TestingFramework.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/test/TestingFramework.kt index e941f04745..3ec0714980 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/test/TestingFramework.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/test/TestingFramework.kt @@ -92,3 +92,9 @@ inline fun assertFailsWithMessage( "expected:<$message> but was:<${exception.message}>" ) } + +inline fun checkSerializationException(action: () -> Unit, assertions: SerializationException.(String) -> Unit) { + val e = assertFailsWith(SerializationException::class, action) + assertNotNull(e.message) + e.assertions(e.message!!) +} diff --git a/formats/json/api/kotlinx-serialization-json.api b/formats/json/api/kotlinx-serialization-json.api index bdc6bdff5f..649cce0ec3 100644 --- a/formats/json/api/kotlinx-serialization-json.api +++ b/formats/json/api/kotlinx-serialization-json.api @@ -87,6 +87,7 @@ public final class kotlinx/serialization/json/JsonArraySerializer : kotlinx/seri public final class kotlinx/serialization/json/JsonBuilder { public final fun getAllowSpecialFloatingPointValues ()Z public final fun getAllowStructuredMapKeys ()Z + public final fun getAllowTrailingComma ()Z public final fun getClassDiscriminator ()Ljava/lang/String; public final fun getCoerceInputValues ()Z public final fun getDecodeEnumsCaseInsensitive ()Z @@ -102,6 +103,7 @@ public final class kotlinx/serialization/json/JsonBuilder { public final fun isLenient ()Z public final fun setAllowSpecialFloatingPointValues (Z)V public final fun setAllowStructuredMapKeys (Z)V + public final fun setAllowTrailingComma (Z)V public final fun setClassDiscriminator (Ljava/lang/String;)V public final fun setCoerceInputValues (Z)V public final fun setDecodeEnumsCaseInsensitive (Z)V @@ -130,6 +132,7 @@ public final class kotlinx/serialization/json/JsonConfiguration { public fun ()V public final fun getAllowSpecialFloatingPointValues ()Z public final fun getAllowStructuredMapKeys ()Z + public final fun getAllowTrailingComma ()Z public final fun getClassDiscriminator ()Ljava/lang/String; public final fun getCoerceInputValues ()Z public final fun getDecodeEnumsCaseInsensitive ()Z diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt index be7a7f0805..26c376ef67 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt @@ -364,6 +364,16 @@ public class JsonBuilder internal constructor(json: Json) { @ExperimentalSerializationApi public var decodeEnumsCaseInsensitive: Boolean = json.configuration.decodeEnumsCaseInsensitive + /** + * Allows parser to accept trailing (ending) commas in JSON objects and arrays, + * making inputs like `[1, 2, 3,]` valid. + * + * Does not affect encoding. + * `false` by default. + */ + @ExperimentalSerializationApi + public var allowTrailingComma: Boolean = json.configuration.allowTrailingComma + /** * Module with contextual and polymorphic serializers to be used in the resulting [Json] instance. * @@ -396,7 +406,7 @@ public class JsonBuilder internal constructor(json: Json) { allowStructuredMapKeys, prettyPrint, explicitNulls, prettyPrintIndent, coerceInputValues, useArrayPolymorphism, classDiscriminator, allowSpecialFloatingPointValues, useAlternativeNames, - namingStrategy, decodeEnumsCaseInsensitive + namingStrategy, decodeEnumsCaseInsensitive, allowTrailingComma ) } } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt b/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt index ea653a6478..053f4cd6de 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt @@ -32,7 +32,9 @@ public class JsonConfiguration @OptIn(ExperimentalSerializationApi::class) inter @ExperimentalSerializationApi public val namingStrategy: JsonNamingStrategy? = null, @ExperimentalSerializationApi - public val decodeEnumsCaseInsensitive: Boolean = false + public val decodeEnumsCaseInsensitive: Boolean = false, + @ExperimentalSerializationApi + public val allowTrailingComma: Boolean = false, ) { /** @suppress Dokka **/ @@ -42,6 +44,6 @@ public class JsonConfiguration @OptIn(ExperimentalSerializationApi::class) inter "allowStructuredMapKeys=$allowStructuredMapKeys, prettyPrint=$prettyPrint, explicitNulls=$explicitNulls, " + "prettyPrintIndent='$prettyPrintIndent', coerceInputValues=$coerceInputValues, useArrayPolymorphism=$useArrayPolymorphism, " + "classDiscriminator='$classDiscriminator', allowSpecialFloatingPointValues=$allowSpecialFloatingPointValues, useAlternativeNames=$useAlternativeNames, " + - "namingStrategy=$namingStrategy, decodeEnumsCaseInsensitive=$decodeEnumsCaseInsensitive)" + "namingStrategy=$namingStrategy, decodeEnumsCaseInsensitive=$decodeEnumsCaseInsensitive, allowTrailingComma=$allowTrailingComma)" } } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonExceptions.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonExceptions.kt index ad5011a937..2eaa377e44 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonExceptions.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonExceptions.kt @@ -46,6 +46,13 @@ internal fun AbstractJsonLexer.throwInvalidFloatingPointDecoded(result: Number): hint = specialFlowingValuesHint) } +internal fun AbstractJsonLexer.invalidTrailingComma(entity: String = "object"): Nothing { + fail("Trailing comma before the end of JSON $entity", + position = currentPosition - 1, + hint = "Trailing commas are non-complaint JSON and not allowed by default. Use 'allowTrailingCommas = true' in 'Json {}' builder to support them." + ) +} + @OptIn(ExperimentalSerializationApi::class) internal fun InvalidKeyKindException(keyDescriptor: SerialDescriptor) = JsonEncodingException( "Value of type '${keyDescriptor.serialName}' can't be used in JSON as a key in the map. " + diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonTreeReader.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonTreeReader.kt index 060c36bd2d..9cb9bb3c56 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonTreeReader.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonTreeReader.kt @@ -13,6 +13,7 @@ internal class JsonTreeReader( private val lexer: AbstractJsonLexer ) { private val isLenient = configuration.isLenient + private val trailingCommaAllowed = configuration.allowTrailingComma private var stackDepth = 0 private fun readObject(): JsonElement = readObjectImpl { @@ -44,8 +45,9 @@ internal class JsonTreeReader( if (lastToken == TC_BEGIN_OBJ) { // Case of empty object lexer.consumeNextToken(TC_END_OBJ) } else if (lastToken == TC_COMMA) { // Trailing comma - lexer.fail("Unexpected trailing comma") - } + if (!trailingCommaAllowed) lexer.invalidTrailingComma() + lexer.consumeNextToken(TC_END_OBJ) + } // else unexpected token? return JsonObject(result) } @@ -66,7 +68,8 @@ internal class JsonTreeReader( if (lastToken == TC_BEGIN_LIST) { // Case of empty object lexer.consumeNextToken(TC_END_LIST) } else if (lastToken == TC_COMMA) { // Trailing comma - lexer.fail("Unexpected trailing comma") + if (!trailingCommaAllowed) lexer.invalidTrailingComma("array") + lexer.consumeNextToken(TC_END_LIST) } return JsonArray(result) } 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 4e373f1fb2..ca79e152ee 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt @@ -122,6 +122,7 @@ internal open class StreamingJsonDecoder( if (json.configuration.ignoreUnknownKeys && descriptor.elementsCount == 0) { skipLeftoverElements(descriptor) } + if (lexer.tryConsumeComma() && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma("") // First consume the object so we know it's correct lexer.consumeNextToken(mode.end) // Then cleanup the path @@ -195,12 +196,12 @@ internal open class StreamingJsonDecoder( return if (lexer.canConsumeValue()) { if (decodingKey) { - if (currentIndex == -1) lexer.require(!hasComma) { "Unexpected trailing comma" } + if (currentIndex == -1) lexer.require(!hasComma) { "Unexpected leading comma" } else lexer.require(hasComma) { "Expected comma after the key-value pair" } } ++currentIndex } else { - if (hasComma) lexer.fail("Expected '}', but had ',' instead") + if (hasComma && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma() CompositeDecoder.DECODE_DONE } } @@ -239,7 +240,7 @@ internal open class StreamingJsonDecoder( hasComma = handleUnknown(key) } } - if (hasComma) lexer.fail("Unexpected trailing comma") + if (hasComma && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma() return elementMarker?.nextUnmarkedIndex() ?: CompositeDecoder.DECODE_DONE } @@ -262,7 +263,7 @@ internal open class StreamingJsonDecoder( if (currentIndex != -1 && !hasComma) lexer.fail("Expected end of the array or comma") ++currentIndex } else { - if (hasComma) lexer.fail("Unexpected trailing comma") + if (hasComma && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma("array") CompositeDecoder.DECODE_DONE } } 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 a1a6a5eed1..c83bdef978 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 @@ -149,7 +149,7 @@ internal abstract class AbstractJsonLexer { protected abstract val source: CharSequence @JvmField - protected var currentPosition: Int = 0 // position in source + internal var currentPosition: Int = 0 // position in source @JvmField val path = JsonPath()