Skip to content

JsonBsonEncoder: fix parsing of JsonPrimitive numbers #1937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
berlix wants to merge 2 commits into
base: main
Choose a base branch
from
+50 −18
Open

JsonBsonEncoder: fix parsing of JsonPrimitive numbers #1937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bcb30a2
JsonBsonEncoder: fix parsing of JsonPrimitive numbers
berlix Apr 9, 2026
817845e
JsonBsonEncoder: fix handling of negative values
berlix Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/JsonBsonEncoder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import kotlinx.serialization.json.JsonNull
import kotlinx.serialization.json.JsonObject
import kotlinx.serialization.json.JsonPrimitive
import kotlinx.serialization.json.double
import kotlinx.serialization.json.int
import kotlinx.serialization.json.long
import kotlinx.serialization.modules.SerializersModule
import org.bson.BsonWriter
import org.bson.codecs.kotlinx.utils.BsonCodecUtils.toJsonNamingStrategy
Expand All @@ -41,7 +39,6 @@ internal class JsonBsonEncoder(
) : BsonEncoderImpl(writer, serializersModule, configuration), JsonEncoder {

companion object {
private val DOUBLE_MIN_VALUE = BigDecimal.valueOf(Double.MIN_VALUE)
private val DOUBLE_MAX_VALUE = BigDecimal.valueOf(Double.MAX_VALUE)
private val INT_MIN_VALUE = BigDecimal.valueOf(Int.MIN_VALUE.toLong())
private val INT_MAX_VALUE = BigDecimal.valueOf(Int.MAX_VALUE.toLong())
Expand Down Expand Up @@ -101,16 +98,16 @@ internal class JsonBsonEncoder(
primitive.isString -> encodeString(content)
content == "true" || content == "false" -> encodeBoolean(content.toBooleanStrict())
else -> {
val decimal = BigDecimal(content)
val decimal = BigDecimal(content).stripTrailingZeros()
when {
decimal.scale() != 0 ->
if (DOUBLE_MIN_VALUE <= decimal && decimal <= DOUBLE_MAX_VALUE) {
decimal.scale() > 0 ->
if (decimal.abs() <= DOUBLE_MAX_VALUE) {
encodeDouble(primitive.double)
} else {
writer.writeDecimal128(Decimal128(decimal))
}
Comment on lines +103 to 108
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double-range check for fractional numbers only allows values >= Double.MIN_VALUE, which is positive. This causes negative fractional values (e.g., -1.1) to skip the double branch and be encoded as Decimal128 instead of a BSON double. Consider checking the absolute value (and treating 0 as a valid double) while still enforcing the finite double range, e.g., using decimal.abs() against Double.MIN_VALUE/Double.MAX_VALUE (and decimal == 0 special-case).

Suggested change
decimal.scale() > 0 ->
if (DOUBLE_MIN_VALUE <= decimal && decimal <= DOUBLE_MAX_VALUE) {
encodeDouble(primitive.double)
} else {
writer.writeDecimal128(Decimal128(decimal))
}
decimal.scale() > 0 -> {
val absoluteDecimal = decimal.abs()
if (decimal.signum() == 0 || (DOUBLE_MIN_VALUE <= absoluteDecimal && absoluteDecimal <= DOUBLE_MAX_VALUE)) {
encodeDouble(primitive.double)
} else {
writer.writeDecimal128(Decimal128(decimal))
}
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

@berlix berlix Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 817845e, with appropriate test cases. I removed the check for DOUBLE_MIN_VALUE entirely, since it wouldn't make a ton of sense - if anything, we'd want to switch to Decimal128's precision whenever Double cannot represent the original number within some (arbitrary) precision (say, max. 1% deviation), and not only when the number happens to be close to zero. But that seems like an inappropriate amount of guesswork to add here.

For the record, this issue was not introduced by my PR. :-)

INT_MIN_VALUE <= decimal && decimal <= INT_MAX_VALUE -> encodeInt(primitive.int)
LONG_MIN_VALUE <= decimal && decimal <= LONG_MAX_VALUE -> encodeLong(primitive.long)
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))
}
}
Expand Down
55 changes: 45 additions & 10 deletions bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import kotlinx.datetime.LocalTime
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.MissingFieldException
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonPrimitive
import kotlinx.serialization.json.buildJsonArray
import kotlinx.serialization.json.buildJsonObject
Expand Down Expand Up @@ -146,10 +147,10 @@ class KotlinSerializerCodecTest {
| "code": {"${'$'}code": "int i = 0;"},
| "codeWithScope": {"${'$'}code": "int x = y", "${'$'}scope": {"y": 1}},
| "dateTime": {"${'$'}date": {"${'$'}numberLong": "1577836801000"}},
| "decimal128": {"${'$'}numberDecimal": "1.0"},
| "decimal128": {"${'$'}numberDecimal": "1.1"},
| "documentEmpty": {},
| "document": {"a": {"${'$'}numberInt": "1"}},
| "double": {"${'$'}numberDouble": "62.0"},
| "double": {"${'$'}numberDouble": "62.1"},
| "int32": {"${'$'}numberInt": "42"},
| "int64": {"${'$'}numberLong": "52"},
| "maxKey": {"${'$'}maxKey": 1},
Expand Down Expand Up @@ -218,6 +219,33 @@ class KotlinSerializerCodecTest {
.append("boolean", BsonBoolean.TRUE)
.append("string", BsonString("String")))
}

@JvmStatic
fun testJsonPrimitiveNumberEncoding(): Stream<Pair<String, String>> {
return Stream.of(
"""{"value": 0}""" to """{"value": 0}""",
"""{"value": 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": 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": -0.1}""" to """{"value": -1E-1}""",
"""{"value": -0.01}""" to """{"value": -1E-2}""",
"""{"value": -0.001}""" to """{"value": -1E-3}""",
"""{"value": 9223372036854775807}""" to """{"value": 9223372036854775807}""",
"""{"value": {"${'$'}numberDecimal": "9223372036854775808"}}""" to """{"value": 9223372036854775808}""",
"""{"value": -9223372036854775808}""" to """{"value": -9223372036854775808}""",
"""{"value": {"${'$'}numberDecimal": "-9223372036854775809"}}""" to
"""{"value": -9223372036854775809}""",
)
}
}

@ParameterizedTest
Expand Down Expand Up @@ -832,9 +860,9 @@ class KotlinSerializerCodecTest {
|"short": 1,
|"int": 22,
|"long": {"$numberLong": "3000000000"},
|"decimal": {"$numberDecimal": "10000000000000000000"}
|"decimal2": {"$numberDecimal": "3.1230E+700"}
|"float": 4.0,
|"decimal": {"$numberDecimal": "1E+19"}
|"decimal2": {"$numberDecimal": "3.123E+700"}
|"float": 4.1,
|"double": 4.2,
|"boolean": true,
|"string": "String"
Expand All @@ -849,9 +877,9 @@ class KotlinSerializerCodecTest {
put("short", 1)
put("int", 22)
put("long", 3_000_000_000)
put("decimal", BigDecimal("10000000000000000000"))
put("decimal2", BigDecimal("3.1230E+700"))
put("float", 4.0)
put("decimal", BigDecimal("1E+19"))
put("decimal2", BigDecimal("3.123E+700"))
put("float", 4.1)
put("double", 4.2)
put("boolean", true)
put("string", "String")
Expand Down Expand Up @@ -1023,10 +1051,10 @@ class KotlinSerializerCodecTest {
put("binary", JsonPrimitive("S2Fma2Egcm9ja3Mh"))
put("boolean", JsonPrimitive(true))
put("dateTime", JsonPrimitive(1577836801000))
put("decimal128", JsonPrimitive(1.0))
put("decimal128", JsonPrimitive(1.1))
put("documentEmpty", buildJsonObject {})
put("document", buildJsonObject { put("a", JsonPrimitive(1)) })
put("double", JsonPrimitive(62.0))
put("double", JsonPrimitive(62.1))
put("int32", JsonPrimitive(42))
put("int64", JsonPrimitive(52))
put("objectId", JsonPrimitive("211111111111111111111112"))
Expand All @@ -1050,6 +1078,13 @@ class KotlinSerializerCodecTest {
assertDecodesTo("""{"value": $jsonAllSupportedTypesDocument}""", dataClassWithAllSupportedJsonTypes)
}

@ParameterizedTest
@MethodSource("testJsonPrimitiveNumberEncoding")
fun testJsonPrimitiveNumberEncoding(test: Pair<String, String>) {
val (expected, actual) = test
assertEncodesTo(expected, Json.parseToJsonElement(actual))
}

@Test
fun testDataFailures() {
assertThrows<MissingFieldException>("Missing data") {
Expand Down