Skip to content

Commit 19f0ead

Browse files
alisa101rsalisa101rs
committed
feat: move comma check to lexer and improve trailing comma reports
1 parent 591bc8f commit 19f0ead

File tree

8 files changed

+170
-95
lines changed

8 files changed

+170
-95
lines changed

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

+7-5
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package kotlinx.serialization.json
66

77
import kotlinx.serialization.*
88
import kotlinx.serialization.builtins.*
9-
import kotlinx.serialization.json.internal.*
109
import kotlinx.serialization.test.*
1110
import kotlin.test.*
1211

@@ -48,7 +47,10 @@ class JsonParserTest : JsonTestBase() {
4847
StringData("https://t.co/M1uhwigsMT"),
4948
default.decodeFromString(StringData.serializer(), """{"data":"https:\/\/t.co\/M1uhwigsMT"}""")
5049
)
51-
assertEquals(StringData("\"test\""), default.decodeFromString(StringData.serializer(), """{"data": "\"test\""}"""))
50+
assertEquals(
51+
StringData("\"test\""),
52+
default.decodeFromString(StringData.serializer(), """{"data": "\"test\""}""")
53+
)
5254
assertEquals(StringData("\u00c9"), default.decodeFromString(StringData.serializer(), """{"data": "\u00c9"}"""))
5355
assertEquals(StringData("""\\"""), default.decodeFromString(StringData.serializer(), """{"data": "\\\\"}"""))
5456
}
@@ -79,11 +81,11 @@ class JsonParserTest : JsonTestBase() {
7981
fun testTrailingComma() {
8082
testTrailingComma("{\"id\":0,}")
8183
testTrailingComma("{\"id\":0 ,}")
82-
testTrailingComma("{\"id\":0 , ,}")
84+
testTrailingComma("{\"id\":0 , ,}", message = "Multiple consecutive commas are not allowed in JSON")
8385
}
8486

85-
private fun testTrailingComma(content: String) {
86-
assertFailsWithSerialMessage("JsonDecodingException", "Trailing comma before the end of JSON object") { Json.parseToJsonElement(content) }
87+
private fun testTrailingComma(content: String, message: String = "Trailing comma before the end of JSON object") {
88+
assertFailsWithSerialMessage("JsonDecodingException", message) { Json.parseToJsonElement(content) }
8789
}
8890

8991
@Test

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

+17-7
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class MissingCommaTest : JsonTestBase() {
2626
@Test
2727
fun missingCommaBetweenFieldsAfterPrimitive() {
2828
val message =
29-
"Unexpected JSON token at offset 8: Expected comma after the key-value pair at path: \$.i"
29+
"Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
3030
val json = """{"i":42 "c":{"i":"string"}}"""
3131

3232
assertFailsWithSerialMessage("JsonDecodingException", message) {
@@ -37,7 +37,7 @@ class MissingCommaTest : JsonTestBase() {
3737
@Test
3838
fun missingCommaBetweenFieldsAfterObject() {
3939
val message =
40-
"Unexpected JSON token at offset 19: Expected comma after the key-value pair at path: \$.c"
40+
"Unexpected JSON token at offset 19: Expected end of the object or comma at path: \$"
4141
val json = """{"c":{"i":"string"}"i":42}"""
4242

4343
assertFailsWithSerialMessage("JsonDecodingException", message) {
@@ -48,7 +48,7 @@ class MissingCommaTest : JsonTestBase() {
4848
@Test
4949
fun allowTrailingCommaDoesNotApplyToCommaBetweenFields() {
5050
val message =
51-
"Unexpected JSON token at offset 8: Expected comma after the key-value pair at path: \$.i"
51+
"Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
5252
val json = """{"i":42 "c":{"i":"string"}}"""
5353

5454
assertFailsWithSerialMessage("JsonDecodingException", message) {
@@ -59,7 +59,7 @@ class MissingCommaTest : JsonTestBase() {
5959
@Test
6060
fun lenientSerializeDoesNotAllowToSkipCommaBetweenFields() {
6161
val message =
62-
"Unexpected JSON token at offset 8: Expected comma after the key-value pair at path: \$.i"
62+
"Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
6363
val json = """{"i":42 "c":{"i":"string"}}"""
6464

6565
assertFailsWithSerialMessage("JsonDecodingException", message) {
@@ -69,7 +69,7 @@ class MissingCommaTest : JsonTestBase() {
6969

7070
@Test
7171
fun missingCommaInDynamicMap() {
72-
val m = "Unexpected JSON token at offset 9: Expected end of the object or comma at path: \$"
72+
val m = "Unexpected JSON token at offset 8: Expected end of the object or comma at path: \$"
7373
val json = """{"i":42 "c":{"i":"string"}}"""
7474
assertFailsWithSerialMessage("JsonDecodingException", m) {
7575
default.parseToJsonElement(json)
@@ -78,7 +78,7 @@ class MissingCommaTest : JsonTestBase() {
7878

7979
@Test
8080
fun missingCommaInArray() {
81-
val m = "Unexpected JSON token at offset 3: Expected end of the array or comma at path: \$[0]"
81+
val m = "Unexpected JSON token at offset 3: Expected end of the array or comma at path: \$"
8282
val json = """[1 2 3 4]"""
8383

8484
assertFailsWithSerialMessage("JsonDecodingException", m) {
@@ -88,11 +88,21 @@ class MissingCommaTest : JsonTestBase() {
8888

8989
@Test
9090
fun missingCommaInStringMap() {
91-
val m = "Unexpected JSON token at offset 9: Expected comma after the key-value pair at path: \$['a']"
91+
val m = "Unexpected JSON token at offset 9: Expected end of the object or comma at path: \$"
9292
val json = """{"a":"1" "b":"2"}"""
9393

9494
assertFailsWithSerialMessage("JsonDecodingException", m) {
9595
default.decodeFromString<Map<String, String>>(json)
9696
}
9797
}
98+
99+
@Test
100+
fun missingCommaInUnknownKeys() {
101+
val m = "Unexpected JSON token at offset 17: Expected end of the object or comma at path: \$"
102+
val json = """{"i":"1","b":"2" "c":"2"}"""
103+
104+
assertFailsWithSerialMessage("JsonDecodingException", m) {
105+
lenient.decodeFromString<Child>(json)
106+
}
107+
}
98108
}

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

+41
Original file line numberDiff line numberDiff line change
@@ -125,4 +125,45 @@ class TrailingCommaTest : JsonTestBase() {
125125
"wl":{"l":[1, 2, 3,],},}"""
126126
assertEquals(Mixed(multipleFields, withMap, withList), tj.decodeFromString(input, mode))
127127
}
128+
129+
@Test
130+
fun testCommaReportedPosition() = parametrizedTest { mode ->
131+
val sd = """{"a":"1", }"""
132+
133+
checkSerializationException({
134+
default.decodeFromString<Map<String, String>>(sd, mode)
135+
}, { message ->
136+
assertContains(
137+
message,
138+
"""Unexpected JSON token at offset 8: Trailing comma before the end of JSON object"""
139+
)
140+
})
141+
}
142+
143+
@Test
144+
fun testMultipleTrailingCommasAreNotAllowedEvenWhenTrailingIsEnabled() = parametrizedTest { mode ->
145+
val sd = """{"a":"1",,}"""
146+
checkSerializationException({
147+
tj.decodeFromString<Map<String, String>>(sd, mode)
148+
}, { message ->
149+
assertContains(
150+
message,
151+
"""Unexpected JSON token at offset 9: Multiple consecutive commas are not allowed in JSON"""
152+
)
153+
})
154+
}
155+
156+
@Test
157+
fun testMultipleTrailingCommasError() = parametrizedTest { mode ->
158+
val sd = """{"a":"1",,,}"""
159+
160+
checkSerializationException({
161+
default.decodeFromString<Map<String, String>>(sd, mode)
162+
}, { message ->
163+
assertContains(
164+
message,
165+
"""Unexpected JSON token at offset 9: Multiple consecutive commas are not allowed in JSON"""
166+
)
167+
})
168+
}
128169
}

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

-7
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,6 @@ internal fun AbstractJsonLexer.throwInvalidFloatingPointDecoded(result: Number):
4646
hint = specialFlowingValuesHint)
4747
}
4848

49-
internal fun AbstractJsonLexer.invalidTrailingComma(entity: String = "object"): Nothing {
50-
fail("Trailing comma before the end of JSON $entity",
51-
position = currentPosition - 1,
52-
hint = "Trailing commas are non-complaint JSON and not allowed by default. Use 'allowTrailingComma = true' in 'Json {}' builder to support them."
53-
)
54-
}
55-
5649
@OptIn(ExperimentalSerializationApi::class)
5750
internal fun InvalidKeyKindException(keyDescriptor: SerialDescriptor) = JsonEncodingException(
5851
"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/JsonPath.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ internal class JsonPath {
2929
/*
3030
* Serial descriptor, map key or the tombstone for map key
3131
*/
32-
private var currentObjectPath = arrayOfNulls<Any?>(8)
32+
private var currentObjectPath = arrayOfNulls<Any>(8)
3333
/*
3434
* Index is a small state-machine used to determine the state of the path:
3535
* >=0 -> index of the element being decoded with the outer class currentObjectPath[currentDepth]

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

+27-27
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
package kotlinx.serialization.json.internal
66

7-
import kotlinx.serialization.ExperimentalSerializationApi
7+
import kotlinx.serialization.*
88
import kotlinx.serialization.json.*
99

1010
@OptIn(ExperimentalSerializationApi::class)
@@ -24,53 +24,52 @@ internal class JsonTreeReader(
2424
readObjectImpl { callRecursive(Unit) }
2525

2626
private inline fun readObjectImpl(reader: () -> JsonElement): JsonObject {
27-
var lastToken = lexer.consumeNextToken(TC_BEGIN_OBJ)
27+
lexer.consumeNextToken(TC_BEGIN_OBJ)
28+
lexer.path.pushDescriptor(JsonObjectSerializer.descriptor)
29+
2830
if (lexer.peekNextToken() == TC_COMMA) lexer.fail("Unexpected leading comma")
2931
val result = linkedMapOf<String, JsonElement>()
3032
while (lexer.canConsumeValue()) {
33+
lexer.path.resetCurrentMapKey()
3134
// Read key and value
3235
val key = if (isLenient) lexer.consumeStringLenient() else lexer.consumeString()
3336
lexer.consumeNextToken(TC_COLON)
37+
38+
lexer.path.updateCurrentMapKey(key)
3439
val element = reader()
3540
result[key] = element
3641
// Verify the next token
37-
lastToken = lexer.consumeNextToken()
38-
when (lastToken) {
39-
TC_COMMA -> Unit // no-op, can continue with `canConsumeValue` that verifies the token after comma
40-
TC_END_OBJ -> break // `canConsumeValue` can return incorrect result, since it checks token _after_ TC_END_OBJ
41-
else -> lexer.fail("Expected end of the object or comma")
42-
}
42+
lexer.consumeCommaOrPeekEnd(
43+
allowTrailing = trailingCommaAllowed,
44+
expectedEnd = '}',
45+
)
4346
}
44-
// Check for the correct ending
45-
if (lastToken == TC_BEGIN_OBJ) { // Case of empty object
46-
lexer.consumeNextToken(TC_END_OBJ)
47-
} else if (lastToken == TC_COMMA) { // Trailing comma
48-
if (!trailingCommaAllowed) lexer.invalidTrailingComma()
49-
lexer.consumeNextToken(TC_END_OBJ)
50-
} // else unexpected token?
47+
48+
lexer.consumeNextToken(TC_END_OBJ)
49+
lexer.path.popDescriptor()
50+
5151
return JsonObject(result)
5252
}
5353

5454
private fun readArray(): JsonElement {
55-
var lastToken = lexer.consumeNextToken()
55+
lexer.consumeNextToken(TC_BEGIN_LIST)
56+
lexer.path.pushDescriptor(JsonArraySerializer.descriptor)
5657
// Prohibit leading comma
5758
if (lexer.peekNextToken() == TC_COMMA) lexer.fail("Unexpected leading comma")
5859
val result = arrayListOf<JsonElement>()
5960
while (lexer.canConsumeValue()) {
61+
lexer.path.updateDescriptorIndex(result.size)
6062
val element = read()
6163
result.add(element)
62-
lastToken = lexer.consumeNextToken()
63-
if (lastToken != TC_COMMA) {
64-
lexer.require(lastToken == TC_END_LIST) { "Expected end of the array or comma" }
65-
}
66-
}
67-
// Check for the correct ending
68-
if (lastToken == TC_BEGIN_LIST) { // Case of empty object
69-
lexer.consumeNextToken(TC_END_LIST)
70-
} else if (lastToken == TC_COMMA) { // Trailing comma
71-
if (!trailingCommaAllowed) lexer.invalidTrailingComma("array")
72-
lexer.consumeNextToken(TC_END_LIST)
64+
lexer.consumeCommaOrPeekEnd(
65+
allowTrailing = trailingCommaAllowed,
66+
expectedEnd = ']',
67+
entity = "array"
68+
)
7369
}
70+
lexer.consumeNextToken(TC_END_LIST)
71+
lexer.path.popDescriptor()
72+
7473
return JsonArray(result)
7574
}
7675

@@ -103,6 +102,7 @@ internal class JsonTreeReader(
103102
--stackDepth
104103
result
105104
}
105+
106106
TC_BEGIN_LIST -> readArray()
107107
else -> lexer.fail("Cannot read Json element because of unexpected ${tokenDescription(token)}")
108108
}

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt

+16-25
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ internal open class StreamingJsonDecoder(
123123
if (descriptor.elementsCount == 0 && descriptor.ignoreUnknownKeys(json)) {
124124
skipLeftoverElements(descriptor)
125125
}
126-
if (lexer.tryConsumeComma() && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma("")
127126
// First consume the object so we know it's correct
128127
lexer.consumeNextToken(mode.end)
129128
// Then cleanup the path
@@ -185,24 +184,21 @@ internal open class StreamingJsonDecoder(
185184
}
186185

187186
private fun decodeMapIndex(): Int {
188-
var hasComma = false
189187
val decodingKey = currentIndex % 2 != 0
190188
if (decodingKey) {
191189
if (currentIndex != -1) {
192-
hasComma = lexer.tryConsumeComma()
190+
lexer.consumeCommaOrPeekEnd(
191+
allowTrailing = json.configuration.allowTrailingComma,
192+
expectedEnd = mode.end
193+
)
193194
}
194195
} else {
195196
lexer.consumeNextToken(COLON)
196197
}
197198

198199
return if (lexer.canConsumeValue()) {
199-
if (decodingKey) {
200-
if (currentIndex == -1) lexer.require(!hasComma) { "Unexpected leading comma" }
201-
else lexer.require(hasComma) { "Expected comma after the key-value pair" }
202-
}
203200
++currentIndex
204201
} else {
205-
if (hasComma && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma()
206202
CompositeDecoder.DECODE_DONE
207203
}
208204
}
@@ -220,19 +216,10 @@ internal open class StreamingJsonDecoder(
220216
private fun decodeObjectIndex(descriptor: SerialDescriptor): Int {
221217
while (true) {
222218
if (currentIndex != -1) {
223-
val next = lexer.peekNextToken()
224-
if (next == TC_END_OBJ) {
225-
currentIndex = CompositeDecoder.DECODE_DONE
226-
return elementMarker?.nextUnmarkedIndex() ?: CompositeDecoder.DECODE_DONE
227-
}
228-
229-
lexer.require(next == TC_COMMA) { "Expected comma after the key-value pair" }
230-
val commaPosition = lexer.currentPosition
231-
lexer.consumeNextToken()
232-
lexer.require(
233-
configuration.allowTrailingComma || lexer.peekNextToken() != TC_END_OBJ,
234-
position = commaPosition
235-
) { "Trailing comma before the end of JSON object" }
219+
lexer.consumeCommaOrPeekEnd(
220+
allowTrailing = json.configuration.allowTrailingComma,
221+
expectedEnd = mode.end
222+
)
236223
}
237224

238225
if (!lexer.canConsumeValue()) break
@@ -270,13 +257,17 @@ internal open class StreamingJsonDecoder(
270257
}
271258

272259
private fun decodeListIndex(): Int {
273-
// Prohibit leading comma
274-
val hasComma = lexer.tryConsumeComma()
260+
if (currentIndex != -1) {
261+
lexer.consumeCommaOrPeekEnd(
262+
allowTrailing = json.configuration.allowTrailingComma,
263+
expectedEnd = mode.end,
264+
entity = "array"
265+
)
266+
}
267+
275268
return if (lexer.canConsumeValue()) {
276-
if (currentIndex != -1 && !hasComma) lexer.fail("Expected end of the array or comma")
277269
++currentIndex
278270
} else {
279-
if (hasComma && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma("array")
280271
CompositeDecoder.DECODE_DONE
281272
}
282273
}

0 commit comments

Comments
 (0)