ReviewOS

zig-utils/zig-js

Errors: name the JSON.parse fault the way JavaScriptCore does

#898
Closed chris opened this 11 days ago ·
11 days ago

Parent: #459

Problem

JSON.parse collapses every syntax fault into one string. JavaScriptCore names the fault. A 40-case differential against a real JSC backend matches 1/40:

inputzig-jsJavaScriptCore
''JSON.parse: invalid JSONJSON Parse error: Unexpected EOF
'{'JSON.parse: invalid JSONJSON Parse error: Expected '}'
'{"a"}'JSON.parse: invalid JSONJSON Parse error: Expected ':' before value in object property definition
"{'a':1}"JSON.parse: invalid JSONJSON Parse error: Single quotes (') are not allowed in JSON
'[1,]'JSON.parse: invalid JSONJSON Parse error: Unexpected comma at the end of array expression
'{"a":1,}'JSON.parse: invalid JSONJSON Parse error: Property name must be a string literal
'NaN'JSON.parse: invalid JSONJSON Parse error: Unexpected identifier "NaN"
'"abc'JSON.parse: invalid JSONJSON Parse error: Unterminated string
'1 2'JSON.parse: trailing charactersJSON Parse error: Unable to parse JSON string

JSON.stringify on a cycle is also wrong, and interestingly matches V8 rather than JSC: zig-js says Converting circular structure to JSON, JSC says JSON.stringify cannot serialize cyclic structures.

The complete JavaScriptCore vocabulary

Every message is prefixed JSON Parse error: . Measured, not guessed:

Unexpected EOF
Unable to parse JSON string                  (trailing content after a complete value)
Expected '}'  /  Expected ']'
Expected ':' before value in object property definition
Property name must be a string literal
Single quotes (') are not allowed in JSON
Unexpected comma at the end of array expression
Unexpected token '<c>'                       (parameterized: } ] , . …)
Unrecognized token '<c>'                     (e.g. '+')
Unexpected identifier "<word>"               (NaN, Infinity, undefined, tru, True, nul)
Unterminated string
Invalid escape character <c>                 (unquoted)
\u must be followed by 4 hex digits
Invalid number
Invalid digits after decimal point
Exponent symbols should be followed by an optional '+' or '-' and then by at least one number

Note '01' and '00' are Unable to parse JSON string, not a number error — JSC parses the leading 0 as a complete value and then reports trailing content.

Required implementation

The parser in src/builtins.zig (~3574-3900) returns a bare error.Invalid from roughly twenty sites, and jsonParse (3444) maps that one error to one message. Carry the reason:

  • add a reason field to the parser struct (an enum plus the one-char / one-word payload the parameterized forms need), set it at each return error.Invalid site;
  • map reason to JSC's exact string at the jsonParse / jsonRawJson boundary;
  • keep the JSON.rawJSON wording aligned with whatever JSC does there — probe it, do not assume it shares JSON.parse's prefix.

Verification

scratchpad/sweep/probe.sh drives both engines and diffs; the 40-case set used above is sweep/jsonvocab.js. Drive the probe from inside a function — zig-js --eval is script code, so an indirect eval in a case shares global scope with the harness and a case declaring var i will clobber the loop counter and silently truncate the run.

11 days ago

Landed in 8ba5966. A 65-case differential against a real JavaScriptCore backend goes 1/65 → 65/65, plus the cyclic-stringify wording.

JSON.rawJSON shares the grammar and now shares the diagnostics.

Worth recording for the next family

Two things the implementation would have got wrong from reading the code alone:

A leading zero is not a number error. 01, 00, 0x10 and 1e5e5 all produce Unable to parse JSON string — JSC parses the leading value and rejects the remainder as trailing content. Guessing "invalid number" would have been wrong for four cases.

A non-string property name has two different messages depending on position. Expected '}' at the first property ({, {a:1}, {,}, {]), but Property name must be a string literal once a comma has promised another one ({"a":1,}, {"a":1,,}). The object parser has to carry that state.

Also: a raw control byte inside a string is Unterminated string, and an identifier-shaped run is echoed whole — which is how tru and nul get reported.

One deliberate remaining difference

For an astral character JSC prints a single UTF-16 code unit, so JSON.parse('😀') names a lone surrogate (mangling its own output in the process). zig-js names the whole code point. Matching JSC would mean emitting a lone surrogate into a message on purpose.

A bug the probe set missed

Formatting the offending character as a single byte panicked the engine's string tripwire on non-ASCII input (JSON.parse('—') → truncated WTF-8). The all-ASCII probe set did not catch it; test/built-ins/JSON did, on the first corpus run. The fault payloads are whole code points now. Worth remembering that a message-text change can still be a memory-safety change when the text is attacker-supplied.

Sign in to comment on this issue.