Skip to content

json: require end of input after the root value at every parser entry - #45386

Draft
robobun wants to merge 4 commits into
mainfrom
robobun/386a5208/json-reject-trailing-content
Draft

robobun wants to merge 4 commits into
mainfrom
robobun/386a5208/json-reject-trailing-content

Conversation

@robobun

@robobun robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Draft: needs a maintainer's yes or no. Is content after the root value an error at every JSON entry point, with no opt-out? Every release accepts such files, so this is proposed for 1.5.

Problem

  • The JSON parser returns the root value and drops what follows. If the root of a package.json closes early (}, in the middle), bun install deletes bun.lock and exits 0 on every release.
  • With git conflict markers after the root, bun run s runs the first object's script. 1.3.14 and npm refuse that file.
  • Cause: if check_len && !parser.at_trailing_end() in run_stage2 (src/parsers/json.rs:194). Most callers pass false.

Fix

  • Parser::parse_document is the only exit of the parser. It requires end of input after the root value (whitespace and comments pass). The check_len flag is deleted.
  • bun init stops on a package.json that does not parse.
  • Verified: test/js/bun/jsonc/ plus two files. 69 tests fail on main.
  • Self-reviewed: 5 concerns raised, 3 addressed, 2 open (Notes).

Background

Downsides

  • Not ready to merge alone. Some readers hide a parse error: with an extra } after the root, require("dep") loads index.js and bun check skips the check script.
  • Every release accepts }, or a second object after the root. 0 of 7,203 scanned JSON files change.
  • A valid document costs no more (7,217 instructions against 7,223). 20,000 trailing spaces cost +26,576. Binary text: +1,024 bytes.
Notes

The two commits

  • The first one changes stage 1 (json_index.rs), which indexes the structural bytes in 8 KiB windows. It writes the end markers with the last window, so the end-of-input check reads an entry that exists.
  • The second one is the rule. Stage 2 (json_stage2.rs) walks the index.

The decision

The rule is: a document is one value, then only whitespace and comments. A yes covers these cases too:

  • a package.json that ends in }, or }}, as the two fixtures of Support comments & trailing commas in require/import package.json #10531 do (test/cli/run/require-and-import-trailing.test.ts). This PR treats that comma as a stray character.
  • a --define value that starts like JSON and has more content (7 days, 5 * 1000).
  • a dependency whose package.json has such a tail.

Order

Readers that hide a parse error (run on 1.3.14, 1.4.3, main a407eae88a and this PR)

input 1.3.14, 1.4.3, main this PR
a workspace member's package.json with an extra } at the end, bun run build in the member runs the member's script runs the root's script, exit 0
a dependency's package.json with main and an extra }, require("dep") loads main loads index.js
tsconfig.json with paths that ends in },, bun main.ts the alias resolves Cannot find module '@lib/x', no word about tsconfig.json
package.json with a check script and an extra }, bun check runs the script No type errors, exit 0
a bundled dependency's package.json with an extra }, bun pm pack --dry-run (1.4.3 and main) 6 files 4 files: its own dependency is left out

Each of these readers already turns any parse error into a default. This PR sends more files down that path. The plan after a yes: #45113 covers the first row. The others fail with the located error in this PR (has_script in script_or_command.rs, pack_command.rs, and the tsconfig report of #41683). bun pm pkg, bun pm version and bun add print the located error, where they print a bare ParserError now. What the resolver does with a dependency's manifest follows the decision above.

Repro

1.3.14 1.4.3 this PR
package.json whose root closes early with },, then bun install deletes bun.lock, exit 0 deletes bun.lock, exit 0 error at package.json:5:2, bun.lock kept
conflict markers and a second object after the root, bun run s exit 1 runs the first object's script, exit 0 exit 1
the same file, bun pm pkg get name exit 1 prints "first", exit 0 exit 1
the same file, bun pm pkg set foo=bar error, file unchanged rewrites the file without the markers and the second object, exit 0 error, file unchanged
Bun.JSONC.parse('{"a":1} {"b":2}') {a:1} {a:1} throws
bun build of a .json with {"x":1} trailing junk embeds {x:1} embeds {x:1} error (the runtime import already throws)

On 1.4.3, bun pm pkg delete, bun pm version and bun add also rewrite the conflict file without its tail.

Tails refused

  • 94 printable ASCII characters after {"a":1} : 1.3.14 refuses 17, 1.4.3 refuses 1, this PR refuses 94.
  • 12 tails after a package.json object through bun pm pkg get: 1.3.14 refuses 4, 1.4.3 refuses 1, this PR refuses 10. The other 2 are comments, which stay valid.
  • 1.3.14 refused a tail only when its first token did not lex. It never looked for end of input. The rewrite in Rewrite the JSON parser around a SIMD structural index #33032 (1.4.0) stopped looking at that token.

Who sees a new error

  • bun install, bun remove: print the error with file, line and column, exit 1.
  • bun add, bun pm pkg set, bun pm pkg delete, bun pm version: print ParserError, exit 1, leave the file as it is.
  • bun init: prints the error, exits 1, writes nothing (new in this PR). Before, it replaced a package.json that did not parse with the template.
  • require() / import of a package.json or .jsonc file, bun build of .json / .jsonc, tsconfig.json in bun build: a build error with file, line and column.
  • bun.lock: --frozen-lockfile fails. Without it, bun install prints the error, ignores the lockfile and writes a new one.
  • Installed-package verification treats such a package as not installed.
  • Registry manifests and other registry answers: the answer fails to parse. npm reads these with JSON.parse, which refuses a tail too.
  • Bun.JSONC.parse: throws SyntaxError, as its types say (@throws {SyntaxError} If the input is not valid JSONC).
  • The readers in the table above.

--define values (39 probed, 1.4.3 against this PR)

  • Errors now (each gave its first value): 7 days, 1 2, 5 * 1000, {"a":1} junk, "a" + "b", "a" "b", [1],, [1] [2], -1 x. 1.3.14 refused "a" + "b" too.
  • Unchanged: the other 30. true false still gives true: the keywords are read before the parser, and that is a separate question.

Deleted with the flag

  • parse_utf8_impl::<CHECK_LEN>: .npmrc values and bun pm pkg set --json call parse_utf8.
  • ParsedJson::parse_jsonc_document (Add bun lint and bun format #44805): bun lint calls parse_jsonc, which has the same rule now. The unit test that compared the two keeps its rows and loses the three assertions that the plain entry accepts a tail.
  • Parser::unexpected_here. parse_value, at_trailing_end, take_tape and cursor are private to the parser.

Tests that pinned the old behaviour

  • json-test-suite.test.ts: 11 JSONTestSuite n_* rows with content after the value move from "valid JSONC" to their own list that asserts the error.
  • jsonc.test.ts: the window-seam test for {"k":1}@@@@ asserts the error.
  • require-and-import-trailing.test.ts: the two fixtures end in },. Their input stays, and they now assert the error. Two new tests with the same file ending in } assert what Support comments & trailing commas in require/import package.json #10531 added (comments and a trailing comma inside the object).
  • bun-pm-pkg.test.ts: two --json rows get the new message (Unexpected 2 is now Expected end of file but found "2").

Measurements (release builds of main a407eae88a and of this PR at cb1bb706c, Linux x64. The later commits put the --define keyword arm and the call form of one cold helper back as on main. Not measured again.)

Instructions of one whole parse_impl_in call, counted with gdb stepi on the third Bun.JSONC.parse call of each input:

input main this PR
401-byte package.json, SIMD-indexed 7,223 7,217
the same with a trailing newline 7,223 7,217
403-byte JSONC with a comment, scalar-indexed 8,458 8,435
20,406-byte package.json 50,595 50,593
407 bytes with a trailing BOM 7,315 7,468
407-byte object, then 20,000 spaces 24,117 50,693
  • Allocator calls inside that call (mimalloc entry points): 12 and 7 frees for the package.json, 8 and 4 frees for the JSONC file, on both builds.
  • StructuralIndex::fill_to calls per document: 1 on both builds (2 for the 20 KB file on both). The first commit is why: without it the check would ask for an index entry that the last refill had not written yet.
  • Release binary: bun is 106,178,312 bytes on both builds. size reports text 106,007,471 against 106,006,447 (+1,024). The bun_parsers::json* symbols sum to 54,754 bytes against 54,389 (+365, nm -S).
  • check_len lines in json.rs: 0 (main 12). Crate-visible items in json_stage2.rs: 4 (main 8).
  • Not run: the criterion benches. scripts/bench-json-rust.sh does not build on main (it compiles highway_json.cpp as C++17, which now needs C++20).

Designs weighed

  • The flag set at each entry point (json: reject content after the root value (JSONC, package.json, tsconfig, bundler loaders) #35072): the bare bool stays in four signatures.
  • An opt-out for --define (commit d7272bf): that door keeps returning the first value (5 * 1000 is 5).
  • Refuse only what 1.3.14 refused: }}, }, or a second object still pass, and no other tool has that rule.
  • esbuild, V8 and JSC check for end of input inside their one document entry. So do bun's JSON5, TOML and YAML parsers. Node's own package.json reader is not strict: it loads a package.json whose tail ends in }.

Self-review

  1. A change of what every JSON reader accepts needs a maintainer's yes. Addressed: the question is at the top.
  2. The NODE_ENV quoting copied bundler: JSON-escape the NODE_ENV auto-define value #41476, and the --define keyword rule is its own question. Addressed: both are out (4535dd2).
  3. The Problem named the smaller case. Addressed: it leads with the deleted bun.lock.
  4. Five readers hide the new parse error. Open, see the table.
  5. The PR conflicts with install: fix regressions and older bugs found after the package manager consolidation #45307 and depends on run: report a package.json that does not parse instead of using a parent's scripts #45113. Open, see Order.

Not in this PR

Tests

  • Changed: test/js/bun/jsonc/jsonc.test.ts, test/js/bun/jsonc/json-test-suite.test.ts, test/cli/run/require-and-import-trailing.test.ts, test/cli/install/bun-pm-pkg.test.ts.
  • cargo test -p bun_parsers --lib --release: 41 passed, 0 failed. CI type-checks these tests but does not run them.
  • Also ran test/regression/issue/32686.test.ts, test/js/bun/resolve/jsonc.test.ts, test/bundler/bundler_loader.test.ts, test/js/bun/ini/ini.test.ts, test/bundler/bundler_env.test.ts.
  • The --define tests are in jsonc.test.ts because they test the same parser.
  • Files scanned for the Downsides count: 7,203 .json and .jsonc files (4,140 package.json) in node_modules, the install cache, Node's test fixtures and this repo. 1.4.3 and this PR accept the same 7,187.

Credit: this starts from #35072 and from commit d7272bf (robobun/b262bd0b/json-reject-trailing-content), whose test matrix it keeps.


no test proof · iteration 2 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-pm-pkg.test.ts

…byte

The SIMD producer pushed the two end sentinels only on the refill after the
one that consumed the last byte. A read of the index entry behind the root
value then made a second cold fill_to call and a window move. The scalar
producer already pushes the sentinels when it reaches the end. Capacity for
both is reserved in with_producer.
Parser::parse_document is the only way to get a root value or the tape out of
the stage-2 parser. It parses the root value and then requires end of input.
Whitespace, a BOM and comments can follow the value. Other content is reported
as 'Expected end of file but found "<token>"'.

The check_len parameter chain, parse_utf8_impl, ParsedJson::parse_jsonc_document
and Parser::unexpected_here are deleted. Their callers (.npmrc values,
pm pkg set --json, bun lint) call the plain entry points, which now have the
same rule.

--define values go through the same parser. A value that starts like JSON must
be JSON to its end. true, false, null and undefined are keywords only as the
whole value: with other content behind the word the value is an auto-quoted
string. NODE_ENV is written as an escaped string literal, so a quote or a
backslash in it no longer ends the define value early.

bun init stops with the parse error when the package.json in the directory does
not parse. It replaced the file with the template before.
@robobun

robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 10:54 PM PT - Oct 15th, 2026

❌ @robobun, your commit eccfa42 has 1 failures in Build #126364 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 45386

That installs a local version of the PR into your bun-45386 executable, so you can run:

bun-45386 --bun

@robobun

robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on the released 1.4.3 (c6da4a4d3) and on main a407eae88a, Linux x64.

# 1. The root object closes early, so "dependencies" is content after the root.
#    1.3.14, 1.4.3 and main: "No packages! Deleted empty lockfile", exit 0.
mkdir dep && echo '{ "name": "dep", "version": "1.0.0" }' > dep/package.json
echo '{ "name": "app", "dependencies": { "dep": "file:./dep" } }' > package.json && bun install
printf '{\n  "name": "app",\n  "scripts": { "s": "echo hi" }\n},\n  "dependencies": { "dep": "file:./dep" }\n}\n' > package.json
bun install

# 2. Git conflict markers after the root object. 1.3.14 refuses this file.
printf '%s\n' '{"name":"first","scripts":{"s":"echo FIRST_BLOCK_RAN"}}' '<<<<<<< HEAD' '{"name":"second","scripts":{"s":"echo SECOND"}}' '>>>>>>> feature' > package.json
bun run s               # 1.4.3: prints FIRST_BLOCK_RAN, exit 0
bun pm pkg set foo=bar  # 1.4.3: rewrites the file without the markers and the second object

A --define keyword followed by other content (true false) gives the keyword
again. Whether that value is an error or a string is a separate question.
A --define value that starts like JSON still has to be JSON to its end,
because it goes through the parser.

NODE_ENV is quoted as on main. #41476 escapes that value.
Comment thread src/ini/lib.rs Outdated
Comment thread src/parsers/json_stage2.rs Outdated
The comment above the parse call in src/ini/lib.rs described two Expr types
that are one type now. The two-line comment in its error arm and the doc
comment of Parser::parse_document are one line each.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants