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

Disallow missing commas between fields (#2287 #2520) #2889

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* 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 comma after the key-value pair at path: \$.i"
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 comma after the key-value pair at path: \$.c"
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 comma after the key-value pair at path: \$.i"
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 comma after the key-value pair at path: \$.i"
val json = """{"i":42 "c":{"i":"string"}}"""

assertFailsWithSerialMessage("JsonDecodingException", message) {
lenient.decodeFromString<Holder>(json)
}
}

@Test
fun missingCommaInDynamicMap(){
alisa101rs marked this conversation as resolved.
Show resolved Hide resolved
val m = "Unexpected JSON token at offset 9: 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: \$[0]"
val json = """[1 2 3 4]"""
alisa101rs marked this conversation as resolved.
Show resolved Hide resolved

assertFailsWithSerialMessage("JsonDecodingException", m) {
default.decodeFromString<List<Int>>(json)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,8 @@ internal open class StreamingJsonDecoder(
descriptor,
discriminatorHolder
)
else -> if (mode == newMode && json.configuration.explicitNulls) {
this
} else {
StreamingJsonDecoder(json, newMode, lexer, descriptor, discriminatorHolder)
}

else -> StreamingJsonDecoder(json, newMode, lexer, descriptor, discriminatorHolder)
}
}

Expand Down Expand Up @@ -220,35 +217,47 @@ internal open class StreamingJsonDecoder(
)

private fun decodeObjectIndex(descriptor: SerialDescriptor): Int {
// hasComma checks are required to properly react on trailing commas
var hasComma = lexer.tryConsumeComma()
while (lexer.canConsumeValue()) { // TODO: consider merging comma consumption and this check
hasComma = false
while (true) {
if (currentIndex != -1) {
val next = lexer.peekNextToken()
if (next == TC_END_OBJ) {
currentIndex = CompositeDecoder.DECODE_DONE
return elementMarker?.nextUnmarkedIndex() ?: CompositeDecoder.DECODE_DONE
}

lexer.require(next == TC_COMMA) { "Expected comma after the key-value pair" }
val commaPosition = lexer.currentPosition
alisa101rs marked this conversation as resolved.
Show resolved Hide resolved
lexer.consumeNextToken()
lexer.require(
configuration.allowTrailingComma || lexer.peekNextToken() != TC_END_OBJ,
position = commaPosition
) { "Trailing comma before the end of JSON object" }
}

if (!lexer.canConsumeValue()) break

val key = decodeStringKey()
lexer.consumeNextToken(COLON)
val index = descriptor.getJsonNameIndex(json, key)
val isUnknown = if (index != UNKNOWN_NAME) {
if (configuration.coerceInputValues && coerceInputValue(descriptor, index)) {
hasComma = lexer.tryConsumeComma()
false // Known element, but coerced
} else {
elementMarker?.mark(index)
return index // Known element without coercing, return it
}
} else {
true // unknown element
currentIndex = index

if (index == UNKNOWN_NAME) {
handleUnknown(descriptor, key)
continue
}

if (isUnknown) { // slow-path for unknown keys handling
hasComma = handleUnknown(descriptor, key)
if (configuration.coerceInputValues && coerceInputValue(descriptor, index)) {
continue
}

elementMarker?.mark(index)
return index
}
if (hasComma && !json.configuration.allowTrailingComma) lexer.invalidTrailingComma()

return elementMarker?.nextUnmarkedIndex() ?: CompositeDecoder.DECODE_DONE
}

private fun handleUnknown(descriptor: SerialDescriptor, key: String): Boolean {
private fun handleUnknown(descriptor: SerialDescriptor, key: String) {
if (descriptor.ignoreUnknownKeys(json) || discriminatorHolder.trySkip(key)) {
lexer.skipElement(configuration.isLenient)
} else {
Expand All @@ -257,7 +266,6 @@ internal open class StreamingJsonDecoder(
lexer.path.popDescriptor()
lexer.failOnUnknownKey(key)
}
return lexer.tryConsumeComma()
}

private fun decodeListIndex(): Int {
Expand Down