Disallow missing commas between fields (#2287 #2520)

formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonParserTest.kt

@@ -6,7 +6,6 @@ package kotlinx.serialization.json
 
 import kotlinx.serialization.*
 import kotlinx.serialization.builtins.*
-import kotlinx.serialization.json.internal.*
 import kotlinx.serialization.test.*
 import kotlin.test.*
 
@@ -79,11 +78,11 @@ class JsonParserTest : JsonTestBase() {
     fun testTrailingComma() {
         testTrailingComma("{\"id\":0,}")
         testTrailingComma("{\"id\":0  ,}")
-        testTrailingComma("{\"id\":0  , ,}")
+        testTrailingComma("{\"id\":0  , ,}", message = "Multiple consecutive commas are not allowed in JSON")
     }
 
-    private fun testTrailingComma(content: String) {
-        assertFailsWithSerialMessage("JsonDecodingException", "Trailing comma before the end of JSON object") {  Json.parseToJsonElement(content) }
+    private fun testTrailingComma(content: String, message: String = "Trailing comma before the end of JSON object") {
+        assertFailsWithSerialMessage("JsonDecodingException", message) { Json.parseToJsonElement(content) }
     }
 
     @Test

formats/json-tests/commonTest/src/kotlinx/serialization/json/MissingCommaTest.kt

@@ -0,0 +1,108 @@
+/*
+ * Copyright 2017-2024 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 MissingCommaTest : JsonTestBase() {
+    @Serializable
+    class Holder(
+        val i: Int,
+        val c: Child,
+    )
+
+    @Serializable
+    class Child(
+        val i: String
+    )
+
+    private val withTrailingComma = Json { allowTrailingComma = true }
+
+    @Test
+    fun missingCommaBetweenFieldsAfterPrimitive() {
+        val message =
+            "Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
+        val json = """{"i":42 "c":{"i":"string"}}"""
+
+        assertFailsWithSerialMessage("JsonDecodingException", message) {
+            default.decodeFromString<Holder>(json)
+        }
+    }
+
+    @Test
+    fun missingCommaBetweenFieldsAfterObject() {
+        val message =
+            "Unexpected JSON token at offset 19: Expected end of the object or comma at path: \$"
+        val json = """{"c":{"i":"string"}"i":42}"""
+
+        assertFailsWithSerialMessage("JsonDecodingException", message) {
+            default.decodeFromString<Holder>(json)
+        }
+    }
+
+    @Test
+    fun allowTrailingCommaDoesNotApplyToCommaBetweenFields() {
+        val message =
+            "Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
+        val json = """{"i":42 "c":{"i":"string"}}"""
+
+        assertFailsWithSerialMessage("JsonDecodingException", message) {
+            withTrailingComma.decodeFromString<Holder>(json)
+        }
+    }
+
+    @Test
+    fun lenientSerializeDoesNotAllowToSkipCommaBetweenFields() {
+        val message =
+            "Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
+        val json = """{"i":42 "c":{"i":"string"}}"""
+
+        assertFailsWithSerialMessage("JsonDecodingException", message) {
+            lenient.decodeFromString<Holder>(json)
+        }
+    }
+
+    @Test
+    fun missingCommaInDynamicMap() {
+        val m = "Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
+        val json = """{"i":42 "c":{"i":"string"}}"""
+        assertFailsWithSerialMessage("JsonDecodingException", m) {
+            default.parseToJsonElement(json)
+        }
+    }
+
+    @Test
+    fun missingCommaInArray() {
+        val m = "Unexpected JSON token at offset 3: Expected end of the array or comma at path: \$"
+        val json = """[1 2 3 4]"""
+
+        assertFailsWithSerialMessage("JsonDecodingException", m) {
+            default.decodeFromString<List<Int>>(json)
+        }
+    }
+
+    @Test
+    fun missingCommaInStringMap() {
+        val m = "Unexpected JSON token at offset 9: Expected end of the object or comma at path: \$"
+        val json = """{"a":"1" "b":"2"}"""
+
+        assertFailsWithSerialMessage("JsonDecodingException", m) {
+            default.decodeFromString<Map<String, String>>(json)
+        }
+    }
+
+    @Test
+    fun missingCommaInUnknownKeys() {
+        val m = "Unexpected JSON token at offset 17: Expected end of the object or comma at path: \$"
+        val json = """{"i":"1","b":"2" "c":"2"}"""
+
+        assertFailsWithSerialMessage("JsonDecodingException", m) {
+            lenient.decodeFromString<Child>(json)
+        }
+    }
+}

formats/json-tests/commonTest/src/kotlinx/serialization/json/TrailingCommaTest.kt

@@ -125,4 +125,45 @@ class TrailingCommaTest : JsonTestBase() {
             "wl":{"l":[1, 2, 3,],},}"""
         assertEquals(Mixed(multipleFields, withMap, withList), tj.decodeFromString(input, mode))
     }
+
+    @Test
+    fun testCommaReportedPosition() = parametrizedTest { mode ->
+        val sd = """{"a":"1",    }"""
+
+        checkSerializationException({
+            default.decodeFromString<Map<String, String>>(sd, mode)
+        }, { message ->
+            assertContains(
+                message,
+                """Unexpected JSON token at offset 8: Trailing comma before the end of JSON object"""
+            )
+        })
+    }
+
+    @Test
+    fun testMultipleTrailingCommasAreNotAllowedEvenWhenTrailingIsEnabled() = parametrizedTest { mode ->
+        val sd = """{"a":"1",,}"""
+        checkSerializationException({
+            tj.decodeFromString<Map<String, String>>(sd, mode)
+        }, { message ->
+            assertContains(
+                message,
+                """Unexpected JSON token at offset 9: Multiple consecutive commas are not allowed in JSON"""
+            )
+        })
+    }
+
+    @Test
+    fun testMultipleTrailingCommasError() = parametrizedTest { mode ->
+        val sd = """{"a":"1",,,}"""
+
+        checkSerializationException({
+            default.decodeFromString<Map<String, String>>(sd, mode)
+        }, { message ->
+            assertContains(
+                message,
+                """Unexpected JSON token at offset 9: Multiple consecutive commas are not allowed in JSON"""
+            )
+        })
+    }
 }

formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonExceptions.kt

@@ -46,13 +46,6 @@ 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 'allowTrailingComma = 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. " +

formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonTreeReader.kt

@@ -4,7 +4,7 @@
 
 package kotlinx.serialization.json.internal
 
-import kotlinx.serialization.ExperimentalSerializationApi
+import kotlinx.serialization.*
 import kotlinx.serialization.json.*
 
 @OptIn(ExperimentalSerializationApi::class)
@@ -24,53 +24,52 @@ internal class JsonTreeReader(
         readObjectImpl { callRecursive(Unit) }
 
     private inline fun readObjectImpl(reader: () -> JsonElement): JsonObject {
-        var lastToken = lexer.consumeNextToken(TC_BEGIN_OBJ)
+        lexer.consumeNextToken(TC_BEGIN_OBJ)
+        lexer.path.pushDescriptor(JsonObjectSerializer.descriptor)
+
         if (lexer.peekNextToken() == TC_COMMA) lexer.fail("Unexpected leading comma")
         val result = linkedMapOf<String, JsonElement>()
         while (lexer.canConsumeValue()) {
+            lexer.path.resetCurrentMapKey()
             // Read key and value
             val key = if (isLenient) lexer.consumeStringLenient() else lexer.consumeString()
             lexer.consumeNextToken(TC_COLON)
+
+            lexer.path.updateCurrentMapKey(key)
             val element = reader()
             result[key] = element
             // Verify the next token
-            lastToken = lexer.consumeNextToken()
-            when (lastToken) {
-                TC_COMMA -> Unit // no-op, can continue with `canConsumeValue` that verifies the token after comma
-                TC_END_OBJ -> break // `canConsumeValue` can return incorrect result, since it checks token _after_ TC_END_OBJ
-                else -> lexer.fail("Expected end of the object or comma")
-            }
+            lexer.consumeCommaOrPeekEnd(
+                allowTrailing = trailingCommaAllowed,
+                expectedEnd = '}',
+            )
         }
-        // Check for the correct ending
-        if (lastToken == TC_BEGIN_OBJ) { // Case of empty object
-            lexer.consumeNextToken(TC_END_OBJ)
-        } else if (lastToken == TC_COMMA) { // Trailing comma
-            if (!trailingCommaAllowed) lexer.invalidTrailingComma()
-            lexer.consumeNextToken(TC_END_OBJ)
-        } // else unexpected token?
+
+        lexer.consumeNextToken(TC_END_OBJ)
+        lexer.path.popDescriptor()
+
         return JsonObject(result)
     }
 
     private fun readArray(): JsonElement {
-        var lastToken = lexer.consumeNextToken()
+        lexer.consumeNextToken(TC_BEGIN_LIST)
+        lexer.path.pushDescriptor(JsonArraySerializer.descriptor)
         // Prohibit leading comma
         if (lexer.peekNextToken() == TC_COMMA) lexer.fail("Unexpected leading comma")
         val result = arrayListOf<JsonElement>()
         while (lexer.canConsumeValue()) {
+            lexer.path.updateDescriptorIndex(result.size)
             val element = read()
             result.add(element)
-            lastToken = lexer.consumeNextToken()
-            if (lastToken != TC_COMMA) {
-                lexer.require(lastToken == TC_END_LIST) { "Expected end of the array or comma" }
-            }
-        }
-        // Check for the correct ending
-        if (lastToken == TC_BEGIN_LIST) { // Case of empty object
-            lexer.consumeNextToken(TC_END_LIST)
-        } else if (lastToken == TC_COMMA) { // Trailing comma
-            if (!trailingCommaAllowed) lexer.invalidTrailingComma("array")
-            lexer.consumeNextToken(TC_END_LIST)
+            lexer.consumeCommaOrPeekEnd(
+                allowTrailing = trailingCommaAllowed,
+                expectedEnd = ']',
+                entity = "array"
+            )
         }
+        lexer.consumeNextToken(TC_END_LIST)
+        lexer.path.popDescriptor()
+
         return JsonArray(result)
     }
 
@@ -103,6 +102,7 @@ internal class JsonTreeReader(
                 --stackDepth
                 result
             }
+
             TC_BEGIN_LIST -> readArray()
             else -> lexer.fail("Cannot read Json element because of unexpected ${tokenDescription(token)}")
         }