Skip to content

Commit

Permalink
Correctly parse invalid numbers in JsonLiteral.long and other extensi…
Browse files Browse the repository at this point in the history
…ons (#2852)

Content must be consumed fully, with no leftovers after number.
Also simplify try/catching logic — JsonLiteral.long should throw NumberFormatException, while decoding from JsonElement should throw JsonDecodingException

Fixes #2849
  • Loading branch information
sandwwraith authored Nov 19, 2024
1 parent d15dfee commit 21c9e97
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package kotlinx.serialization.json
import kotlinx.serialization.*
import kotlinx.serialization.descriptors.*
import kotlinx.serialization.encoding.*
import kotlinx.serialization.test.*
import kotlin.test.*

class JsonElementDecodingTest : JsonTestBase() {
Expand Down Expand Up @@ -107,4 +108,13 @@ class JsonElementDecodingTest : JsonTestBase() {
assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = JsonNull), """{"value":null}""", noExplicitNullsOrDefaultsJson)
assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = null), """{}""", noExplicitNullsOrDefaultsJson)
}

@Test
fun testLiteralIncorrectParsing() {
val str = """{"a": "3 digit then random string"}"""
val obj = Json.decodeFromString<JsonObject>(str)
assertFailsWithMessage<NumberFormatException>("Expected input to contain a single valid number") {
println(obj.getValue("a").jsonPrimitive.long)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package kotlinx.serialization.json.serializers

import kotlinx.serialization.Serializable
import kotlinx.serialization.json.*
import kotlinx.serialization.test.*
import kotlin.test.*
Expand Down Expand Up @@ -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")
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
Expand Up @@ -256,7 +256,7 @@ public val JsonElement.jsonNull: JsonNull
*/
public val JsonPrimitive.int: Int
get() {
val result = mapExceptions { StringJsonLexer(content).consumeNumericLiteral() }
val result = exceptionToNumberFormatException { parseLongImpl() }
if (result !in Int.MIN_VALUE..Int.MAX_VALUE) throw NumberFormatException("$content is not an Int")
return result.toInt()
}
Expand All @@ -266,7 +266,7 @@ public val JsonPrimitive.int: Int
*/
public val JsonPrimitive.intOrNull: Int?
get() {
val result = mapExceptionsToNull { StringJsonLexer(content).consumeNumericLiteral() } ?: return null
val result = exceptionToNull { parseLongImpl() } ?: return null
if (result !in Int.MIN_VALUE..Int.MAX_VALUE) return null
return result.toInt()
}
Expand All @@ -275,14 +275,13 @@ public val JsonPrimitive.intOrNull: Int?
* Returns content of current element as long
* @throws NumberFormatException if current element is not a valid representation of number
*/
public val JsonPrimitive.long: Long get() = mapExceptions { StringJsonLexer(content).consumeNumericLiteral() }
public val JsonPrimitive.long: Long get() = exceptionToNumberFormatException { parseLongImpl() }

/**
* Returns content of current element as long or `null` if current element is not a valid representation of number
*/
public val JsonPrimitive.longOrNull: Long?
get() =
mapExceptionsToNull { StringJsonLexer(content).consumeNumericLiteral() }
get() = exceptionToNull { parseLongImpl() }

/**
* Returns content of current element as double
Expand Down Expand Up @@ -326,15 +325,15 @@ public val JsonPrimitive.contentOrNull: String? get() = if (this is JsonNull) nu
private fun JsonElement.error(element: String): Nothing =
throw IllegalArgumentException("Element ${this::class} is not a $element")

private inline fun <T> mapExceptionsToNull(f: () -> T): T? {
private inline fun <T> exceptionToNull(f: () -> T): T? {
return try {
f()
} catch (e: JsonDecodingException) {
null
}
}

private inline fun <T> mapExceptions(f: () -> T): T {
private inline fun <T> exceptionToNumberFormatException(f: () -> T): T {
return try {
f()
} catch (e: JsonDecodingException) {
Expand All @@ -345,3 +344,6 @@ private inline fun <T> mapExceptions(f: () -> T): T {
@PublishedApi
internal fun unexpectedJson(key: String, expected: String): Nothing =
throw IllegalArgumentException("Element $key is not a $expected")

// Use this function to avoid re-wrapping exception into NumberFormatException
internal fun JsonPrimitive.parseLongImpl(): Long = StringJsonLexer(content).consumeNumericLiteralFully()
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,24 @@ private sealed class AbstractJsonTreeDecoder(
getPrimitiveValue(tag, "boolean", JsonPrimitive::booleanOrNull)

override fun decodeTaggedByte(tag: String) = getPrimitiveValue(tag, "byte") {
val result = int
val result = parseLongImpl()
if (result in Byte.MIN_VALUE..Byte.MAX_VALUE) result.toByte()
else null
}

override fun decodeTaggedShort(tag: String) = getPrimitiveValue(tag, "short") {
val result = int
val result = parseLongImpl()
if (result in Short.MIN_VALUE..Short.MAX_VALUE) result.toShort()
else null
}

override fun decodeTaggedInt(tag: String) = getPrimitiveValue(tag, "int") { int }
override fun decodeTaggedLong(tag: String) = getPrimitiveValue(tag, "long") { long }
override fun decodeTaggedInt(tag: String) = getPrimitiveValue(tag, "int") {
val result = parseLongImpl()
if (result in Int.MIN_VALUE..Int.MAX_VALUE) result.toInt()
else null
}

override fun decodeTaggedLong(tag: String) = getPrimitiveValue(tag, "long") { parseLongImpl() }

override fun decodeTaggedFloat(tag: String): Float {
val result = getPrimitiveValue(tag, "float") { float }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

open fun peekNextToken(): Byte {
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 21c9e97

Please sign in to comment.