Skip to content

Commit ae9eb10

Browse files
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. JAVA-6280 Co-authored-by: Pritam Acharya <pritamacharya.work@gmail.com>
1 parent 533b2b6 commit ae9eb10

3 files changed

Lines changed: 222 additions & 18 deletions

File tree

bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/JsonBsonEncoder.kt

Lines changed: 97 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package org.bson.codecs.kotlinx
1717

1818
import java.math.BigDecimal
1919
import kotlinx.serialization.ExperimentalSerializationApi
20+
import kotlinx.serialization.SerializationException
2021
import kotlinx.serialization.SerializationStrategy
2122
import kotlinx.serialization.json.Json
2223
import kotlinx.serialization.json.JsonArray
@@ -30,6 +31,25 @@ import org.bson.BsonWriter
3031
import org.bson.codecs.kotlinx.utils.BsonCodecUtils.toJsonNamingStrategy
3132
import org.bson.types.Decimal128
3233

34+
/**
35+
* A [BsonEncoderImpl] that also implements [JsonEncoder], so that a kotlinx.serialization [JsonElement] can be used as
36+
* a property of a `@Serializable` class and written straight to a [BsonWriter].
37+
*
38+
* A [JsonElement] tree is written as plain JSON rather than as Extended JSON, which limits how faithfully BSON types
39+
* survive it:
40+
* - **Numbers are typed from the literal's text.** JSON has a single number type, so a literal containing a fraction or
41+
* an exponent becomes a BSON double and anything else becomes an int, long or `Decimal128`. See [encodeJsonPrimitive]
42+
* for the details and for why a `BigDecimal` is not preserved.
43+
* - **Extended JSON is not interpreted.** A nested object such as `{"$oid": "..."}` is written literally, as a
44+
* sub-document with a `$`-prefixed field name, not as the BSON type it denotes.
45+
* - **BSON type identity is not preserved across a round trip.** [JsonBsonDecoder] flattens each BSON type into a bare
46+
* [JsonPrimitive] - an `ObjectId` becomes its hexadecimal string, a date-time becomes a number of milliseconds and a
47+
* binary value becomes a Base64 or UUID string - so re-encoding a decoded [JsonElement] can yield a different BSON
48+
* type from the one that was read.
49+
*
50+
* Declare a property with the type you need - `Decimal128`, `ObjectId`, `BsonValue` or one of its subtypes, all of
51+
* which are handled by [defaultSerializersModule] - when a BSON type has to be preserved exactly.
52+
*/
3353
@OptIn(ExperimentalSerializationApi::class)
3454
internal class JsonBsonEncoder(
3555
writer: BsonWriter,
@@ -44,6 +64,7 @@ internal class JsonBsonEncoder(
4464
private val INT_MAX_VALUE = BigDecimal.valueOf(Int.MAX_VALUE.toLong())
4565
private val LONG_MIN_VALUE = BigDecimal.valueOf(Long.MIN_VALUE)
4666
private val LONG_MAX_VALUE = BigDecimal.valueOf(Long.MAX_VALUE)
67+
private const val MAX_MESSAGE_LITERAL_LENGTH = 64
4768
}
4869

4970
override val json = Json {
@@ -92,30 +113,101 @@ internal class JsonBsonEncoder(
92113
})
93114
}
94115

116+
/**
117+
* Encodes a JSON primitive as the closest BSON type.
118+
*
119+
* JSON defines a single number type, so `3` and `3.0` denote the same value and a numeric literal's text is the
120+
* only available signal of its type. A literal containing a fraction or an exponent is encoded as a BSON double,
121+
* matching how the driver's own JSON parser types numbers: `org.bson.json.JsonScanner` reads any literal with a `.`
122+
* or an exponent as a double. Within the range of a double, `{"a": 1e20}` therefore encodes to the same BSON type
123+
* through this codec as it does through `BsonDocument.parse`.
124+
*
125+
* Outside the range of the matching BSON type a literal widens to a `Decimal128` rather than losing data, which is
126+
* where this codec is deliberately more precise than `JsonScanner`: `1e400` and `1e-330` widen here, where
127+
* `BsonDocument.parse` yields `Infinity` and `0.0`, and `9223372036854775808` widens where `JsonScanner` throws.
128+
* Subnormal literals such as `1e-320` are the exception, encoding as doubles and losing precision exactly as they
129+
* do in `JsonScanner`.
130+
*
131+
* Note that kotlinx.serialization has no `BigDecimal` support, so a `BigDecimal` placed in a `JsonObject` is stored
132+
* as the string from its `toString()` and cannot be distinguished from a hand-written literal.
133+
* `BigDecimal("1E+19")` consequently encodes as a BSON double, not a `Decimal128`. Use `Decimal128`, or `BsonValue`
134+
* with `BsonValueSerializer`, when the BSON type must be preserved exactly.
135+
*/
95136
private fun encodeJsonPrimitive(primitive: JsonPrimitive) {
96137
val content = primitive.content
97138
when {
98139
primitive.isString -> encodeString(content)
99140
content == "true" || content == "false" -> encodeBoolean(content.toBooleanStrict())
100141
else -> {
101-
val decimal = BigDecimal(content).stripTrailingZeros()
142+
val decimal = parseNumericLiteral(content)
102143
when {
103-
decimal.scale() > 0 -> {
144+
isFloatingLiteral(content) -> {
104145
val abs = decimal.abs()
105146
if ((decimal.signum() == 0 || abs >= DOUBLE_MIN_VALUE) && abs <= DOUBLE_MAX_VALUE) {
106-
encodeDouble(decimal.toDouble())
147+
encodeDouble(parseDouble(content))
107148
} else {
108-
writer.writeDecimal128(Decimal128(decimal))
149+
encodeDecimal128(content, decimal)
109150
}
110151
}
111152
INT_MIN_VALUE <= decimal && decimal <= INT_MAX_VALUE -> encodeInt(decimal.toInt())
112153
LONG_MIN_VALUE <= decimal && decimal <= LONG_MAX_VALUE -> encodeLong(decimal.toLong())
113-
else -> writer.writeDecimal128(Decimal128(decimal))
154+
else -> encodeDecimal128(content, decimal)
114155
}
115156
}
116157
}
117158
}
118159

160+
/** Determines whether a numeric literal was written with a fraction or an exponent. */
161+
private fun isFloatingLiteral(content: String): Boolean = content.any { it == '.' || it == 'e' || it == 'E' }
162+
163+
/**
164+
* Parses a non-string, non-boolean JSON literal as a [BigDecimal].
165+
*
166+
* The content is not guaranteed to be a JSON number: kotlinx.serialization does not validate a
167+
* `JsonUnquotedLiteral`, and permits `NaN` and `Infinity` when `allowSpecialFloatingPointValues` is enabled.
168+
*/
169+
private fun parseNumericLiteral(content: String): BigDecimal =
170+
try {
171+
BigDecimal(content)
172+
} catch (e: NumberFormatException) {
173+
throw notANumber(content, e)
174+
}
175+
176+
/**
177+
* Parses a floating-point literal as a [Double].
178+
*
179+
* Parsing the literal rather than converting the [BigDecimal] preserves `-0.0`, which `BigDecimal` cannot
180+
* represent. `BigDecimal` also accepts any Unicode decimal digit, whereas this accepts only ASCII, so a literal
181+
* that parsed as a [BigDecimal] may still be rejected here.
182+
*/
183+
private fun parseDouble(content: String): Double = content.toDoubleOrNull() ?: throw notANumber(content)
184+
185+
/**
186+
* Encodes a numeric literal that cannot be represented as a BSON int, long or double.
187+
*
188+
* `Decimal128` holds at most 34 significant digits and will not round, so report an unrepresentable literal rather
189+
* than letting a bare `NumberFormatException` escape the codec.
190+
*/
191+
private fun encodeDecimal128(content: String, decimal: BigDecimal) {
192+
val decimal128 =
193+
try {
194+
Decimal128(decimal)
195+
} catch (e: NumberFormatException) {
196+
throw SerializationException(
197+
"Cannot encode the JSON number '${abbreviate(content)}': " +
198+
"its range or precision exceeds BSON Decimal128.",
199+
e)
200+
}
201+
writer.writeDecimal128(decimal128)
202+
}
203+
204+
private fun notANumber(content: String, cause: NumberFormatException? = null): SerializationException =
205+
SerializationException("Cannot encode '${abbreviate(content)}' as BSON: it is not a valid JSON number.", cause)
206+
207+
/** Keeps a literal quoted in an exception message from being unbounded, as it is user supplied. */
208+
private fun abbreviate(content: String): String =
209+
if (content.length <= MAX_MESSAGE_LITERAL_LENGTH) content else content.take(MAX_MESSAGE_LITERAL_LENGTH) + "..."
210+
119211
private fun encodeJsonObject(obj: JsonObject) {
120212
writer.writeStartDocument()
121213
obj.forEach { k, v ->

bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodec.kt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,27 @@ import org.bson.codecs.pojo.annotations.BsonRepresentation
4848
* The Kotlin serializer codec which utilizes the kotlinx serialization module.
4949
*
5050
* Use the [create] method to create the codec
51+
*
52+
* ## Using JsonElement properties
53+
*
54+
* A `kotlinx.serialization.json.JsonElement` may be used as a property of a `@Serializable` class, which is convenient
55+
* for schemaless data. Such a property is written as plain JSON rather than as Extended JSON, so BSON types are not
56+
* preserved through it:
57+
* - JSON defines a single number type, so a numeric literal's type is inferred from its text. A literal containing a
58+
* fraction or an exponent is encoded as a BSON double; anything else becomes an int, a long, or, when it exceeds
59+
* those, a `Decimal128`. A `BigDecimal` placed in a `JsonObject` is stored as its `toString()` and cannot be
60+
* distinguished from a hand-written literal, so `BigDecimal("1E+19")` is encoded as a BSON double.
61+
* - Extended JSON is not interpreted. A nested object such as `{"$oid": "..."}` is written literally, as a sub-document
62+
* with a `$`-prefixed field name, rather than as the BSON type it denotes.
63+
* - Decoding flattens each BSON type into a plain JSON primitive: an `ObjectId` becomes its hexadecimal string, a
64+
* date-time a number of milliseconds, a binary value a Base64 or UUID string. Re-encoding a decoded `JsonElement` can
65+
* therefore produce a different BSON type from the one originally read.
66+
*
67+
* Declare a property with the type you need instead - `Decimal128`, `ObjectId`, `BsonValue` or one of its subtypes are
68+
* all supported by [defaultSerializersModule] - when a BSON type has to be preserved exactly:
69+
* ```
70+
* @Serializable data class Money(val metadata: JsonObject, val amount: @Contextual BsonDecimal128)
71+
* ```
5172
*/
5273
@OptIn(ExperimentalSerializationApi::class, InternalSerializationApi::class)
5374
public class KotlinSerializerCodec<T : Any>

bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt

Lines changed: 104 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import kotlinx.serialization.MissingFieldException
2929
import kotlinx.serialization.SerializationException
3030
import kotlinx.serialization.json.Json
3131
import kotlinx.serialization.json.JsonPrimitive
32+
import kotlinx.serialization.json.JsonUnquotedLiteral
3233
import kotlinx.serialization.json.buildJsonArray
3334
import kotlinx.serialization.json.buildJsonObject
3435
import kotlinx.serialization.json.put
@@ -47,6 +48,7 @@ import org.bson.BsonInvalidOperationException
4748
import org.bson.BsonMaxKey
4849
import org.bson.BsonMinKey
4950
import org.bson.BsonString
51+
import org.bson.BsonType
5052
import org.bson.BsonUndefined
5153
import org.bson.codecs.DecoderContext
5254
import org.bson.codecs.EncoderContext
@@ -128,6 +130,7 @@ import org.junit.jupiter.api.Test
128130
import org.junit.jupiter.api.assertThrows
129131
import org.junit.jupiter.params.ParameterizedTest
130132
import org.junit.jupiter.params.provider.MethodSource
133+
import org.junit.jupiter.params.provider.ValueSource
131134

132135
@OptIn(ExperimentalSerializationApi::class)
133136
@Suppress("LargeClass")
@@ -231,21 +234,38 @@ class KotlinSerializerCodecTest {
231234
fun testJsonPrimitiveNumberEncoding(): Stream<Pair<String, String>> {
232235
return Stream.of(
233236
"""{"value": 0}""" to """{"value": 0}""",
234-
"""{"value": 0}""" to """{"value": 0.0}""",
237+
"""{"value": 0.0}""" to """{"value": 0.0}""",
235238
"""{"value": 1.1}""" to """{"value": 1.1E0}""",
236-
"""{"value": 11}""" to """{"value": 1.1E1}""",
237-
"""{"value": 110}""" to """{"value": 1.1E2}""",
238-
"""{"value": 1100}""" to """{"value": 1.1E3}""",
239+
"""{"value": 11.0}""" to """{"value": 1.1E1}""",
240+
"""{"value": 110.0}""" to """{"value": 1.1E2}""",
241+
"""{"value": 1100.0}""" to """{"value": 1.1E3}""",
239242
"""{"value": 0.1}""" to """{"value": 1E-1}""",
240243
"""{"value": 0.01}""" to """{"value": 1E-2}""",
241244
"""{"value": 0.001}""" to """{"value": 1E-3}""",
242245
"""{"value": -1.1}""" to """{"value": -1.1E0}""",
243-
"""{"value": -11}""" to """{"value": -1.1E1}""",
244-
"""{"value": -110}""" to """{"value": -1.1E2}""",
245-
"""{"value": -1100}""" to """{"value": -1.1E3}""",
246+
"""{"value": -11.0}""" to """{"value": -1.1E1}""",
247+
"""{"value": -110.0}""" to """{"value": -1.1E2}""",
248+
"""{"value": -1100.0}""" to """{"value": -1.1E3}""",
246249
"""{"value": -0.1}""" to """{"value": -1E-1}""",
247250
"""{"value": -0.01}""" to """{"value": -1E-2}""",
248251
"""{"value": -0.001}""" to """{"value": -1E-3}""",
252+
"""{"value": -0.0}""" to """{"value": -0.0}""",
253+
"""{"value": 3.0}""" to """{"value": 3.0}""",
254+
"""{"value": 1.0E20}""" to """{"value": 1.0E20}""",
255+
"""{"value": 30.0}""" to """{"value": 3.0E1}""",
256+
// An exponent alone marks a literal as floating point, with or without a fraction,
257+
// matching how org.bson.json.JsonScanner types numbers.
258+
"""{"value": 1.0E20}""" to """{"value": 1e20}""",
259+
"""{"value": -1.0E20}""" to """{"value": -1e20}""",
260+
"""{"value": 100000.0}""" to """{"value": 1E5}""",
261+
"""{"value": 1.0E20}""" to """{"value": 1e+20}""",
262+
// A magnitude beyond Double widens to Decimal128.
263+
// JsonScanner would yield Infinity and 0.0 here.
264+
"""{"value": {"${'$'}numberDecimal": "1E+400"}}""" to """{"value": 1e400}""",
265+
"""{"value": {"${'$'}numberDecimal": "1E-330"}}""" to """{"value": 1e-330}""",
266+
// The negative side of each threshold still encodes as a double.
267+
"""{"value": 1.7976931348623157E308}""" to """{"value": 1.7976931348623157E308}""",
268+
"""{"value": 1.0E-320}""" to """{"value": 1e-320}""",
249269
"""{"value": 9223372036854775807}""" to """{"value": 9223372036854775807}""",
250270
"""{"value": {"${'$'}numberDecimal": "9223372036854775808"}}""" to """{"value": 9223372036854775808}""",
251271
"""{"value": -9223372036854775808}""" to """{"value": -9223372036854775808}""",
@@ -1038,9 +1058,9 @@ class KotlinSerializerCodecTest {
10381058
|"short": 1,
10391059
|"int": 22,
10401060
|"long": {"$numberLong": "3000000000"},
1041-
|"decimal": {"$numberDecimal": "1E+19"}
1042-
|"decimal2": {"$numberDecimal": "3.123E+700"}
1043-
|"float": 4.1,
1061+
|"decimal": {"$numberDecimal": "10000000000000000000"}
1062+
|"decimal2": {"$numberDecimal": "3.1230E+700"}
1063+
|"float": 4.0,
10441064
|"double": 4.2,
10451065
|"boolean": true,
10461066
|"string": "String"
@@ -1055,9 +1075,9 @@ class KotlinSerializerCodecTest {
10551075
put("short", 1)
10561076
put("int", 22)
10571077
put("long", 3_000_000_000)
1058-
put("decimal", BigDecimal("1E+19"))
1059-
put("decimal2", BigDecimal("3.123E+700"))
1060-
put("float", 4.1)
1078+
put("decimal", BigDecimal("10000000000000000000"))
1079+
put("decimal2", BigDecimal("3.1230E+700"))
1080+
put("float", 4.0)
10611081
put("double", 4.2)
10621082
put("boolean", true)
10631083
put("string", "String")
@@ -1066,6 +1086,21 @@ class KotlinSerializerCodecTest {
10661086
assertRoundTrips(expected, dataClass)
10671087
}
10681088

1089+
@Test
1090+
fun testDataClassWithJsonElementBigDecimal() {
1091+
// A BigDecimal in a JsonObject is stored as its toString.
1092+
// It is indistinguishable from a hand-written literal.
1093+
// Within double range it encodes as a double, beyond it as a Decimal128.
1094+
// Neither round-trips textually, so this asserts encoding only.
1095+
assertEncodesTo(
1096+
"""{"value": {"withinDouble": 1.0E19, "beyondDouble": {"$numberDecimal": "1E+400"}}}""",
1097+
DataClassWithJsonElement(
1098+
buildJsonObject {
1099+
put("withinDouble", BigDecimal("1E+19"))
1100+
put("beyondDouble", BigDecimal("1E+400"))
1101+
}))
1102+
}
1103+
10691104
@Test
10701105
fun testDataClassWithJsonElements() {
10711106
val expected =
@@ -1263,6 +1298,62 @@ class KotlinSerializerCodecTest {
12631298
assertEncodesTo(expected, Json.parseToJsonElement(actual))
12641299
}
12651300

1301+
@ParameterizedTest
1302+
@ValueSource(
1303+
strings =
1304+
[
1305+
// Beyond Long and 34 digits: the integral branch.
1306+
"12345678901234567890123456789012345678901234",
1307+
// Beyond Double and 34 digits: the floating branch.
1308+
"1.2345678901234567890123456789012345678e400",
1309+
// Within 34 significant digits, but the exponent exceeds the Decimal128 range.
1310+
"1E+7000",
1311+
"1E-7000"])
1312+
fun testJsonPrimitiveNumberExceedingDecimal128(literal: String) {
1313+
val exception =
1314+
assertThrows<SerializationException> { serialize(Json.parseToJsonElement("""{"value": $literal}""")) }
1315+
assertTrue(exception.message!!.contains(literal), "Should name the literal: ${exception.message}")
1316+
assertTrue(exception.cause is NumberFormatException, "Should retain the cause: ${exception.cause}")
1317+
}
1318+
1319+
@ParameterizedTest
1320+
@ValueSource(strings = ["NaN", "Infinity", "-Infinity"])
1321+
fun testJsonPrimitiveNonFiniteDouble(literal: String) {
1322+
// Double.toString renders these in a form that is not a valid JSON number.
1323+
// Report that rather than leaking a NumberFormatException from BigDecimal.
1324+
val nonFinite = buildJsonObject { put("value", JsonPrimitive(literal.toDouble())) }
1325+
val exception = assertThrows<SerializationException> { serialize(nonFinite) }
1326+
assertTrue(exception.message!!.contains(literal), "Should name the literal: ${exception.message}")
1327+
assertTrue(exception.cause is NumberFormatException, "Should retain the cause: ${exception.cause}")
1328+
}
1329+
1330+
@ParameterizedTest
1331+
// BigDecimal accepts any Unicode decimal digit, String.toDouble only ASCII.
1332+
// Arabic-Indic and fullwidth digits must not escape as a NumberFormatException.
1333+
@ValueSource(strings = ["١.٥", "1.5", "١e٢", "abc", "", " ", "1_000", "0x10", "1d"])
1334+
fun testJsonPrimitiveUnquotedLiteralIsNotANumber(literal: String) {
1335+
val notANumber = buildJsonObject { put("value", JsonUnquotedLiteral(literal)) }
1336+
val exception = assertThrows<SerializationException> { serialize(notANumber) }
1337+
assertTrue(exception.message!!.contains("not a valid JSON number"), "Got: ${exception.message}")
1338+
}
1339+
1340+
@ParameterizedTest
1341+
@MethodSource("testJsonPrimitiveNumberEncoding")
1342+
fun testJsonPrimitiveNumberMatchesJsonScanner(test: Pair<String, String>) {
1343+
// isFloatingLiteral keys off the literal text to agree with the driver's own JSON parser.
1344+
// Assert that agreement directly, not only against hand-written expectations.
1345+
// Literals outside the range of the matching BSON type are excluded:
1346+
// there the codec widens to Decimal128 where JsonScanner loses data or throws.
1347+
val literal = test.second
1348+
val viaScanner = runCatching { BsonDocument.parse(literal) }
1349+
val viaCodec = serialize(Json.parseToJsonElement(literal))
1350+
if (viaScanner.getOrNull() == viaCodec) return
1351+
assertEquals(
1352+
BsonType.DECIMAL128,
1353+
viaCodec["value"]!!.bsonType,
1354+
"Only a widening to Decimal128 may differ from JsonScanner: $literal gave $viaCodec")
1355+
}
1356+
12661357
@Test
12671358
fun testDataFailures() {
12681359
assertThrows<MissingFieldException>("Missing data") {

0 commit comments

Comments
 (0)