-
Notifications
You must be signed in to change notification settings - Fork 664
Correctly parse invalid numbers in JsonLiteral.long and other extensions #2852
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| package kotlinx.serialization.json.serializers | ||
|
|
||
| import kotlinx.serialization.Serializable | ||
| import kotlinx.serialization.json.* | ||
| import kotlinx.serialization.test.* | ||
| import kotlin.test.* | ||
|
|
@@ -201,4 +202,17 @@ class JsonPrimitiveSerializerTest : JsonTestBase() { | |
| assertUnsignedNumberEncoding(expected, actual, JsonPrimitive(actual)) | ||
| } | ||
| } | ||
|
|
||
| @Serializable | ||
| class OuterLong(val a: Long) | ||
|
|
||
| @Test | ||
| fun testRejectingIncorrectNumbers() = parametrizedTest { mode -> | ||
| checkSerializationException({ | ||
| default.decodeFromString(OuterLong.serializer(), """{"a":"12:34:45"}""", mode) | ||
| }, { | ||
| if (mode == JsonTestingMode.TREE) assertContains(it, "Failed to parse literal '\"12:34:45\"' as a long value at element: \$.a") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also check exception type?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| else assertContains(it, "Unexpected JSON token at offset 5: Expected closing quotation mark at path: \$.a") | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -223,12 +223,16 @@ internal abstract class AbstractJsonLexer { | |
| fail(charToTokenClass(expected)) | ||
| } | ||
|
|
||
| internal fun fail(expectedToken: Byte, wasConsumed: Boolean = true): Nothing { | ||
| internal inline fun fail( | ||
| expectedToken: Byte, | ||
| wasConsumed: Boolean = true, | ||
| message: (expected: String, source: String) -> String = { expected, source -> "Expected $expected, but had '$source' instead" } | ||
| ): Nothing { | ||
| // Slow path, never called in normal code, can avoid optimizing it | ||
| val expected = tokenDescription(expectedToken) | ||
| val position = if (wasConsumed) currentPosition - 1 else currentPosition | ||
| val s = if (currentPosition == source.length || position < 0) "EOF" else source[position].toString() | ||
| fail("Expected $expected, but had '$s' instead", position) | ||
| fail(message(expected, s), position) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it expected to print in exception only first wrong character?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because generally, we do not know where input ends (e.g. in the case of |
||
| } | ||
|
|
||
| open fun peekNextToken(): Byte { | ||
|
|
@@ -671,6 +675,15 @@ internal abstract class AbstractJsonLexer { | |
| } | ||
| } | ||
|
|
||
| fun consumeNumericLiteralFully(): Long { | ||
| val result = consumeNumericLiteral() | ||
| val next = consumeNextToken() | ||
| if (next != TC_EOF) { | ||
| fail(TC_EOF) { _, source -> "Expected input to contain a single valid number, but got '$source' after it" } | ||
| } | ||
| return result | ||
| } | ||
|
|
||
|
|
||
| fun consumeBoolean(): Boolean { | ||
| return consumeBoolean(skipWhitespaces()) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
println?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll throw exception before that anyway