toml: parse nested array literals with adjacent brackets - #34004
Conversation
Bun.TOML.parse rejected nested array literals with adjacent brackets like `a = [[1]]` while accepting the whitespace-separated `a = [ [1] ]`. The lexer's allow_double_bracket flag (which merges `[[`/`]]` into array-of-tables header tokens) was forced to true at the top of parse_value_inner and again after every compound value closed, so a `]]` (or at depth >=3 a `[[`) inside an array was lexed as a single header token and the parse failed. parse_assignment and parse_value_inner now save the caller's flag on entry, hold it false while inside an array or inline table, and restore the saved value before lexing the token that follows the value. This keeps `[[header]]` detection working after a top-level value while treating adjacent brackets inside values as two separate tokens.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 1:32 AM PT - Jul 12th, 2026
✅ @robobun, your commit 7f24b5e421347b5e0973a8c34463c3a3efe5928a passed in 🧪 To try this PR locally: bunx bun-pr 34004That installs a local version of the PR into your bun-34004 --bun |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Re #32953: that is a full parser rewrite (+12.5k / -2k). This PR is a targeted ~50-line fix for the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — targeted fix for the allow_double_bracket flag leaking into value position.
What was reviewed:
- Traced the save/restore flow through every
parse_value_innerarm (scalars,t_empty_array,t_open_bracket,t_open_brace) and nestedparse_assignment— flag isfalsefor everynext()inside a compound value and restored to the caller's value exactly before the token following the closing delimiter is lexed. - Confirmed against
lexer.rsthatallow_double_bracketinitstrueandexpect_assignment()lexes the value's first token with flag=false, so[[ina = [[1]]correctly tokenizes as twot_open_bracket. - Checked the removed post-
next()writes (t_empty_array, inline-table loop) were either dead or now subsumed byparse_assignment's own restore. - Tests cover both directions: nested arrays at depth 1–3, inside inline tables, and
[[header]]recognition after every value shape.
Extended reasoning...
Overview
Changes src/parsers/toml.rs (~18 lines net) to fix Bun.TOML.parse rejecting nested array literals with adjacent brackets (a = [[1]]). The lexer's allow_double_bracket flag — which merges [[ / ]] into array-of-tables header tokens — was being forced to true at the top of every parse_value_inner call and again after each compound value, so ]] inside a value merged into t_close_bracket_double and failed the array's t_close_bracket expect. The fix converts the unconditional writes into a save/restore of the caller's flag: compound-value arms set it false before lexing their body and restore the saved value before lexing the token that follows. parse_assignment gets the same treatment so its restore point propagates the correct outer value into parse_value_inner.
Adds 2 tests (18 assertions) to test/js/bun/resolve/toml/toml-parse.test.ts covering nested arrays at depths 1–3, empty nested arrays, arrays inside inline tables and vice versa, and a regression guard that [[header]] on the line after each value shape (scalar, [1], [], {…}, [[1]]) is still lexed as an array-of-tables header.
Security risks
None. This turns a parse rejection of valid TOML into acceptance; no new input classes reach downstream code that weren't already reachable via the whitespace-separated form (a = [ [1] ]). No allocation, path, or FFI changes.
Level of scrutiny
Low–medium. The parser is user-input-facing but the change is a localized state-flag discipline fix with no unsafe code touched. I traced the flag value at every lexer.next() / lexer.expect() call site in the diff against the actual lexer bracket-handling code (lexer.rs:518–540) and each restore point lands correctly: top-level values restore true so a following [[t]] header merges; nested values restore false so adjacent ] stay single tokens. The removed allow_double_bracket = false after the inline-table parse_assignment call is now redundant because parse_assignment itself restores to its captured outer (which is false inside the brace arm).
Other factors
Test coverage is thorough for a fix this size — both the bug and the inverse regression (header merging after values) are asserted, plus the pre-existing whitespace-separated forms. The PR description shows tests fail on system Bun and pass on the debug build. There is an open full-rewrite PR (#32953) that would subsume this, but this fix is independent and the added tests will apply to the rewrite too. No prior human review comments to address.
There was a problem hiding this comment.
LGTM — targeted lexer-flag save/restore fix with thorough test coverage.
What was reviewed:
- Traced
allow_double_bracketthroughparse_assignment→expect_assignment→parse_value_innerfor[[1]],[[[1]]],[[]],{a=[[1]]},[{b=[[1]]}]— flag is held false at everynext()inside a value and restored before the token after the outermost delimiter. - Confirmed the moved
= falsebeforenext()in thet_open_bracket/t_open_bracearms is what stops]]/[[from merging mid-value; scalar arms inherit the caller's flag so[[header]]after a top-level value still merges. - Checked
parse_maybe_trailing_commaand thet_empty_arrayarm still seefalseinside compound values after the removed post-assignment reset —parse_assignmentnow restores to the savedfalse, so state is equivalent.
Extended reasoning...
Overview
Changes src/parsers/toml.rs (~11 net lines across parse_assignment and parse_value_inner) to save/restore the lexer's allow_double_bracket flag instead of unconditionally forcing it to true, and adds two tests (18 assertions) to test/js/bun/resolve/toml/toml-parse.test.ts. The flag controls whether the lexer merges adjacent [[ / ]] into array-of-tables header tokens; the bug was that it was set true at the top of every parse_value_inner call, so nested array literals like [[1]] had their closing ]] mis-lexed as a header-close token.
Security risks
None. Pure boolean flag manipulation in a recursive-descent parser. No new allocations, no unsafe changes, no untrusted-length arithmetic. The change strictly widens the set of valid TOML that parses — previously-rejected inputs now succeed, and the second test block verifies previously-accepted inputs (headers after values, whitespace-separated brackets) still parse identically.
Level of scrutiny
Low-to-medium. This is a self-contained lexer-mode bugfix in a leaf parser (Bun.TOML.parse and TOML config imports). The state machine is small enough to trace by hand: I walked a = [[1]], a = [[[1]]], a = [[]], t = {a = [[1]]}, a = [{b = [[1]]}], and each header-after-value case through the new flag transitions and confirmed the flag is false at every next() call that could see adjacent brackets inside a value, and true (via the saved outer value) at the next() that lexes the token following the outermost value. The removed = false after parse_assignment in the t_open_brace loop is now redundant because parse_assignment itself restores to the saved value (which is false when called from inside a brace/bracket arm). The removed = true in the t_empty_array arm is safe because that arm no longer needs to force the flag — it inherits whatever the caller set (true at top level via the restore in parse_assignment, false when nested).
Other factors
Test coverage is unusually thorough for a fix this size: it exercises depth 2 and 3, empty nested arrays, mixed nesting with inline tables in both directions, the whitespace-separated forms that already worked, and — critically — a second test that guards the restore path for every value shape (scalar, non-empty array, empty array, inline table, nested array) followed by a [[header]]. The PR description shows the tests fail on system Bun and pass on both debug-ASAN and release builds. The bug-hunting system found no issues. The overlapping full-rewrite PR (#32953) is orthogonal — these tests remain valid regardless.
|
CI status: build #72090 has 284 jobs passed, 0 test failures, 0 error annotations. The only red is The diff is green; ready for review/merge. |
1 participant
Reproduction
Any TOML nested array literal with adjacent
[or]brackets was rejected, while the same document with whitespace between brackets parsed. All of these are valid TOML 1.0.Cause
The lexer has an
allow_double_bracketflag that merges adjacent[[/]]into singlet_open_bracket_double/t_close_bracket_doubletokens, used to distinguish[[array.of.tables]]headers from[table]headers.parse_value_innerforced this flag totrueat the top of every call and again after each compound value's closing delimiter, so insidea = [[1]]the]]was merged into a header-close token (and at depth 3+, the[[was merged into a header-open token) and the array parse failed on the unexpected token.Fix
parse_assignmentandparse_value_innernow save the caller's flag value on entry, hold itfalsewhile lexing tokens inside an array or inline table, and restore the saved value before lexing the token that follows the value. At top level the saved value istrue, so a[[header]]on the line after a value is still recognised; at any nested depth the saved value isfalse, so adjacent brackets stay two tokens.Verification
bun bd test test/js/bun/resolve/toml/passes (22 tests).smol-tomlfor 12 nested-array and header-after-value cases; all match.[review] gate passed · iteration 2 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 2
evidence per changed file