Skip to content

Commit

Permalink
Correctly parse invalid numbers in JsonLiteral.long and other extensions
Browse files Browse the repository at this point in the history
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 committed Nov 8, 2024
1 parent 38977b3 commit 6ea0d2c
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 6ea0d2c

Please sign in to comment.