From c9fe8228084afc6a8800c20f92b26f028b1e344e Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Tue, 11 Aug 2026 14:29:24 +0100 Subject: [PATCH] Fix regression in JSON primitive handling JSON floating-point literals with an integral value were encoded as integer BSON types. Change to type the literal from its text instead. A literal containing a fraction or an exponent is encoded as a BSON double, matching how the driver's own JSON parser types numbers. Outside the range of the matching BSON type a literal still widens to a Decimal128 rather than losing data. Because kotlinx.serialization has no BigDecimal support, a BigDecimal in a JsonObject is stored as its toString() and cannot be distinguished from a hand-written literal, so BigDecimal("1E+19") encodes as a double. Callers needing an exact BSON type should declare a typed property such as Decimal128 or BsonValue. Invalid and unrepresentable numeric literals now report a SerializationException rather than a raw NumberFormatException. BigDecimal accepts any Unicode decimal digit whereas JSON numbers are ASCII, so the characters are checked before parsing, keeping integral and floating literals consistent. JAVA-6280 Co-authored-by: Pritam Acharya --- .../bson/codecs/kotlinx/JsonBsonEncoder.kt | 112 +++++++++++++- .../codecs/kotlinx/KotlinSerializerCodec.kt | 21 +++ .../kotlinx/KotlinSerializerCodecTest.kt | 139 ++++++++++++++++-- 3 files changed, 254 insertions(+), 18 deletions(-) diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/JsonBsonEncoder.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/JsonBsonEncoder.kt index 085bbac6a50..00c49704f78 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/JsonBsonEncoder.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/JsonBsonEncoder.kt @@ -17,6 +17,7 @@ package org.bson.codecs.kotlinx import java.math.BigDecimal import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.SerializationException import kotlinx.serialization.SerializationStrategy import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonArray @@ -30,6 +31,25 @@ import org.bson.BsonWriter import org.bson.codecs.kotlinx.utils.BsonCodecUtils.toJsonNamingStrategy import org.bson.types.Decimal128 +/** + * A [BsonEncoderImpl] that also implements [JsonEncoder], so that a kotlinx.serialization [JsonElement] can be used as + * a property of a `@Serializable` class and written straight to a [BsonWriter]. + * + * A [JsonElement] tree is written as plain JSON rather than as Extended JSON, which limits how faithfully BSON types + * survive it: + * - **Numbers are typed from the literal's text.** JSON has a single number type, so a literal containing a fraction or + * an exponent becomes a BSON double and anything else becomes an int, long or `Decimal128`. See [encodeJsonPrimitive] + * for the details and for why a `BigDecimal` is not preserved. + * - **Extended JSON is not interpreted.** A nested object such as `{"$oid": "..."}` is written literally, as a + * sub-document with a `$`-prefixed field name, not as the BSON type it denotes. + * - **BSON type identity is not preserved across a round trip.** [JsonBsonDecoder] flattens each BSON type into a bare + * [JsonPrimitive] - an `ObjectId` becomes its hexadecimal string, a date-time becomes a number of milliseconds and a + * binary value becomes a Base64 or UUID string - so re-encoding a decoded [JsonElement] can yield a different BSON + * type from the one that was read. + * + * Declare a property with the type you need - `Decimal128`, `ObjectId`, `BsonValue` or one of its subtypes, all of + * which are handled by [defaultSerializersModule] - when a BSON type has to be preserved exactly. + */ @OptIn(ExperimentalSerializationApi::class) internal class JsonBsonEncoder( writer: BsonWriter, @@ -44,6 +64,9 @@ internal class JsonBsonEncoder( private val INT_MAX_VALUE = BigDecimal.valueOf(Int.MAX_VALUE.toLong()) private val LONG_MIN_VALUE = BigDecimal.valueOf(Long.MIN_VALUE) private val LONG_MAX_VALUE = BigDecimal.valueOf(Long.MAX_VALUE) + private const val MAX_MESSAGE_LITERAL_LENGTH = 64 + /** The JSON number grammar (RFC 8259, section 6). */ + private val JSON_NUMBER = Regex("""-?(0|[1-9]\d*)(\.\d+)?([eE][-+]?\d+)?""") } override val json = Json { @@ -92,30 +115,109 @@ internal class JsonBsonEncoder( }) } + /** + * Encodes a JSON primitive as the closest BSON type. + * + * JSON defines a single number type, so `3` and `3.0` denote the same value and a numeric literal's text is the + * only available signal of its type. A literal containing a fraction or an exponent is encoded as a BSON double, + * matching how the driver's own JSON parser types numbers: `org.bson.json.JsonScanner` reads any literal with a `.` + * or an exponent as a double. Within the range of a double, `{"a": 1e20}` therefore encodes to the same BSON type + * through this codec as it does through `BsonDocument.parse`. + * + * Outside the range of the matching BSON type a literal widens to a `Decimal128` rather than losing data, which is + * where this codec is deliberately more precise than `JsonScanner`: `1e400` and `1e-330` widen here, where + * `BsonDocument.parse` yields `Infinity` and `0.0`, and `9223372036854775808` widens where `JsonScanner` throws. + * + * Subnormal literals such as `1e-320` are the exception, encoding as doubles and losing precision exactly as they + * do in `JsonScanner`. + * + * Note: `kotlinx.serialization` has no `BigDecimal` support, so a `BigDecimal` placed in a `JsonObject` is stored + * as the string from its `toString()` and cannot be distinguished from a hand-written literal. + * `BigDecimal("1E+19")` consequently encodes as a BSON double, not a `Decimal128`. Use `Decimal128`, or `BsonValue` + * with `BsonValueSerializer`, when the BSON type must be preserved exactly. + */ private fun encodeJsonPrimitive(primitive: JsonPrimitive) { val content = primitive.content when { primitive.isString -> encodeString(content) content == "true" || content == "false" -> encodeBoolean(content.toBooleanStrict()) else -> { - val decimal = BigDecimal(content).stripTrailingZeros() + val decimal = parseNumericLiteral(content) when { - decimal.scale() > 0 -> { + isFloatingLiteral(content) -> { val abs = decimal.abs() if ((decimal.signum() == 0 || abs >= DOUBLE_MIN_VALUE) && abs <= DOUBLE_MAX_VALUE) { - encodeDouble(decimal.toDouble()) + encodeDouble(parseDouble(content)) } else { - writer.writeDecimal128(Decimal128(decimal)) + encodeDecimal128(content, decimal) } } INT_MIN_VALUE <= decimal && decimal <= INT_MAX_VALUE -> encodeInt(decimal.toInt()) LONG_MIN_VALUE <= decimal && decimal <= LONG_MAX_VALUE -> encodeLong(decimal.toLong()) - else -> writer.writeDecimal128(Decimal128(decimal)) + else -> encodeDecimal128(content, decimal) } } } } + /** Determines whether a numeric literal was written with a fraction or an exponent. */ + private fun isFloatingLiteral(content: String): Boolean = content.any { it == '.' || it == 'e' || it == 'E' } + + /** + * Parses a non-string, non-boolean JSON literal as a [BigDecimal]. + * + * The content is not guaranteed to be a JSON number: kotlinx.serialization does not validate a + * `JsonUnquotedLiteral`, and permits `NaN` and `Infinity` when `allowSpecialFloatingPointValues` is enabled. + * + * The literal is matched against the JSON number grammar before parsing because `BigDecimal` is more lenient than + * JSON: it accepts any Unicode decimal digit, a leading `+`, a leading `.` and a trailing `.`. Without the check + * `"١٢٣"`, `"+1"`, `".5"` and `"1."` would all be encoded as numbers, while `"١.٥"` was rejected, since only the + * latter also fails to parse as a [Double]. The grammar is the one accepted by [org.bson.json.JsonScanner], so a + * literal is typed here only if the driver's own JSON parser would also read it as a number. + */ + private fun parseNumericLiteral(content: String): BigDecimal { + if (!JSON_NUMBER.matches(content)) throw notANumber(content) + return try { + BigDecimal(content) + } catch (e: NumberFormatException) { + throw notANumber(content, e) + } + } + + /** + * Parses a floating-point literal as a [Double]. + * + * Parsing the literal rather than converting the [BigDecimal] preserves `-0.0`, which `BigDecimal` cannot + * represent. + */ + private fun parseDouble(content: String): Double = content.toDoubleOrNull() ?: throw notANumber(content) + + /** + * Encodes a numeric literal that cannot be represented as a BSON int, long or double. + * + * `Decimal128` holds at most 34 significant digits and will not round, so report an unrepresentable literal rather + * than letting a bare `NumberFormatException` escape the codec. + */ + private fun encodeDecimal128(content: String, decimal: BigDecimal) { + val decimal128 = + try { + Decimal128(decimal) + } catch (e: NumberFormatException) { + throw SerializationException( + "Cannot encode the JSON number '${abbreviate(content)}': " + + "its range or precision exceeds BSON Decimal128.", + e) + } + writer.writeDecimal128(decimal128) + } + + private fun notANumber(content: String, cause: NumberFormatException? = null): SerializationException = + SerializationException("Cannot encode '${abbreviate(content)}' as BSON: it is not a valid JSON number.", cause) + + /** Keeps a literal quoted in an exception message from being unbounded, as it is user supplied. */ + private fun abbreviate(content: String): String = + if (content.length <= MAX_MESSAGE_LITERAL_LENGTH) content else content.take(MAX_MESSAGE_LITERAL_LENGTH) + "..." + private fun encodeJsonObject(obj: JsonObject) { writer.writeStartDocument() obj.forEach { k, v -> diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodec.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodec.kt index 0c7491b2278..b0e42545ee1 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodec.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodec.kt @@ -48,6 +48,27 @@ import org.bson.codecs.pojo.annotations.BsonRepresentation * The Kotlin serializer codec which utilizes the kotlinx serialization module. * * Use the [create] method to create the codec + * + * ## Using JsonElement properties + * + * A `kotlinx.serialization.json.JsonElement` may be used as a property of a `@Serializable` class, which is convenient + * for schemaless data. Such a property is written as plain JSON rather than as Extended JSON, so BSON types are not + * preserved through it: + * - JSON defines a single number type, so a numeric literal's type is inferred from its text. A literal containing a + * fraction or an exponent is encoded as a BSON double; anything else becomes an int, a long, or, when it exceeds + * those, a `Decimal128`. A `BigDecimal` placed in a `JsonObject` is stored as its `toString()` and cannot be + * distinguished from a hand-written literal, so `BigDecimal("1E+19")` is encoded as a BSON double. + * - Extended JSON is not interpreted. A nested object such as `{"$oid": "..."}` is written literally, as a sub-document + * with a `$`-prefixed field name, rather than as the BSON type it denotes. + * - Decoding flattens each BSON type into a plain JSON primitive: an `ObjectId` becomes its hexadecimal string, a + * date-time a number of milliseconds, a binary value a Base64 or UUID string. Re-encoding a decoded `JsonElement` can + * therefore produce a different BSON type from the one originally read. + * + * Declare a property with the type you need instead - `Decimal128`, `ObjectId`, `BsonValue` or one of its subtypes are + * all supported by [defaultSerializersModule] - when a BSON type has to be preserved exactly: + * ``` + * @Serializable data class Money(val metadata: JsonObject, val amount: @Contextual BsonDecimal128) + * ``` */ @OptIn(ExperimentalSerializationApi::class, InternalSerializationApi::class) public class KotlinSerializerCodec diff --git a/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt b/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt index c4c3246891d..733c01298c2 100644 --- a/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt +++ b/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt @@ -29,6 +29,7 @@ import kotlinx.serialization.MissingFieldException import kotlinx.serialization.SerializationException import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.JsonUnquotedLiteral import kotlinx.serialization.json.buildJsonArray import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.put @@ -47,6 +48,7 @@ import org.bson.BsonInvalidOperationException import org.bson.BsonMaxKey import org.bson.BsonMinKey import org.bson.BsonString +import org.bson.BsonType import org.bson.BsonUndefined import org.bson.codecs.DecoderContext import org.bson.codecs.EncoderContext @@ -128,6 +130,7 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource +import org.junit.jupiter.params.provider.ValueSource @OptIn(ExperimentalSerializationApi::class) @Suppress("LargeClass") @@ -231,21 +234,42 @@ class KotlinSerializerCodecTest { fun testJsonPrimitiveNumberEncoding(): Stream> { return Stream.of( """{"value": 0}""" to """{"value": 0}""", - """{"value": 0}""" to """{"value": 0.0}""", + """{"value": 0.0}""" to """{"value": 0.0}""", """{"value": 1.1}""" to """{"value": 1.1E0}""", - """{"value": 11}""" to """{"value": 1.1E1}""", - """{"value": 110}""" to """{"value": 1.1E2}""", - """{"value": 1100}""" to """{"value": 1.1E3}""", + """{"value": 11.0}""" to """{"value": 1.1E1}""", + """{"value": 110.0}""" to """{"value": 1.1E2}""", + """{"value": 1100.0}""" to """{"value": 1.1E3}""", """{"value": 0.1}""" to """{"value": 1E-1}""", """{"value": 0.01}""" to """{"value": 1E-2}""", """{"value": 0.001}""" to """{"value": 1E-3}""", """{"value": -1.1}""" to """{"value": -1.1E0}""", - """{"value": -11}""" to """{"value": -1.1E1}""", - """{"value": -110}""" to """{"value": -1.1E2}""", - """{"value": -1100}""" to """{"value": -1.1E3}""", + """{"value": -11.0}""" to """{"value": -1.1E1}""", + """{"value": -110.0}""" to """{"value": -1.1E2}""", + """{"value": -1100.0}""" to """{"value": -1.1E3}""", """{"value": -0.1}""" to """{"value": -1E-1}""", """{"value": -0.01}""" to """{"value": -1E-2}""", """{"value": -0.001}""" to """{"value": -1E-3}""", + """{"value": -0.0}""" to """{"value": -0.0}""", + """{"value": 3.0}""" to """{"value": 3.0}""", + """{"value": 1.0E20}""" to """{"value": 1.0E20}""", + """{"value": 30.0}""" to """{"value": 3.0E1}""", + // An exponent alone marks a literal as floating point, with or without a fraction, + // matching how org.bson.json.JsonScanner types numbers. + """{"value": 1.0E20}""" to """{"value": 1e20}""", + """{"value": -1.0E20}""" to """{"value": -1e20}""", + """{"value": 100000.0}""" to """{"value": 1E5}""", + """{"value": 1.0E20}""" to """{"value": 1e+20}""", + // A magnitude beyond Double widens to Decimal128. + """{"value": {"${'$'}numberDecimal": "1E+400"}}""" to """{"value": 1e400}""", + """{"value": {"${'$'}numberDecimal": "1E-330"}}""" to """{"value": 1e-330}""", + // The negative side of each threshold still encodes as a double. + """{"value": 1.7976931348623157E308}""" to """{"value": 1.7976931348623157E308}""", + """{"value": 1.0E-320}""" to """{"value": 1e-320}""", + // An integral literal takes the narrowest BSON type that holds it. + """{"value": 2147483647}""" to """{"value": 2147483647}""", + """{"value": 2147483648}""" to """{"value": 2147483648}""", + """{"value": -2147483648}""" to """{"value": -2147483648}""", + """{"value": -2147483649}""" to """{"value": -2147483649}""", """{"value": 9223372036854775807}""" to """{"value": 9223372036854775807}""", """{"value": {"${'$'}numberDecimal": "9223372036854775808"}}""" to """{"value": 9223372036854775808}""", """{"value": -9223372036854775808}""" to """{"value": -9223372036854775808}""", @@ -1038,9 +1062,9 @@ class KotlinSerializerCodecTest { |"short": 1, |"int": 22, |"long": {"$numberLong": "3000000000"}, - |"decimal": {"$numberDecimal": "1E+19"} - |"decimal2": {"$numberDecimal": "3.123E+700"} - |"float": 4.1, + |"decimal": {"$numberDecimal": "10000000000000000000"} + |"decimal2": {"$numberDecimal": "3.1230E+700"} + |"float": 4.0, |"double": 4.2, |"boolean": true, |"string": "String" @@ -1055,9 +1079,9 @@ class KotlinSerializerCodecTest { put("short", 1) put("int", 22) put("long", 3_000_000_000) - put("decimal", BigDecimal("1E+19")) - put("decimal2", BigDecimal("3.123E+700")) - put("float", 4.1) + put("decimal", BigDecimal("10000000000000000000")) + put("decimal2", BigDecimal("3.1230E+700")) + put("float", 4.0) put("double", 4.2) put("boolean", true) put("string", "String") @@ -1066,6 +1090,21 @@ class KotlinSerializerCodecTest { assertRoundTrips(expected, dataClass) } + @Test + fun testDataClassWithJsonElementBigDecimal() { + // A BigDecimal in a JsonObject is stored as its toString. + // It is indistinguishable from a hand-written literal. + // Within double range it encodes as a double, beyond it as a Decimal128. + // Neither round-trips textually, so this asserts encoding only. + assertEncodesTo( + """{"value": {"withinDouble": 1.0E19, "beyondDouble": {"$numberDecimal": "1E+400"}}}""", + DataClassWithJsonElement( + buildJsonObject { + put("withinDouble", BigDecimal("1E+19")) + put("beyondDouble", BigDecimal("1E+400")) + })) + } + @Test fun testDataClassWithJsonElements() { val expected = @@ -1263,6 +1302,80 @@ class KotlinSerializerCodecTest { assertEncodesTo(expected, Json.parseToJsonElement(actual)) } + @ParameterizedTest + @ValueSource( + strings = + [ + // Beyond Long and 34 digits: the integral branch. + "12345678901234567890123456789012345678901234", + // Beyond Double and 34 digits: the floating branch. + "1.2345678901234567890123456789012345678e400", + // Within 34 significant digits, but the exponent exceeds the Decimal128 range. + "1E+7000", + "1E-7000"]) + fun testJsonPrimitiveNumberExceedingDecimal128(literal: String) { + val exception = + assertThrows { serialize(Json.parseToJsonElement("""{"value": $literal}""")) } + assertTrue(exception.message!!.contains(literal), "Should name the literal: ${exception.message}") + assertTrue(exception.cause is NumberFormatException, "Should retain the cause: ${exception.cause}") + } + + @ParameterizedTest + @ValueSource(strings = ["NaN", "Infinity", "-Infinity"]) + fun testJsonPrimitiveNonFiniteDouble(literal: String) { + // Double.toString renders these in a form that is not a valid JSON number. + // Report that rather than leaking a NumberFormatException from BigDecimal. + val nonFinite = buildJsonObject { put("value", JsonPrimitive(literal.toDouble())) } + val exception = assertThrows { serialize(nonFinite) } + assertTrue(exception.message!!.contains(literal), "Should name the literal: ${exception.message}") + } + + @ParameterizedTest + // BigDecimal is more lenient than the JSON number grammar. Reject any invalid JSON number. + @ValueSource( + strings = + [ + // JSON permits '+' only after an exponent letter. + "+1", + "+1e+5", + // JSON requires a digit on both sides of the decimal point. + "1.", + ".5", + // JSON forbids a leading zero. + "01", + // BigDecimal accepts any Unicode decimal digit, on the integral path as well as the + // floating one. + "\u0661\u0662\u0663", + "\u0661.\u0665", + // Double.parseDouble accepts a trailing type suffix, so a valid prefix is not + // enough. + "1d", + "abc", + "", + " "]) + fun testJsonPrimitiveUnquotedLiteralIsNotANumber(literal: String) { + val notANumber = buildJsonObject { put("value", JsonUnquotedLiteral(literal)) } + val exception = assertThrows { serialize(notANumber) } + assertTrue(exception.message!!.contains("not a valid JSON number"), "Got: ${exception.message}") + } + + @ParameterizedTest + @MethodSource("testJsonPrimitiveNumberEncoding") + fun testJsonPrimitiveNumberMatchesJsonScanner(test: Pair) { + // isFloatingLiteral keys off the literal text to agree with the driver's own JSON parser. + // Assert that agreement directly, not only against hand-written expectations. + // Literals outside the range of the matching BSON type are excluded: + // there the codec widens to Decimal128 where JsonScanner loses data or throws. + val literal = test.second + val viaScanner = runCatching { BsonDocument.parse(literal) } + val viaCodec = serialize(Json.parseToJsonElement(literal)) + if (viaScanner.getOrNull() == viaCodec) return + assertEquals( + BsonType.DECIMAL128, + viaCodec["value"]!!.bsonType, + "Only a widening to Decimal128 may differ from JsonScanner: $literal gave $viaCodec") + } + @Test fun testDataFailures() { assertThrows("Missing data") {