Skip to content

Commit 6af3721

Browse files
bobzhangclaude
andcommitted
fix(json): preserve -0 sign and stop panicking on lone-surrogate strings
Two parser bugs found by adversarial QuickCheck testing: - #4049: parse aborted (panic, not ParseError) on strings that mix a raw lone trailing surrogate with any escape sequence, e.g. the 5-code-unit text " U+DC00 \n ": lex_string_slow's flush sliced with the checked ctx.input[start:end], which aborts when the code unit at a boundary is a trailing surrogate. Use view(start_offset~, end_offset~) (bounds checks only), matching the fast path in lex_string, so such strings parse successfully instead of crashing the process. - #4053: parse("-0") returned +0.0 while parse("-0.0") / parse("-0e0") returned -0.0: the integer fast path in lex_number_end negated the mantissa as an Int64 (where -0 == 0) before converting to Double. Negate after the conversion so every spelling of negative zero keeps the IEEE-754 sign bit, per RFC 8259 number semantics. Both fixes are backend-independent (wasm-gc, js, native) and covered by deterministic regression tests in lex_string_test.mbt and lex_number_test.mbt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d5a4518 commit 6af3721

4 files changed

Lines changed: 62 additions & 5 deletions

File tree

json/lex_number.mbt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,12 +399,16 @@ fn ParseContext::lex_number_end(
399399
// returned Double lossless. `reinterpret_as_uint64` / `reinterpret_as_int64`
400400
// are value-preserving here because both operands sit in [0, 2^53), well
401401
// inside the overlap of Int64+ and UInt64.
402+
//
403+
// The sign is applied after the Int64 -> Double conversion so that `-0`
404+
// parses to the IEEE-754 negative zero (`-(0L)` is still `0L`, but
405+
// `-(0.0)` is `-0.0`), matching the `-0.0` / `-0e0` paths below.
402406
if !scan.many_digits &&
403407
scan.exponent == 0L &&
404408
scan.mantissa <= SAFE_INTEGER_LIMIT.reinterpret_as_uint64() {
405-
let v = scan.mantissa.reinterpret_as_int64()
406-
let signed = if scan.negative { -v } else { v }
407-
return { value: signed.to_double(), repr: None }
409+
let v = scan.mantissa.reinterpret_as_int64().to_double()
410+
let value = if scan.negative { -v } else { v }
411+
return { value, repr: None }
408412
}
409413
return ctx.lex_integer_end(start, end)
410414
}

json/lex_number_test.mbt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,27 @@ test "parse number with huge exponent" {
170170
),
171171
)
172172
}
173+
174+
///|
175+
/// Every spelling of a negative zero — including values that underflow to
176+
/// zero — must keep the IEEE-754 sign bit. The integer spelling `-0` used to
177+
/// lose it: the integer fast path negated an `Int64` (where `-0 == 0`)
178+
/// before converting to `Double`.
179+
test "parse preserves the sign of negative zero" {
180+
fn is_negative(text : String) -> Bool raise {
181+
guard @json.parse(text) is Number(n, ..) else { fail("not a number") }
182+
n.reinterpret_as_int64() < 0L
183+
}
184+
185+
// Zero literals.
186+
assert_true(is_negative("-0"))
187+
assert_true(is_negative("-0.0"))
188+
assert_true(is_negative("-0e0"))
189+
assert_true(is_negative("-0.00E-7"))
190+
assert_false(is_negative("0"))
191+
assert_false(is_negative("0.0"))
192+
// Negative values that underflow to zero.
193+
assert_true(is_negative("-1e-400"))
194+
assert_true(is_negative("-4.9e-325"))
195+
assert_true(is_negative("-1e-999999999999999999999999999999999999"))
196+
}

json/lex_string.mbt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,12 @@ fn ParseContext::lex_string_slow(ctx : ParseContext) -> String raise ParseError
3939
let buf = StringBuilder()
4040
let mut start = ctx.offset
4141
fn flush(end : Int) {
42-
if start > 0 && end > start {
43-
buf.write_view(ctx.input[start:end])
42+
if end > start {
43+
// `view(start_offset~, end_offset~)` only bounds-checks; the checked
44+
// `ctx.input[start:end]` aborts when the code unit at a boundary is a
45+
// trailing surrogate, which a raw lone surrogate inside the string
46+
// (accepted by the fast path in `lex_string`) can trigger here.
47+
buf.write_view(ctx.input.view(start_offset=start, end_offset=end))
4448
}
4549
}
4650

json/lex_string_test.mbt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,28 @@ test "lex_hex_digits accepts all hex digit ranges" {
8989
),
9090
)
9191
}
92+
93+
///|
94+
/// Regression for #4049: a raw lone surrogate combined with an escape used to
95+
/// abort the process instead of parsing. The slow-path lexer's `flush` sliced
96+
/// the pending run with the checked `ctx.input[start:end]`, which panics when
97+
/// the code unit at a slice boundary is a trailing surrogate; the escape-free
98+
/// fast path already accepted the same strings.
99+
test "lone surrogates with escapes parse instead of aborting" {
100+
let lone_low = String::from_array([(0xDC00).unsafe_to_char()])
101+
let lone_high = String::from_array([(0xD800).unsafe_to_char()])
102+
// Lone trailing surrogate before / after an escape.
103+
assert_true(
104+
@json.parse("\"" + lone_low + "\\n\"") == Json::string(lone_low + "\n"),
105+
)
106+
assert_true(
107+
@json.parse("\"\\n" + lone_low + "\"") == Json::string("\n" + lone_low),
108+
)
109+
// Lone leading surrogate with an escape.
110+
assert_true(
111+
@json.parse("\"" + lone_high + "\\t\"") == Json::string(lone_high + "\t"),
112+
)
113+
// Surrogates on both sides of an escaped backslash, via stringify/parse.
114+
let json = Json::string(lone_low + "\\" + lone_high)
115+
assert_true(@json.parse(json.stringify()) == json)
116+
}

0 commit comments

Comments
 (0)