From 038502ad366be1af6ad8f206b6237f657e18f0b2 Mon Sep 17 00:00:00 2001 From: Hongbo Zhang Date: Sat, 15 Aug 2026 11:18:53 +0800 Subject: [PATCH 1/3] fix(json): reject unpaired \uXXXX surrogate escapes in strings Fixes #4062. The parser decoded every \uXXXX escape by writing the hex value into the result unchecked (buf.write_char(c.unsafe_to_char())), so "\uD800" manufactured an ill-formed lone-surrogate string out of perfectly valid ASCII JSON input, violating the String well-formedness invariant. An escaped leading surrogate (\uD800-\uDBFF) must now be immediately followed by an escaped trailing surrogate (\uDC00-\uDFFF); the pair is combined into one Unicode scalar value. A bare trailing-surrogate escape, an unpaired leading-surrogate escape, and mixed escaped/raw halves raise the documented ParseError (InvalidChar). Spec position: RFC 8259 section 8.2 flags unpaired surrogate escapes as unpredictable-behavior territory and RFC 7493 (I-JSON) forbids them. Ecosystem: JS JSON.parse and Python accept them, Go substitutes U+FFFD, serde_json rejects - we align with serde_json. BEHAVIOR CHANGE: "\uD800" previously parsed successfully into an ill-formed string and is now a parse error. Valid escaped pairs (\uD83D\uDE00) parse exactly as before. Deterministic regression tests in lex_string_test.mbt cover every unpaired spelling, the exact ParseError shape, and still-accepted valid pairs in both hex digit cases. Co-Authored-By: Claude Fable 5 --- json/lex_string.mbt | 33 ++++++++++++++++++++++++++++++- json/lex_string_test.mbt | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/json/lex_string.mbt b/json/lex_string.mbt index fbaa8d3872..c44090f52f 100644 --- a/json/lex_string.mbt +++ b/json/lex_string.mbt @@ -63,7 +63,38 @@ fn ParseContext::lex_string_slow(ctx : ParseContext) -> String raise ParseError Some('/') => buf.write_char('/') Some('u') => { let c = ctx.lex_hex_digits(4) - buf.write_char(c.unsafe_to_char()) + if c is (0xD800..=0xDBFF) { + // A leading-surrogate escape is only meaningful as the first + // half of an escaped surrogate pair; combine it with the + // immediately following trailing-surrogate escape into one + // Unicode scalar value. Anything else would manufacture a + // string containing an unpaired surrogate, which MoonBit + // strings disallow (RFC 8259 calls the behavior for such + // escapes unpredictable; I-JSON forbids them). + match ctx.read_char() { + Some('\\') => () + Some(_) => ctx.invalid_char(shift=-1) + None => raise InvalidEof + } + match ctx.read_char() { + Some('u') => () + Some(_) => ctx.invalid_char(shift=-1) + None => raise InvalidEof + } + let c2 = ctx.lex_hex_digits(4) + if c2 is (0xDC00..=0xDFFF) { + let combined = (c << 10) + c2 - 0x35fdc00 + buf.write_char(combined.unsafe_to_char()) + } else { + ctx.invalid_char(shift=-1) + } + } else if c is (0xDC00..=0xDFFF) { + // A bare trailing-surrogate escape can never form a scalar + // value. + ctx.invalid_char(shift=-1) + } else { + buf.write_char(c.unsafe_to_char()) + } } Some(_) => ctx.invalid_char(shift=-1) None => raise InvalidEof diff --git a/json/lex_string_test.mbt b/json/lex_string_test.mbt index 31303069cb..5cd3720cd3 100644 --- a/json/lex_string_test.mbt +++ b/json/lex_string_test.mbt @@ -89,3 +89,45 @@ test "lex_hex_digits accepts all hex digit ranges" { ), ) } + +///| +/// Regression for #4062. The parser used to decode every `\uXXXX` escape by +/// writing the hex value into the result unchecked, manufacturing an +/// ill-formed lone-surrogate string out of valid ASCII JSON input. MoonBit +/// strings stay Unicode well-formed, so an escaped leading surrogate must be +/// immediately followed by an escaped trailing surrogate (the pair combines +/// into one scalar value); every unpaired spelling is now a ParseError. +/// RFC 8259 calls the behavior for unpaired surrogate escapes unpredictable +/// and RFC 7493 (I-JSON) forbids them; rejecting matches serde_json. +test "unpaired surrogate escapes are rejected with a clean parse error" { + // An unpaired leading-surrogate escape. + assert_false(@json.valid("\"\\uD800\"")) + assert_false(@json.valid("\"\\uD800x\"")) + assert_false(@json.valid("\"\\uD800\\n\"")) + // A bare trailing-surrogate escape. + assert_false(@json.valid("\"\\uDC00\"")) + assert_false(@json.valid("\"a\\uDFFF b\"")) + // Two leading-surrogate escapes in a row. + assert_false(@json.valid("\"\\uD800\\uD800\"")) + // An escaped pair in reverse order. + assert_false(@json.valid("\"\\uDE00\\uD83D\"")) + // Mixed escaped/raw halves do not pair up. + let lone_low = String::from_array([(0xDC00).unsafe_to_char()]) + assert_false(@json.valid("\"\\uD800" + lone_low + "\"")) + // The failure is the documented ParseError, not an ill-formed string. + debug_inspect( + expect_parse_error("\"\\uDC00\"", "expected InvalidChar"), + content=( + #|InvalidChar({ line: 1, column: 6 }, '0') + ), + ) + // Valid escaped pairs still combine into one scalar value, with any hex + // digit case, in any position. + assert_true(@json.parse("\"\\uD83D\\uDE00\"") == Json::string("\u{1F600}")) + assert_true(@json.parse("\"\\udbff\\udfff\"") == Json::string("\u{10FFFF}")) + assert_true( + @json.parse("\"a\\uD83D\\uDE00b\\n\"") == Json::string("a\u{1F600}b\n"), + ) + // Non-surrogate escapes are unaffected. + assert_true(@json.parse("\"\\u0041\\uFFFD\"") == Json::string("A\u{FFFD}")) +} From cea6d3a073cd34b7c99bf58c1aafc94dc58167a4 Mon Sep 17 00:00:00 2001 From: Hongbo Zhang Date: Sun, 16 Aug 2026 16:13:10 +0800 Subject: [PATCH 2/3] fix(json): address review feedback on surrogate escape rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the contract, fixes where the error points, and splits the regression test. The accepted language is now written down. `parse` says that every string in the result is well-formed Unicode, that an escaped leading surrogate must be followed by an escaped trailing one, and — the part that was missing — that this is a limit on what a string may *contain*, which RFC 8259 §9 leaves to the implementation. It is not a claim about which documents are grammatically well formed (§8.2 admits these escapes) nor of I-JSON conformance, which restricts more than this. The reason is stated too: `String` is required to be well-formed, so the alternatives are handing back one that is not, or substituting U+FFFD and silently merging two distinct keys. `valid` now defines validity as "what `parse` accepts" instead of leaving it open. The README gains the same in short form with a runnable example, and CHANGELOG records the break, including that JavaScript's `JSON.stringify` emits these escapes so input other parsers accept can now be rejected. The error position had two problems. It named whichever character the scan stopped on, so `"\uDC00"` blamed the final, valid, hex digit; and it was computed as `shift=-1` after `read_char()`, which lands inside a non-BMP character — `"\uD800😀"` reported a broken half one column too far. Both are replaced by one documented convention: the position is the backslash that opens the offending escape, wherever the scan stopped. `lex_hex_digits` and the unknown-escape arm had the same `-1` defect for a non-BMP character and now use `-c.utf16_len()`, as `lex_main.mbt` already did. The single regression test became six, so one failing assertion can no longer mask the rest, and the matrix grew: U+10000 as well as U+10FFFF, EOF and malformed or short hex in the second escape, adjacent pairs, object keys as the other route into string lexing, and exact error assertions pinning the position convention. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + json/README.mbt.md | 27 ++++++++++ json/lex_string.mbt | 33 +++++++++--- json/lex_string_test.mbt | 105 +++++++++++++++++++++++++++++++-------- json/parse.mbt | 28 ++++++++++- 5 files changed, 166 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 290ec431f3..64d8420c08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ changelog should follow [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) #### Changed +- **BREAKING**: `@json.parse` now rejects unpaired `\uXXXX` surrogate escapes in strings, raising `ParseError::InvalidChar` at the backslash that opens the offending escape; an escaped leading surrogate must be followed immediately by an escaped trailing surrogate, and the pair decodes to the character it denotes. Previously such escapes were decoded unchecked and produced a `String` that was not well-formed Unicode. `@json.valid` reports the same documents as invalid. JSON emitted by `JSON.stringify` in JavaScript can contain these escapes, so input that JavaScript and Python accept may now be rejected — as it is by Rust's serde_json - `@json.inspect` has been migrated to `json_inspect` - `String::sub` and `StringView::sub` now panic on invalid indices instead of raising `CreatingViewError`. The `CreatingViewError` type has been removed. diff --git a/json/README.mbt.md b/json/README.mbt.md index e33bf48baf..0c7b1db096 100644 --- a/json/README.mbt.md +++ b/json/README.mbt.md @@ -51,6 +51,33 @@ test "parse and validate jsons" { } ``` +#### What may appear inside a string + +Every string in a parsed document is well-formed Unicode, so each `\uXXXX` +escape has to denote a Unicode scalar value. An escaped leading surrogate +must be followed immediately by an escaped trailing surrogate, and the pair +decodes to the single character it stands for; an escape that cannot pair up +is a parse error. + +```mbt check +///| +test "surrogate escapes" { + // A surrogate pair decodes to the one character it denotes. + assert_true(@json.parse("\"\\uD83D\\uDE00\"") == Json::string("😀")) + // An escape that cannot pair up is rejected rather than producing a + // string that is not well-formed Unicode. + assert_false(@json.valid("\"\\uD800\"")) +} +``` + +RFC 8259 §9 leaves what a string may contain to the implementation, and this +is where MoonBit draws that line: a `String` is required to be well-formed, +so the alternatives would be to hand one back that is not, or to substitute +U+FFFD and lose the difference between two distinct keys. Note that +`JSON.stringify` in JavaScript does emit lone surrogates this way, so a +document JavaScript and Python accept can be rejected here — as it is by +Rust's serde_json; Go substitutes U+FFFD instead. + ### Object Navigation ```mbt check diff --git a/json/lex_string.mbt b/json/lex_string.mbt index c44090f52f..3db187efe3 100644 --- a/json/lex_string.mbt +++ b/json/lex_string.mbt @@ -62,6 +62,9 @@ fn ParseContext::lex_string_slow(ctx : ParseContext) -> String raise ParseError Some('\\') => buf.write_char('\\') Some('/') => buf.write_char('/') Some('u') => { + // The backslash that opened this escape. `ctx.offset` is just + // past the `u`, and `\` and `u` are one code unit each. + let escape_start = ctx.offset - 2 let c = ctx.lex_hex_digits(4) if c is (0xD800..=0xDBFF) { // A leading-surrogate escape is only meaningful as the first @@ -73,12 +76,12 @@ fn ParseContext::lex_string_slow(ctx : ParseContext) -> String raise ParseError // escapes unpredictable; I-JSON forbids them). match ctx.read_char() { Some('\\') => () - Some(_) => ctx.invalid_char(shift=-1) + Some(_) => ctx.unpaired_surrogate(escape_start) None => raise InvalidEof } match ctx.read_char() { Some('u') => () - Some(_) => ctx.invalid_char(shift=-1) + Some(_) => ctx.unpaired_surrogate(escape_start) None => raise InvalidEof } let c2 = ctx.lex_hex_digits(4) @@ -86,17 +89,17 @@ fn ParseContext::lex_string_slow(ctx : ParseContext) -> String raise ParseError let combined = (c << 10) + c2 - 0x35fdc00 buf.write_char(combined.unsafe_to_char()) } else { - ctx.invalid_char(shift=-1) + ctx.unpaired_surrogate(escape_start) } } else if c is (0xDC00..=0xDFFF) { // A bare trailing-surrogate escape can never form a scalar // value. - ctx.invalid_char(shift=-1) + ctx.unpaired_surrogate(escape_start) } else { buf.write_char(c.unsafe_to_char()) } } - Some(_) => ctx.invalid_char(shift=-1) + Some(c) => ctx.invalid_char(shift=-c.utf16_len()) None => raise InvalidEof } start = ctx.offset @@ -113,6 +116,22 @@ fn ParseContext::lex_string_slow(ctx : ParseContext) -> String raise ParseError buf.to_string() } +///| +/// Reports the `\uXXXX` escape that begins at `escape_start` as invalid. +/// +/// Every rejection of an unpaired surrogate points here — at the backslash +/// opening the offending escape — rather than at whichever character the +/// scan happened to stop on. Blaming the stopping point would name a +/// perfectly valid hex digit for `"\uDC00"`, and for a leading surrogate +/// followed by a non-BMP character it would name a position inside that +/// character. +fn[T] ParseContext::unpaired_surrogate( + ctx : ParseContext, + escape_start : Int, +) -> T raise ParseError { + ctx.invalid_char(shift=escape_start - ctx.offset) +} + ///| fn ParseContext::lex_hex_digits( ctx : ParseContext, @@ -123,7 +142,9 @@ fn ParseContext::lex_hex_digits( Some('0'..='9' as c) => c.to_int() - '0' Some('A'..='F' as c) => c.to_int() - 'A' + 10 Some('a'..='f' as c) => c.to_int() - 'a' + 10 - Some(_) => ctx.invalid_char(shift=-1) + // `-1` would land inside the character when it is not in the BMP, + // reporting a broken half at the wrong column. + Some(c) => ctx.invalid_char(shift=-c.utf16_len()) None => raise InvalidEof } continue (r << 4) | d diff --git a/json/lex_string_test.mbt b/json/lex_string_test.mbt index 5cd3720cd3..84cead18a1 100644 --- a/json/lex_string_test.mbt +++ b/json/lex_string_test.mbt @@ -90,44 +90,107 @@ test "lex_hex_digits accepts all hex digit ranges" { ) } +// Regression for #4062. The parser used to decode every `\uXXXX` escape by +// writing the hex value into the result unchecked, manufacturing an +// ill-formed lone-surrogate string out of valid ASCII JSON input. MoonBit +// strings stay well-formed Unicode, so an escaped leading surrogate must be +// followed immediately by an escaped trailing surrogate — the pair decodes +// to the one character it denotes — and every unpaired spelling is a +// `ParseError`. +// +// The cases are split by shape so that one failing assertion cannot mask the +// rest; the old parser accepted all of the rejections below, and aborted +// outright on the escaped-leading + raw-trailing one. + ///| -/// Regression for #4062. The parser used to decode every `\uXXXX` escape by -/// writing the hex value into the result unchecked, manufacturing an -/// ill-formed lone-surrogate string out of valid ASCII JSON input. MoonBit -/// strings stay Unicode well-formed, so an escaped leading surrogate must be -/// immediately followed by an escaped trailing surrogate (the pair combines -/// into one scalar value); every unpaired spelling is now a ParseError. -/// RFC 8259 calls the behavior for unpaired surrogate escapes unpredictable -/// and RFC 7493 (I-JSON) forbids them; rejecting matches serde_json. -test "unpaired surrogate escapes are rejected with a clean parse error" { - // An unpaired leading-surrogate escape. +test "unpaired leading-surrogate escape is rejected" { + // Nothing after it at all, and a closing quote after it. + assert_false(@json.valid("\"\\uD800")) assert_false(@json.valid("\"\\uD800\"")) + // A character that is not the start of an escape. assert_false(@json.valid("\"\\uD800x\"")) + // An escape that is not `\u`. assert_false(@json.valid("\"\\uD800\\n\"")) - // A bare trailing-surrogate escape. - assert_false(@json.valid("\"\\uDC00\"")) - assert_false(@json.valid("\"a\\uDFFF b\"")) // Two leading-surrogate escapes in a row. assert_false(@json.valid("\"\\uD800\\uD800\"")) - // An escaped pair in reverse order. + // A second escape that is present but malformed or cut short. + assert_false(@json.valid("\"\\uD800\\uZZZZ\"")) + assert_false(@json.valid("\"\\uD800\\u00\"")) + assert_false(@json.valid("\"\\uD800\\u\"")) + assert_false(@json.valid("\"\\uD800\\u")) +} + +///| +test "bare trailing-surrogate escape is rejected" { + assert_false(@json.valid("\"\\uDC00\"")) + assert_false(@json.valid("\"a\\uDFFF b\"")) + // An escaped pair in reverse order: the trailing half is bare. assert_false(@json.valid("\"\\uDE00\\uD83D\"")) - // Mixed escaped/raw halves do not pair up. +} + +///| +test "escaped and raw surrogate halves do not pair up" { let lone_low = String::from_array([(0xDC00).unsafe_to_char()]) assert_false(@json.valid("\"\\uD800" + lone_low + "\"")) - // The failure is the documented ParseError, not an ill-formed string. +} + +///| +test "unpaired surrogate escapes are rejected in object keys too" { + assert_false(@json.valid("{\"\\uD800\": 1}")) + assert_false(@json.valid("{\"a\": 1, \"\\uDC00\": 2}")) + // ...and a valid pair in a key still works. + assert_true(@json.valid("{\"\\uD83D\\uDE00\": 1}")) +} + +///| +test "the surrogate error names the escape that could not pair up" { + // The position is the backslash opening the offending escape, not + // whichever character the scan stopped on — that would blame the last, + // perfectly valid, hex digit. Columns count code units from zero, so the + // backslash after the opening quote is column 1. debug_inspect( expect_parse_error("\"\\uDC00\"", "expected InvalidChar"), content=( - #|InvalidChar({ line: 1, column: 6 }, '0') + #|InvalidChar({ line: 1, column: 1 }, '\\') ), ) - // Valid escaped pairs still combine into one scalar value, with any hex - // digit case, in any position. - assert_true(@json.parse("\"\\uD83D\\uDE00\"") == Json::string("\u{1F600}")) + debug_inspect( + expect_parse_error("\"ab\\uD800x\"", "expected InvalidChar"), + content=( + #|InvalidChar({ line: 1, column: 3 }, '\\') + ), + ) + // A non-BMP character after the escape occupies two code units; the + // reported column must not land inside it. + debug_inspect( + expect_parse_error("\"\\uD800\u{1F600}\"", "expected InvalidChar"), + content=( + #|InvalidChar({ line: 1, column: 1 }, '\\') + ), + ) + // Running out of input is still an EOF error rather than a character one. + debug_inspect( + expect_parse_error("\"\\uD800", "expected InvalidEof"), + content="InvalidEof", + ) +} + +///| +test "valid surrogate pairs decode to one scalar value" { + // Both ends of the supplementary range. + assert_true(@json.parse("\"\\uD800\\uDC00\"") == Json::string("\u{10000}")) assert_true(@json.parse("\"\\udbff\\udfff\"") == Json::string("\u{10FFFF}")) + // Any hex digit case, and in any position within the string. + assert_true(@json.parse("\"\\uD83D\\uDE00\"") == Json::string("\u{1F600}")) assert_true( @json.parse("\"a\\uD83D\\uDE00b\\n\"") == Json::string("a\u{1F600}b\n"), ) - // Non-surrogate escapes are unaffected. + // Two pairs running together are decoded independently. + assert_true( + @json.parse("\"\\uD83D\\uDE00\\uD83D\\uDE01\"") == + Json::string("\u{1F600}\u{1F601}"), + ) + // Non-surrogate escapes are unaffected, including the replacement + // character, which is a scalar value like any other. assert_true(@json.parse("\"\\u0041\\uFFFD\"") == Json::string("A\u{FFFD}")) } diff --git a/json/parse.mbt b/json/parse.mbt index 174e361dd4..47f2dea568 100644 --- a/json/parse.mbt +++ b/json/parse.mbt @@ -13,7 +13,10 @@ // limitations under the License. ///| -/// Validate input and return whether it is valid. +/// Returns whether `input` parses. "Valid" here means exactly what `parse` +/// accepts, so it includes the restriction on string contents described +/// there: a document whose only defect is an unpaired surrogate escape is +/// reported as invalid. pub fn valid(input : StringView) -> Bool { try { parse(input) |> ignore @@ -25,6 +28,29 @@ pub fn valid(input : StringView) -> Bool { ///| /// Parse a JSON input string into a Json value, with an optional maximum nesting depth (default is 1024) +/// +/// ## What strings may contain +/// +/// Every string in the result is well-formed Unicode: each `\uXXXX` escape +/// has to denote a Unicode scalar value. An escaped leading surrogate +/// (`\uD800`–`\uDBFF`) must therefore be followed immediately by an escaped +/// trailing surrogate (`\uDC00`–`\uDFFF`), and the pair is decoded as the +/// one character it stands for; an escape that cannot pair up raises +/// `InvalidChar`, positioned at the backslash that opens it. +/// +/// This is a limit on what a string may *contain*, which RFC 8259 §9 leaves +/// to the implementation — not a claim about which documents are +/// grammatically well formed, since §8.2 admits unpaired surrogate escapes, +/// nor a claim of I-JSON (RFC 7493) conformance, which restricts more than +/// this. It is chosen because MoonBit's `String` is required to be +/// well-formed, so the alternatives are to hand back a string that violates +/// that invariant, or to substitute U+FFFD and silently lose the +/// distinction between two different keys. A parse error is the only one of +/// the three a caller can see and act on. +/// +/// The cost is real: `JSON.stringify` in JavaScript emits lone surrogates as +/// `\uXXXX`, so some JSON that JavaScript and Python accept is rejected +/// here. Rust's serde_json rejects it too; Go substitutes U+FFFD. #label_migration(max_nesting_depth, fill=false) pub fn parse( input : StringView, From 701b9eff0fdda419193706ba39e87758f37312c4 Mon Sep 17 00:00:00 2001 From: Hongbo Zhang Date: Sun, 16 Aug 2026 16:26:30 +0800 Subject: [PATCH 3/3] fix(json): tighten the surrogate docs and error coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up nits from review. Two doc claims were imprecise. "Each escape must denote a scalar value" is not true of either half of an accepted pair, so it now says an escape must denote a scalar on its own *or* be one half of a correctly ordered pair. And the promise that a failed pairing always raises `InvalidChar` at the backslash overstated it: running out of input still raises `InvalidEof`, and a malformed second escape is reported as the hex-digit error it is. Both are qualified now. The comparison to other parsers is narrowed too — serde_json rejects when parsing into `String`/`Value` while its byte mode admits WTF-8, and it is Go's `encoding/json` that substitutes U+FFFD, its v2 parser being stricter. The test preamble claimed the old parser accepted every rejection below it, which was false: the ones that also run out of input or misspell the second escape were already errors. Corrected, and the escaped-leading + raw-trailing case moved last, since that is the one that aborted the process outright before this change and would otherwise mask the blocks after it. Added the exact assertions that were missing: the second mismatch arm, a second escape that is well formed but is not a trailing surrogate, and a trailing backslash at EOF — all pinning the one documented position. Also covers the two arms whose `shift` was corrected, where an unknown escape naming a non-BMP character and a non-BMP character among hex digits are now reported whole rather than as a broken half one column further on. Co-Authored-By: Claude Opus 5 (1M context) --- json/README.mbt.md | 12 ++++--- json/lex_string_test.mbt | 67 +++++++++++++++++++++++++++++++++------- json/parse.mbt | 16 +++++++--- 3 files changed, 75 insertions(+), 20 deletions(-) diff --git a/json/README.mbt.md b/json/README.mbt.md index 0c7b1db096..b34fc8b30e 100644 --- a/json/README.mbt.md +++ b/json/README.mbt.md @@ -54,10 +54,11 @@ test "parse and validate jsons" { #### What may appear inside a string Every string in a parsed document is well-formed Unicode, so each `\uXXXX` -escape has to denote a Unicode scalar value. An escaped leading surrogate -must be followed immediately by an escaped trailing surrogate, and the pair -decodes to the single character it stands for; an escape that cannot pair up -is a parse error. +escape must denote a Unicode scalar value on its own or be one half of a +correctly ordered surrogate pair. An escaped leading surrogate must be +followed immediately by an escaped trailing surrogate, and the pair decodes +to the single character it stands for; an escape that cannot pair up is a +parse error, reported at the backslash that opens it. ```mbt check ///| @@ -76,7 +77,8 @@ so the alternatives would be to hand one back that is not, or to substitute U+FFFD and lose the difference between two distinct keys. Note that `JSON.stringify` in JavaScript does emit lone surrogates this way, so a document JavaScript and Python accept can be rejected here — as it is by -Rust's serde_json; Go substitutes U+FFFD instead. +Rust's serde_json when parsing into `String` or `Value`; Go's +`encoding/json` substitutes U+FFFD instead. ### Object Navigation diff --git a/json/lex_string_test.mbt b/json/lex_string_test.mbt index 84cead18a1..57cd6b9af1 100644 --- a/json/lex_string_test.mbt +++ b/json/lex_string_test.mbt @@ -99,14 +99,17 @@ test "lex_hex_digits accepts all hex digit ranges" { // `ParseError`. // // The cases are split by shape so that one failing assertion cannot mask the -// rest; the old parser accepted all of the rejections below, and aborted -// outright on the escaped-leading + raw-trailing one. +// rest. The old parser accepted the *unpaired-surrogate* spellings below — +// it already rejected the ones that also run out of input or misspell the +// second escape — and aborted the process outright on the escaped-leading + +// raw-trailing one, which is why that case is kept last. ///| test "unpaired leading-surrogate escape is rejected" { - // Nothing after it at all, and a closing quote after it. - assert_false(@json.valid("\"\\uD800")) + // A closing quote after it, and nothing after it at all. (The second was + // already an EOF error before the surrogate rule.) assert_false(@json.valid("\"\\uD800\"")) + assert_false(@json.valid("\"\\uD800")) // A character that is not the start of an escape. assert_false(@json.valid("\"\\uD800x\"")) // An escape that is not `\u`. @@ -128,12 +131,6 @@ test "bare trailing-surrogate escape is rejected" { assert_false(@json.valid("\"\\uDE00\\uD83D\"")) } -///| -test "escaped and raw surrogate halves do not pair up" { - let lone_low = String::from_array([(0xDC00).unsafe_to_char()]) - assert_false(@json.valid("\"\\uD800" + lone_low + "\"")) -} - ///| test "unpaired surrogate escapes are rejected in object keys too" { assert_false(@json.valid("{\"\\uD800\": 1}")) @@ -168,11 +165,50 @@ test "the surrogate error names the escape that could not pair up" { #|InvalidChar({ line: 1, column: 1 }, '\\') ), ) - // Running out of input is still an EOF error rather than a character one. + // The same position whichever way the pairing fails: a follower that is + // not a backslash, a backslash not followed by `u`, and a second escape + // that is well formed but is not a trailing surrogate. + debug_inspect( + expect_parse_error("\"\\uD800\\n\"", "expected InvalidChar"), + content=( + #|InvalidChar({ line: 1, column: 1 }, '\\') + ), + ) + debug_inspect( + expect_parse_error("\"\\uD800\\u0041\"", "expected InvalidChar"), + content=( + #|InvalidChar({ line: 1, column: 1 }, '\\') + ), + ) + // Running out of input is still an EOF error rather than a character one, + // including on a trailing backslash where the second escape should start. debug_inspect( expect_parse_error("\"\\uD800", "expected InvalidEof"), content="InvalidEof", ) + debug_inspect( + expect_parse_error("\"\\uD800\\", "expected InvalidEof"), + content="InvalidEof", + ) +} + +///| +test "an escape naming a non-BMP character reports it whole" { + // These two arms read a character and then step back by its width. Using + // a fixed step of one code unit landed inside a non-BMP character and + // reported a broken half one column further on. + debug_inspect( + expect_parse_error("\"\\\u{1F600}\"", "expected InvalidChar"), + content=( + #|InvalidChar({ line: 1, column: 2 }, '😀') + ), + ) + debug_inspect( + expect_parse_error("\"\\u1\u{1F600}23\"", "expected InvalidChar"), + content=( + #|InvalidChar({ line: 1, column: 4 }, '😀') + ), + ) } ///| @@ -194,3 +230,12 @@ test "valid surrogate pairs decode to one scalar value" { // character, which is a scalar value like any other. assert_true(@json.parse("\"\\u0041\\uFFFD\"") == Json::string("A\u{FFFD}")) } + +///| +test "escaped and raw surrogate halves do not pair up" { + // Kept last: before the surrogate rule this input aborted the process + // rather than raising, so a regression here would take the whole test + // binary with it. + let lone_low = String::from_array([(0xDC00).unsafe_to_char()]) + assert_false(@json.valid("\"\\uD800" + lone_low + "\"")) +} diff --git a/json/parse.mbt b/json/parse.mbt index 47f2dea568..a6acc6d16f 100644 --- a/json/parse.mbt +++ b/json/parse.mbt @@ -32,11 +32,17 @@ pub fn valid(input : StringView) -> Bool { /// ## What strings may contain /// /// Every string in the result is well-formed Unicode: each `\uXXXX` escape -/// has to denote a Unicode scalar value. An escaped leading surrogate +/// must denote a Unicode scalar value on its own, or be one half of a +/// correctly ordered surrogate pair. An escaped leading surrogate /// (`\uD800`–`\uDBFF`) must therefore be followed immediately by an escaped /// trailing surrogate (`\uDC00`–`\uDFFF`), and the pair is decoded as the -/// one character it stands for; an escape that cannot pair up raises -/// `InvalidChar`, positioned at the backslash that opens it. +/// one character it stands for. +/// +/// An escape that cannot pair up raises `InvalidChar` positioned at the +/// backslash that opens it — that one position, whatever the scan actually +/// stopped on. Input that simply runs out still raises `InvalidEof`, and a +/// second escape that is itself malformed is reported as the hex-digit +/// error it is, at the offending digit. /// /// This is a limit on what a string may *contain*, which RFC 8259 §9 leaves /// to the implementation — not a claim about which documents are @@ -50,7 +56,9 @@ pub fn valid(input : StringView) -> Bool { /// /// The cost is real: `JSON.stringify` in JavaScript emits lone surrogates as /// `\uXXXX`, so some JSON that JavaScript and Python accept is rejected -/// here. Rust's serde_json rejects it too; Go substitutes U+FFFD. +/// here. Rust's serde_json rejects it too when parsing into `String` or +/// `Value`, though its byte-oriented mode admits WTF-8; Go's `encoding/json` +/// substitutes U+FFFD, while its experimental v2 parser is stricter. #label_migration(max_nesting_depth, fill=false) pub fn parse( input : StringView,