Skip to content

Commit 8430930

Browse files
JoshRosenclaude
andauthored
Fix %d/%o/%x overflow for large doubles (#605)
This PR fixes an overflow bug when large doubles are used in jsonnet string formatting: - `%d`, `%o`, and `%x` format specs used `Double.toLong` to convert to an integer, which silently clamps to `Long.MAX_VALUE` (~9.2e18) for large double values like `1e19` or `1e20` - Add a `truncateToInteger` helper (mirroring the idiom in [`RenderUtils.renderDouble`](https://github.com/databricks/sjsonnet/blame/9bd52b5ea0564cbb532d72bc345a65f54233c2d3/sjsonnet/src/sjsonnet/Renderer.scala#L256-L264)) that uses `Long` as a fast path and falls back to `BigDecimal.toBigInt` for values exceeding `Long` range - Sign and digits both derive from the truncated `BigInt`, so edge cases like `-0.3` (which truncates to `0`) are handled naturally. **Test plan** - Added unit tests for `%d`, `%o`, `%x` with values exceeding Long range (`1e19`, `1e20`, `-1e19`) - Added edge cases for fractional negatives (`-0.3`, `-0.9`) that truncate to zero - Expected values verified against official C and Go jsonnet implementations - Existing `format.jsonnet` test suite covers format strings that produce outputs with leading negative signs. --- All code written by Claude Opus 4.6. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 77af254 commit 8430930

2 files changed

Lines changed: 42 additions & 19 deletions

File tree

sjsonnet/src/sjsonnet/Format.scala

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -236,21 +236,28 @@ object Format {
236236
output.toString()
237237
}
238238

239-
private def formatInteger(formatted: FormatSpec, s: Double): String = {
239+
// Truncate a double toward zero, returning the integer part as a BigInt.
240+
// Uses Long as a fast path (mirrors RenderUtils.renderDouble); falls back to
241+
// BigDecimal for values that exceed Long range (~9.2e18).
242+
private def truncateToInteger(s: Double): BigInt = {
240243
val sl = s.toLong
241-
val (lhs, rhs) = if (sl < 0) {
242-
("-", sl.toString.substring(1))
243-
} else {
244-
("", sl.toString)
245-
}
244+
if (sl.toDouble == s) BigInt(sl)
245+
else BigDecimal(s).toBigInt
246+
}
247+
248+
private def formatInteger(formatted: FormatSpec, s: Double): String = {
249+
val i = truncateToInteger(s)
250+
val negative = i.signum < 0
251+
val lhs = if (negative) "-" else ""
252+
val rhs = i.abs.toString(10)
246253
val rhs2 = precisionPad(lhs, rhs, formatted.precision)
247254
widen(
248255
formatted,
249256
lhs,
250257
"",
251258
rhs2,
252259
numeric = true,
253-
signedConversion = sl >= 0
260+
signedConversion = !negative
254261
)
255262
}
256263

@@ -275,36 +282,34 @@ object Format {
275282
}
276283

277284
private def formatOctal(formatted: FormatSpec, s: Double): String = {
278-
val (lhs, rhs) = if (s < 0) {
279-
("-", s.toLong.abs.toOctalString)
280-
} else {
281-
("", s.toLong.toOctalString)
282-
}
285+
val i = truncateToInteger(s)
286+
val negative = i.signum < 0
287+
val lhs = if (negative) "-" else ""
288+
val rhs = i.abs.toString(8)
283289
val rhs2 = precisionPad(lhs, rhs, formatted.precision)
284290
widen(
285291
formatted,
286292
lhs,
287293
if (!formatted.alternate || rhs2(0) == '0') "" else "0",
288294
rhs2,
289295
numeric = true,
290-
signedConversion = s > 0
296+
signedConversion = !negative
291297
)
292298
}
293299

294300
private def formatHexadecimal(formatted: FormatSpec, s: Double): String = {
295-
val (lhs, rhs) = if (s < 0) {
296-
("-", s.toLong.abs.toHexString)
297-
} else {
298-
("", s.toLong.toHexString)
299-
}
301+
val i = truncateToInteger(s)
302+
val negative = i.signum < 0
303+
val lhs = if (negative) "-" else ""
304+
val rhs = i.abs.toString(16)
300305
val rhs2 = precisionPad(lhs, rhs, formatted.precision)
301306
widen(
302307
formatted,
303308
lhs,
304309
if (!formatted.alternate) "" else "0x",
305310
rhs2,
306311
numeric = true,
307-
signedConversion = s > 0
312+
signedConversion = !negative
308313
)
309314
}
310315

sjsonnet/test/src/sjsonnet/EvaluatorTests.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,24 @@ object EvaluatorTests extends TestSuite {
572572
"-123.45600"
573573
)
574574
}
575+
test("formatIntegerOverflow") {
576+
// %d, %o, %x should not overflow to Long.MAX_VALUE for large doubles
577+
eval("'%d' % 1e19", useNewEvaluator = useNewEvaluator) ==> ujson.Str("10000000000000000000")
578+
eval("'%d' % 1e20", useNewEvaluator = useNewEvaluator) ==> ujson.Str(
579+
"100000000000000000000"
580+
)
581+
eval("'%d' % -1e19", useNewEvaluator = useNewEvaluator) ==> ujson.Str(
582+
"-10000000000000000000"
583+
)
584+
eval("'%o' % 1e19", useNewEvaluator = useNewEvaluator) ==> ujson.Str(
585+
"1053071060221172000000"
586+
)
587+
eval("'%x' % 1e19", useNewEvaluator = useNewEvaluator) ==> ujson.Str("8ac7230489e80000")
588+
// Sign should be determined from the truncated integer, not the original double:
589+
// -0.3 truncates to 0, so no minus sign should appear.
590+
eval("'%5.3d' % -0.3", useNewEvaluator = useNewEvaluator) ==> ujson.Str(" 000")
591+
eval("'%d' % -0.9", useNewEvaluator = useNewEvaluator) ==> ujson.Str("0")
592+
}
575593
test("strict") {
576594
eval("({ a: 1 } { b: 2 }).a", strict = false, useNewEvaluator = useNewEvaluator) ==> ujson
577595
.Num(1)

0 commit comments

Comments
 (0)