Conversation
`JSON.stringify` emitted statically-known record field names as raw bytes: the key label was built by interpolating the decoded name into a JSON fragment and only running the C/LLVM string-literal escaper over it. A name containing `"` produced syntactically invalid JSON, `\` lost its escape, and C0 control bytes were written verbatim, so the result no longer round-tripped through `JSON.parse`. Encode the name with `JSON.stringify` at both label sites in each backend's record writer (all-required and droppable/overflow branches); the existing C/LLVM literal layer then quotes the already-valid JSON text. Index-signature keys were already correct because they go through the runtime `scr_jb_put_json_str`. Adds a differential corpus program covering backslash, double quote, control bytes, optional-field records, index-signature records, nested records, the `space` pretty printer, and a `JSON.parse` round-trip.
Contributor
|
@xia-chao is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
Please review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JSON.stringifybuilt the label by interpolating the decoded name into a JSON fragment and only ran the C/LLVM string-literal escaper over it, so\lost its escape,"closed the string and produced invalid JSON, and C0 control bytes were written verbatim.JSON.stringifyat compile time and keeps the existing literal layer:cStringLiteral/host.cstrthen quotes the already-valid JSON text, so the runtime buffer receives valid JSON. Index-signature keys were already correct (they callscr_jb_put_json_str); only the static-label branches were wrong.tests/corpus/2853-json-stringify-record-key-escapes.ts, a differential program that covers backslash, double quote, control bytes, optional-field records, index-signature records, a nested record, an array element, thespacepretty printer, and aJSON.parseround-trip.Reported in #312 (that report has the debugger screenshots for both the wrong and the correct path).
Before, on the published 0.1.1:
After this change, both backends match Node v24.15.0 byte for byte:
Validation
node packages/cli/dist/main.js run tests/corpus/2853-json-stringify-record-key-escapes.ts(LLVM) and--backend c: stdout byte-identical to Node v24.15.0, exit 0pnpm vitest run tests/harness/differential.test.ts -t "2853-json-stringify-record-key-escapes"passedtests/harness/differential.test.tscorpus run launched locally; results to follow in a commentno-non-null-assertionwarnings in the touched file)pnpm test:sandboxnot run locally (no Vercel Sandbox credentials on this machine)Follow-ups
The same interpolation pattern is used by the
%jformat path and island boundary serialization, which share this record writer, so they inherit the fix. No other raw static JSON label site remains:scr_jb_edge_proponly feeds the circular-structure error message (Node prints that name unescaped too), and the index-overflow path already used the runtime escaper.