Correctly parse invalid numbers in JsonLiteral.long and other extensions (#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

Author: sandwwraith

formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonElementDecodingTest.kt

@@ -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() {
@@ -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)
+        }
+    }
 }

formats/json-tests/commonTest/src/kotlinx/serialization/json/serializers/JsonPrimitiveSerializerTest.kt

@@ -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")
+            else assertContains(it, "Unexpected JSON token at offset 5: Expected closing quotation mark at path: \$.a")
+        })
+    }
 }

formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt

@@ -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()
     }
@@ -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()
     }
@@ -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
@@ -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) {
@@ -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()

formats/json/commonMain/src/kotlinx/serialization/json/internal/TreeJsonDecoder.kt

@@ -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 }

formats/json/commonMain/src/kotlinx/serialization/json/internal/lexer/AbstractJsonLexer.kt

@@ -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 {
@@ -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())