Skip to content

Rewrite the TOML parser for v1.1.0 conformance - #32953

Merged
dylan-conway merged 44 commits into
mainfrom
claude/toml-test-suite
Jul 16, 2026
Merged

Rewrite the TOML parser for v1.1.0 conformance#32953
dylan-conway merged 44 commits into
mainfrom
claude/toml-test-suite

Conversation

@dylan-conway

@dylan-conway dylan-conway commented Jun 28, 2026

Copy link
Copy Markdown
Member

What this adds

A spec-exact TOML v1.1.0 parser (src/parsers/toml.rs), replacing the previous JS-lexer-derived implementation, plus the official toml-lang/toml-test conformance suite as a generated bun:test file, following the same pattern as the YAML and JSON5 conformance suites.

Conformance: 100% of the official suite, before → after

Measured against the toml-test v1.1.0 manifest (commit 4d77658d): 217 valid + 481 invalid + 1 out-of-range-integer + 9 invalid-encoding cases — every case in the manifest, zero exclusions.

before after
total 160/699 (23%)¹ 708/708 (100%)
valid documents parsed correctly 148/217 217/217
invalid documents rejected with the asserted SyntaxError 12/481 481/481
invalid documents wrongly accepted 144 0
invalid-encoding documents (raw byte input) untestable¹ 9/9
date/time literals all rejected as syntax errors parse as strings of their source text

¹ The 9 invalid-encoding cases contain ill-formed UTF-8 that a JS string cannot carry; they became testable when TOML.parse gained binary input in this PR.

Parser

  • All four TOML date/time types (offset/local date-time, local date, local time), returned as strings of the source text
  • Correct inf/nan, underscore and leading-zero validation, 0x/0o/0b integers
  • Integers outside Number.MAX_SAFE_INTEGER throw instead of silently losing precision (TOML requires lossless handling or an error)
  • Table / array-of-tables / dotted-key / inline-table definition-state rules enforced; duplicate keys rejected, including non-ASCII keys (the old byte-view comparison of UTF-16 keys missed duplicates and falsely rejected distinct keys)
  • Control characters, bare carriage returns, and ill-formed UTF-8 rejected (simdutf-validated); leading BOM handled
  • Multi-line string delimiter/trimming rules, CRLF→LF normalization, exact escape validation including TOML 1.1's \xHH and \e
  • TOML 1.1 additions: optional seconds in times, multi-line inline tables with trailing commas
  • Dotted keys are parsed iteratively (the old parser capped them at 512 segments)

Bun.TOML.parse

  • Throws SyntaxError with precise messages (previously a BuildMessage), matching JSON.parse, Bun.YAML.parse, and Bun.JSON5.parse
  • Converts the AST directly to JS values (previously printed to JSON and re-parsed)
  • Accepts Blob/TypedArray/DataView/ArrayBuffer input like the YAML and JSON5 siblings
  • Parse errors carry the redaction flag so secrets in malformed config files stay out of logs

Bun.TOML.stringify (new)

Serializes a JavaScript object to a TOML document — this API did not exist before. Same surface as the YAML/JSON5 siblings: (value, replacer, space), where replacer throws and space is accepted but ignored (TOML output is line-oriented).

  • Idiomatic layout: scalar keyvals first, then [table] and [[array-of-tables]] sections; mixed arrays use inline tables; keys are bare when possible, quoted otherwise
  • Date values become TOML offset date-times
  • null, BigInt, circular structures, and non-object top-level values throw (TOML cannot represent them); undefined/function/symbol properties are skipped
  • Integral doubles beyond ±(2^53 − 1) are emitted as floats so documents round-trip through any TOML reader; unpaired surrogates get the same USVString replacement as TOML.parse string input

Tests

  • test/js/bun/toml/toml-test-suite.test.ts — generated official suite (708 tests); every rejection test asserts the exact full error message
  • test/js/bun/toml/generate_toml_test_suite.ts — pins the upstream commit, decodes the suite's tagged-JSON expectations, inlines every case; --check mode fails if the committed suite is stale
  • test/js/bun/toml/toml.test.ts — hand-written coverage beyond the official suite: input types (Buffer/TypedArray subviews/DataView/ArrayBuffer/SharedArrayBuffer/Blob), JS value mapping (__proto__ safety, key ordering, no Unicode normalization), safe-integer boundaries, source-text date/time preservation, multi-line string edge cases, recursion-limit and GC robustness, the SyntaxError message contract, and TOML.stringify (exact layout output, escaping, round-trips, every error contract, and a GC stress test)

Performance

1.15×–1.49× faster than the previous parser at every document size, growing with size. Release builds from this pipeline (darwin-aarch64), interleaved A/B, best of 3 rounds (each sample = median of 15 timed batches), real-world inputs both parsers accept:

document size old new change
uv.lock (apache/airflow) 2.9 MB 246 MB/s (12.0 ms) 367 MB/s (8.0 ms) 1.49×
uv.lock (langgenius/dify) 662 KB 237 MB/s 338 MB/s 1.43×
poetry.lock (python-poetry/poetry) 201 KB 249 MB/s 341 MB/s 1.37×
Cargo.lock (zed-industries/zed) 495 KB 194 MB/s 262 MB/s 1.35×
Cargo.lock (this repo) 71 KB 199 MB/s 259 MB/s 1.30×
pyproject.toml (python-poetry/poetry) 5.7 KB 168 MB/s 221 MB/s 1.31×
poetry.lock (truncated at a package boundary) 2.4 KB 16.2 µs/op 13.5 µs/op 1.20×
bunfig.toml 126 B 3.29 µs/op 2.85 µs/op 1.16×

Basic strings borrow the source bytes when no escape requires decoding (the same approach the YAML parser uses for plain scalars), so string-dominated documents — lockfiles in particular — see the largest gains.

Behavioral note: stricter parsing of invalid config files

The old parser silently accepted some invalid TOML, so a small set of existing bunfig.toml files (about 1% in a sample of 99 real-world bunfigs from GitHub) will go from silently tolerated to a startup SyntaxError that states the fix. The one pattern observed in the wild is unquoted string values — easy to carry over from .npmrc syntax — which now produce a purpose-built message:

TOML Parse error: Strings must be quoted: "isolated"

The other formerly-tolerated patterns (backslash escapes in basic strings being silently dropped, missing newlines between key/value pairs, bare keys containing :) were silent data corruption or non-portable syntax that every other TOML implementation already rejects; none appeared in the sample.

Deletions

The old TOML lexer (src/parsers/toml/lexer.rs), and the now-caller-less E::Object::set_rope, get_or_put_array, and set. The bake error reporter's JS-escape decoder (which borrowed the old TOML lexer) is extracted standalone with its exact semantics.

Closes #22426
Closes #28680
Closes #28681
Closes #28687

Also addresses the TOML half of #32025 (\u{…} is now correctly rejected; the JS-lexer half remains).


Merge with main (38cfba6)

One conflict, in src/ast/e.rs. Main's hardening round (#33072) added flags: own_key_property_flags(&key) to the property-construction sites, which marks a __proto__ key as computed so it becomes an own property rather than setting the prototype. This branch deletes set, set_rope, and get_or_put_array, replacing set with append_property.

Git merged the new flags into append_property on its own; the conflict was only the bodies of the two functions this branch removes. Kept them removed, after checking that neither has a caller anywhere in the tree and that every live construction site (put, get_or_put_object, append_property) carries the new flags, so the hardening is preserved on every path that still exists.

Verified on the merge result: the TOML suites pass (810 tests, including the full conformance suite), and the INI and bun init tests pass (73 tests) since they are the remaining get_or_put_object callers. test/bake/dev/production.test.ts also passes again now that #33204 pinned the React build the bake harness installs.

Merge with main (dbac342)

Conflicts with #33722, which fixed \u{...} overflow and unterminated-brace handling in the old TOML lexer's copy of the JS escape loop.

  • src/parsers/toml/lexer.rs (modify/delete): kept deleted. The new parser rejects \u{...} at the opening brace (\u{…} is JavaScript syntax, not TOML), so neither the overflow nor the unterminated-brace path is reachable.
  • test/js/bun/resolve/toml/toml-parse.test.ts: kept this branch's content and folded the Panic in debug on invalid unicode escape sequence #30825 crash-regression inputs into the existing rejects JS-style \u{XX} escapes test. Main's still accepts in-range \u{...} test asserted \u{41} decodes to "A", which contradicts spec-compliant TOML and is already asserted to throw by this file.
  • src/runtime/bake/DevServer/js_escape.rs: ported the one-line unterminated-brace fix, since that decoder was extracted from the code lexer: fix \u{...} escape overflow and unterminated-brace accept #33722 patched and is documented as preserving its exact semantics. Overflow was already prevented by the existing 0x10FFFF clamp.

Merge with main (aba8757)

Conflicts with #33925 (repo-wide snake_case directory cleanup), which renamed src/runtime/bake/DevServer/ to dev_server/ and replaced the #[path = "../DevServer/..."] module-path attributes with plain mod declarations.

  • src/runtime/bake/dev_server/mod.rs: adopted main's simplified submodule block and added the js_escape declaration.
  • src/runtime/bake/DevServer/js_escape.rs: moved to dev_server/js_escape.rs following the rename. Git tracked the rename of this branch's ErrorReportRequest.rs changes into error_report_request.rs, and its super::js_escape::decode_js_escape_sequences call is intact.

Merge with main (ace42f9)

Conflicts with #33909, which replaced bun_core::Error with per-crate thiserror enums across the codebase.

  • src/parsers/toml.rs: main changed the old file's error handling; this branch replaces the whole file. Kept the new parser and adapted TOML::parse to return crate::Result<Expr>, mapping the internal PErr to parsers::Error::{SyntaxError, Alloc, StackOverflow}.
  • src/parsers/toml/lexer.rs (modify/delete): kept deleted.
  • src/parsers/error.rs, src/runtime/error.rs: removed the From<toml::lexer::Error> impls that referenced the deleted file.
  • src/runtime/bake/dev_server/js_escape.rs: adapted to return Result<(), crate::Error> with crate::Error::SyntaxError, since bun_core::err! is gone.
  • src/runtime/api/TOMLObject.rs: matched on bun_parsers::Error::Alloc(_) in place of the removed bun_core::err!("OutOfMemory").
  • src/ast/lexer_log.rs: doc comment combining both sides.

Translates the toml-lang/toml-test suite (TOML v1.0.0 manifest, pinned
commit 4d77658d) into a generated bun:test file, following the pattern
of the YAML and JSON5 conformance suites: a checked-in generator
inlines each case, with expectations decoded from the suite's own
tagged-JSON files. The generator's --check mode regenerates to a temp
file and fails if the committed suite is stale.

Asserted type contract:
- integers: number within Number.MAX_SAFE_INTEGER, BigInt beyond
- date/time values: strings (source text), compared after normalizing
  the separator to "T", uppercasing "Z", and trimming trailing zeros
  from fractional seconds
- invalid documents must throw SyntaxError (matching JSON.parse,
  Bun.YAML, and Bun.JSON5)

209 valid + 488 invalid cases; 9 non-UTF-8 inputs are excluded and
listed in the header (a JS string input cannot carry invalid bytes).

Baseline on a current debug build: 159/697 pass. The suite documents
the parser's gaps (date/time literals rejected, inf/nan mis-parsed,
int64 precision loss, BOM handling, array-of-tables lexing in value
position, missing rejection of invalid documents, error class);
parser fixes follow on this branch until the suite is green.
@github-actions

Copy link
Copy Markdown
Contributor

Found 5 issues this PR may fix:

  1. Bun.TOML.parse fails on DateTime values #28687 - PR's conformance suite explicitly documents "all date/time literals are rejected as syntax errors", which is exactly this bug
  2. TOML parser converts tab escapes to formfeeds? #28681 - Conformance suite covers correct string escape decoding, which would catch the tab-to-formfeed conversion bug
  3. TOML multi-line strings not properly trimmed #28680 - Conformance suite includes tests for multi-line basic string line-ending backslash handling
  4. bug: bun doesn't support toml v1.0.0 #22426 - Conformance suite tests compliance with TOML v1.0.0 spec, directly addressing this issue about lack of v1.0.0 support
  5. Unterminated \u{ escape in string literals is accepted instead of raising a syntax error #32025 - Conformance suite tests invalid string escapes including unterminated \u{ sequences in TOML

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #28687
Fixes #28681
Fixes #28680
Fixes #22426
Fixes #32025

🤖 Generated with Claude Code

@robobun

robobun commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator
Updated 3:22 AM PT - Jul 16th, 2026

@robobun, your commit 970d41b is building: #73882

- Integers outside Number.MAX_SAFE_INTEGER are asserted to throw instead
  of expecting BigInt: TOML requires lossless handling or an error, and
  mixed number/BigInt output is not acceptable API. The affected
  upstream-valid case moves to a documented out-of-range block citing
  toml-lang/toml-test#154 (64-bit range is a "should").
- Rejection tests assert the exact full SyntaxError message, captured
  from the in-tree parser at generation time; cases the parser does not
  yet reject with SyntaxError assert only the class until regenerated.
  toThrow(string) is substring matching and toThrow(Error) ignores the
  class, so the tests use an explicit try/catch with toBeInstanceOf
  plus message equality.
Switch the generated suite from the v1.0.0 manifest to v1.1.0
(released 2025-12-24). TOML 1.1 is a strict superset of 1.0: optional
seconds in times, multi-line inline tables with trailing commas, and
\xHH/\e string escapes. The datetime comparator gains one rule for
omitted seconds (07:32 is the same value as 07:32:00, which is how the
upstream expectations spell it).
@dylan-conway dylan-conway changed the title Add official toml-lang/toml-test conformance suite for TOML Add official toml-lang/toml-test conformance suite (TOML v1.1.0) Jun 28, 2026
Replaces the JS-lexer-derived TOML parser with a spec-exact byte-level
recursive-descent parser. The official toml-test conformance suite now
passes 699/699 (from 160/699): all four date/time types parse (as
strings of their source text), inf/nan and underscore/leading-zero
validation are correct, control characters and bare carriage returns
are rejected, a leading BOM is handled, table and array-of-tables
definition-state rules are enforced, multi-line strings trim and
normalize CRLF correctly, and escape sequences are validated exactly
(including TOML 1.1's \xHH and \e).

Behavior changes beyond conformance:
- integers outside Number.MAX_SAFE_INTEGER throw instead of silently
  losing precision (TOML requires lossless handling or an error)
- Bun.TOML.parse throws SyntaxError (previously BuildMessage), converts
  the AST directly to JS values instead of printing JSON and re-parsing,
  and accepts Blob/Buffer input like the YAML and JSON5 siblings
- parse errors carry the redact flag so bunfig secrets stay out of logs
- duplicate non-ASCII keys are detected correctly (the old byte-view
  comparison of UTF-16 keys missed duplicates and falsely rejected
  distinct keys)

The shared Expr-to-JS conversion moves from JSON5Object to the api
module and is reused by TOML. The bake error reporter's JS-escape
decoder, which borrowed the old TOML lexer, is extracted standalone
with its exact semantics. Dead code removed in the same change:
the old toml lexer, E::Object::set_rope/get_or_put_array/set.

Fixes #22426
Fixes #28680
Fixes #28681
Fixes #28687
@dylan-conway dylan-conway changed the title Add official toml-lang/toml-test conformance suite (TOML v1.1.0) Rewrite the TOML parser for v1.1.0 conformance Jun 28, 2026
@dylan-conway
dylan-conway marked this pull request as ready for review June 28, 2026 01:49
@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The TOML parser is rewritten as a byte-level recursive-descent parser with TOML 1.1 string, number, date/time, and inline-table handling. Shared Expr-to-JS conversion moves into api.rs, DevServer gets a JS escape decoder, and TOML tests, docs, and types are updated.

Changes

TOML parsing and runtime conversion

Layer / File(s) Summary
Object property append API and comment updates
src/ast/e.rs, src/bun_core/string/immutable.rs
Object removes set, set_rope, and get_or_put_array, and adds append_property that appends unconditionally. Nearby comments and shared lexer-stepper docs remove TOML references.
TOML parser entry point and structure
src/parsers/Cargo.toml, src/parsers/toml.rs
TOML becomes a unit struct with a static parse entry point. Parser state, error helpers, cursor utilities, trivia skipping, UTF-8 validation, document parsing, table header handling, and key-path assignment are added.
TOML values, numbers, and strings
src/parsers/toml.rs
Value dispatch, keyword validation, arrays, inline tables, dotted-key container creation, number and datetime parsing, basic-string escapes, and literal-string handling are implemented in the new parser.
Shared Expr-to-JS conversion and host wiring
src/runtime/api.rs, src/runtime/api/JSON5Object.rs, src/runtime/api/TOMLObject.rs
estring_to_js, expr_to_js, and expr_to_js_with_check are added in api.rs. JSON5Object calls the shared helper, and TOMLObject switches to direct conversion and structured TOML parse error reporting.
DevServer JS escape decoding extraction
src/runtime/bake/DevServer/js_escape.rs, src/runtime/bake/dev_server/mod.rs, src/runtime/bake/DevServer/ErrorReportRequest.rs
A new js_escape module adds UTF-8 JavaScript escape decoding. ErrorReportRequest replaces the TOML-lexer-based path with the new helper, and the dev-server module exposes it.
TOML tests, generator, docs, and type surface
test/js/bun/toml/generate_toml_test_suite.ts, test/js/bun/toml/toml.test.ts, test/js/bun/resolve/toml/toml-parse.test.ts, docs/runtime/toml.mdx, packages/bun-types/bun.d.ts
The TOML test suite adds rejection coverage for escape parsing and non-ASCII keys. A new generator builds toml-test cases from the upstream manifest, and the TOML docs and type declarations expand to the new input and behavior surface.

Possibly related PRs

  • oven-sh/bun#31255: Also changes src/runtime/api/TOMLObject.rs error-handling behavior for TOML parse failures.

Suggested reviewers

  • alii
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The parser rewrite and new tests address date/time parsing, multiline string trimming, escape handling, and datetime support for the linked bugs.
Out of Scope Changes check ✅ Passed I don't see materially unrelated changes; the docs, typings, tests, and helper extraction all support the TOML parser rewrite.
Title check ✅ Passed The title clearly summarizes the main change: rewriting the TOML parser for TOML v1.1.0 conformance.
Description check ✅ Passed The description is detailed and includes the PR purpose plus verification results, even though it doesn't use the exact template headings.

Comment @coderabbitai help to get the list of available commands.

The 9 toml-test invalid-encoding cases cannot be JS strings, so the
generated suite now passes them to TOML.parse as bytes (which the
parser must reject: a TOML document must be valid UTF-8 as a whole),
bringing the suite to 708 tests with zero exclusions.

Types: TOML.parse accepts TypedArray/DataView/ArrayBuffer input and
documents the SyntaxError contract. Docs: the supported-features
section reflects TOML v1.1.0 (date/times as strings, lossless-integer
errors, multi-line inline tables, \xHH and \e escapes).
@dylan-conway
dylan-conway requested a review from alii as a code owner June 28, 2026 01:55
@mintlify

mintlify Bot commented Jun 28, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
bun 🟢 Ready View Preview Jun 28, 2026, 1:56 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/bake/DevServer/js_escape.rs`:
- Around line 1-9: Condense the added explanatory comments in
js_escape::decode_escape_sequences so each comment block stays within the repo’s
3-line limit. Keep only the essential summary near the module docs and the
decode_escape_sequences behavior notes, and move the extra
implementation/history detail out of the source comment or into PR/docs. Also
trim the later comment block referenced in the review so it follows the same
limit.
- Around line 35-37: The escape decoder currently treats an unfinished trailing
backslash or an unterminated `\u{...` sequence as success in the `js_escape`
parsing loop. Update the escape-handling logic to detect end-of-input while
inside an escape and return an error instead of `Ok(())` or emitting a partial
codepoint, using the relevant decoder/iterator flow in `js_escape.rs` (including
the repeated logic at the later occurrence) to ensure malformed escapes are
rejected consistently.

In `@test/js/bun/toml/generate_toml_test_suite.ts`:
- Around line 54-57: Keep the invalid-UTF-8 corpus in
generate_toml_test_suite.ts and stop filtering out TextDecoder failures via the
excluded list. Update the generation logic around utf8Strict and the suite
emitters so raw bytes are preserved and byte-input cases are produced as
Uint8Array/Blob-backed tests for the byte path. Remove the “untestable”
exclusion/comment and ensure the BOM/invalid-UTF-8 inputs remain part of the
generated TOML suite.
- Around line 11-14: The TOML test generator currently clones a moving upstream
HEAD and also skips invalid UTF-8 byte-input cases. Update generateTomlTestSuite
(and its git-cloning helper path) to clone toml-lang/toml-test with argv-based
git calls, then checkout the fixed revision the suite is intended to track so
--check stays stable; also keep the byte-input cases by preserving the
Blob/TypedArray-style tests instead of dropping entries that fail UTF-8
decoding.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 733551a7-ed0c-426d-b08c-8a0cad44a6af

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9331d and e1ae2e5.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • src/ast/e.rs
  • src/bun_core/string/immutable.rs
  • src/parsers/Cargo.toml
  • src/parsers/toml.rs
  • src/parsers/toml/lexer.rs
  • src/runtime/api.rs
  • src/runtime/api/JSON5Object.rs
  • src/runtime/api/TOMLObject.rs
  • src/runtime/bake/DevServer/ErrorReportRequest.rs
  • src/runtime/bake/DevServer/js_escape.rs
  • src/runtime/bake/dev_server/mod.rs
  • test/js/bun/resolve/toml/toml-parse.test.ts
  • test/js/bun/toml/generate_toml_test_suite.ts
  • test/js/bun/toml/toml-test-suite.test.ts
💤 Files with no reviewable changes (1)
  • src/parsers/toml/lexer.rs

Comment thread src/runtime/bake/DevServer/js_escape.rs Outdated
Comment thread src/runtime/bake/dev_server/js_escape.rs
Comment thread test/js/bun/toml/generate_toml_test_suite.ts
Comment thread test/js/bun/toml/generate_toml_test_suite.ts Outdated
toml.test.ts covers what the official conformance suite cannot: the
JS-facing surface (string/Buffer/TypedArray/DataView/ArrayBuffer/
SharedArrayBuffer/Blob input, subarray offsets, toString coercion),
JS value mapping (__proto__ and constructor keys as own properties,
array-index key ordering, no Unicode normalization of keys, -0.0,
safe-integer boundaries), source-text preservation for all date/time
spellings, multi-line string delimiter edge cases, robustness (deep
nesting throws RangeError instead of crashing, 1 MB strings, GC
stress), and the SyntaxError message contract.

Errors at end of input previously trailed off ("Expected '=' after a
key but found"); they now name the end of file.
Parsing dotted paths is iterative (the old parser capped them at 512
segments; the rewrite has no cap), so the recursion limit is reached in
the Expr-to-JS conversion instead — pin that both sides fail with a
clean RangeError rather than a crash, and that a 1000-segment dotted
key now parses.
Release frames are much smaller than debug/ASAN frames, so overflow
depths must be far past the limit at the smallest frame size: nested
arrays/inline tables use 2M (the recursive parse trips the stack check
early, so the depth is never paid in full). Dotted-key parsing is
iterative and pays the full depth, so it uses 250k — about a second in
debug builds while still overflowing the JS-conversion recursion on
release frames.
Comment thread src/parsers/toml.rs Outdated
Comment thread src/parsers/toml.rs Outdated
Comment thread src/parsers/toml.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/runtime/toml.mdx`:
- Around line 48-56: The TOML docs overstate conformance by saying Bun passes
the complete official toml-test suite, but the generated suite in
generate_toml_test_suite.ts rewrites out-of-range integer fixtures into
rejection tests because Bun throws beyond Number.MAX_SAFE_INTEGER. Update the
claim in toml.mdx to narrow it to Bun’s adapted TOML v1.1 coverage or explicitly
mention the JavaScript-number exception, keeping the language accurate for the
integer behavior.

In `@packages/bun-types/bun.d.ts`:
- Around line 794-798: The TOML parse overload in bun.d.ts narrows DataView too
much by using DataView<ArrayBuffer>, which excludes valid shared-backed views
despite the input type already allowing ArrayBufferLike. Update the parse
signature to use DataView<ArrayBufferLike> so the overload matches all supported
DataView byte sources and type-checks consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 93d0d7d6-9a35-4811-811c-bc575bf69a87

📥 Commits

Reviewing files that changed from the base of the PR and between e1ae2e5 and 2355ad2.

📒 Files selected for processing (6)
  • docs/runtime/toml.mdx
  • packages/bun-types/bun.d.ts
  • src/parsers/toml.rs
  • test/js/bun/toml/generate_toml_test_suite.ts
  • test/js/bun/toml/toml-test-suite.test.ts
  • test/js/bun/toml/toml.test.ts

Comment thread docs/runtime/toml.mdx Outdated
Comment thread packages/bun-types/bun.d.ts Outdated
The old parser silently accepted bare words as string values
(`linker = isolated`), which is the most common spec violation in
real-world bunfig.toml files; rejecting it with "Expected a number but
found 'i'" pointed users in the wrong direction. Bare words in value
position now produce: Strings must be quoted: "isolated".
Comment thread test/js/bun/toml/generate_toml_test_suite.ts Outdated
Comment thread packages/bun-types/bun.d.ts Outdated
- Newlines and comments are no longer accepted between '=' and the
  value inside inline tables (keyval-sep is `ws %x3D ws`); the inline
  loop now reuses parse_keyval, which already enforced this at the
  top level
- Decimal integers accumulate as an unsigned magnitude so i64::MIN
  gets the lossless-representation message instead of the wrong
  "outside the 64-bit signed range" diagnostic
- vec_into_slice duplicated ArenaVecExt::into_bump_slice; use the
  trait and drop the redundant unsafe helper
- The suite generator checks out its pinned upstream commit when
  cloning, so --check stays stable as toml-test advances
- TOML.parse types add Blob and widen DataView to ArrayBufferLike
  (JSONL's identical DataView narrowing fixed in the same pass);
  docs name the one deliberate suite deviation (out-of-range
  integers throw)
A full production-by-production audit of the parser against the
official TOML 1.1.0 ABNF found exactly one behavior worth pinning:
JS string input converts to UTF-8 with USVString replacement before
parsing, so unpaired surrogates become U+FFFD (matching TextEncoder
and the YAML/JSON5 siblings), while the same content as bytes is
ill-formed UTF-8 and rejects.
Comment thread src/parsers/toml.rs
Comment thread src/parsers/toml.rs Outdated
Comment thread src/ast/e.rs Outdated
parse_basic_string copied every byte into an arena buffer even when
the decoded content is identical to the source. Borrow the source
slice like parse_literal_string already does, copying only when an
escape, CRLF normalization, quote-run content, or line-ending
backslash makes the decoded bytes diverge. Most real-world strings
(package names, versions, hashes in lockfiles) take the borrow path.
Comment thread test/js/bun/toml/toml.test.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
test/js/bun/toml/generate_toml_test_suite.ts (1)

43-46: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Use argv-based git invocations here.

tmp is interpolated into a shell command, so a TMPDIR containing spaces or shell metacharacters can break this generator or execute unintended shell syntax on the developer machine. Call git with an argument array instead of execSync("...").

Proposed fix
-  execSync(`git clone https://github.com/toml-lang/toml-test.git ${tmp}`, { stdio: "inherit" });
-  execSync(`git -c advice.detachedHead=false checkout ${PINNED_COMMIT}`, { cwd: tmp, stdio: "inherit" });
+  execFileSync("git", ["clone", "https://github.com/toml-lang/toml-test.git", tmp], {
+    stdio: "inherit",
+  });
+  execFileSync("git", ["-c", "advice.detachedHead=false", "checkout", PINNED_COMMIT], {
+    cwd: tmp,
+    stdio: "inherit",
+  });
import { execFileSync } from "node:child_process";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/js/bun/toml/generate_toml_test_suite.ts` around lines 43 - 46, The toml
test generator is building shell command strings in the clone/checkout flow,
which makes the `tmp` path unsafe when it contains spaces or shell
metacharacters. Update the `generate_toml_test_suite.ts` logic to use argv-based
child process calls instead of string-based `execSync` for the `git clone` and
`git checkout` steps, and switch to the appropriate API such as `execFileSync`
so `tmp` is passed as a separate argument. Keep the existing `mkdtempSync`,
`PINNED_COMMIT`, and `git` workflow, but route the commands through argument
arrays to avoid shell interpolation.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@test/js/bun/toml/generate_toml_test_suite.ts`:
- Around line 43-46: The toml test generator is building shell command strings
in the clone/checkout flow, which makes the `tmp` path unsafe when it contains
spaces or shell metacharacters. Update the `generate_toml_test_suite.ts` logic
to use argv-based child process calls instead of string-based `execSync` for the
`git clone` and `git checkout` steps, and switch to the appropriate API such as
`execFileSync` so `tmp` is passed as a separate argument. Keep the existing
`mkdtempSync`, `PINNED_COMMIT`, and `git` workflow, but route the commands
through argument arrays to avoid shell interpolation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2ca83b12-abc7-412f-9bc7-36979d60ff67

📥 Commits

Reviewing files that changed from the base of the PR and between 2355ad2 and 71733f1.

📒 Files selected for processing (7)
  • docs/runtime/toml.mdx
  • packages/bun-types/bun.d.ts
  • src/parsers/toml.rs
  • src/runtime/bake/DevServer/js_escape.rs
  • test/js/bun/toml/generate_toml_test_suite.ts
  • test/js/bun/toml/toml-test-suite.test.ts
  • test/js/bun/toml/toml.test.ts

dylan-conway and others added 3 commits July 10, 2026 16:00
#33874 removed the debug_assert block that consumed `result`, leaving a
clippy::let_and_return violation that every open PR now trips. The code
is identical on main; this is merge hygiene.
…regex

The literal `"xxx"` padding matched the gate's TODO/FIXME/XXX scan as a
false positive. The test only cares that the padding is 3 bytes.
@robobun

robobun commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Post-sync CI triage. Of the three fast-gate failures on 3b9c945, two are inherited from main and one is a false positive; two small commits pushed to clear the ones that can be cleared here.

  • cargo clippy (let_and_return at src/paths/resolve_path.rs:1146): Build Windows openat object names from NT device paths #33874 on main removed the debug_assert block that consumed result, leaving a dead binding. The code is byte-identical on main; this PR does not touch the file, but the clippy workflow only runs on PRs so every open PR trips it. Fixed in 4230961 (inline the slice expression). cargo clippy -p bun_paths -- -D warnings passes.
  • robobun/evidence (TODO/FIXME in added lines): false positive on a string literal, Buffer.from("xxx" + doc + "yyy") in toml.test.ts:32, padding for the Uint8Array subarray-offset test. Renamed to "<<<" / ">>>" in 71edf45; the test only cares that the padding is 3 bytes. 83/83 on the file after.
  • TypeScript types (11/12 fail, ENOENT lib.es2020.d.ts): the identical failure is on main HEAD b330399. The bun-types test harness cannot find TypeScript's bundled lib files; it is unrelated to this PR's bun.d.ts changes (which type-checked 12/12 on the previous head). Left alone here; it will go green when main fixes it.

Review threads: 31/31 resolved, none opened since the last round. Buildkite #71674 is still mid-flight.

Conflicts with #33909 (per-crate thiserror enums replacing bun_core::Error):

- src/parsers/toml.rs: kept this branch's new parser; adapted TOML::parse
  to return crate::Result<Expr> and map the internal PErr to
  parsers::Error::{SyntaxError, Alloc, StackOverflow}.
- src/parsers/toml/lexer.rs (modify/delete): kept deleted.
- src/parsers/error.rs, src/runtime/error.rs: removed the
  From<toml::lexer::Error> impls that referenced the deleted file.
- src/runtime/bake/dev_server/js_escape.rs: adapted to return
  Result<(), crate::Error> with crate::Error::SyntaxError.
- src/runtime/api/TOMLObject.rs: matched on bun_parsers::Error::Alloc(_)
  in place of the removed bun_core::err!("OutOfMemory").
- src/ast/lexer_log.rs: doc comment combines both sides (js only, new
  crate::Error syntax).

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found on this pass. Deferring to a maintainer for final sign-off given the scope — a full parser rewrite, a new public TOML.stringify API, and a user-visible tightening of what bunfig.toml accepts.

Checked the scanner's bounds handling at position 0 (check_underscore's wrapping_sub, peek_at past EOF), the meta HashMap keying on arena pointer addresses (stable for the parse lifetime), that append_property carries own_key_property_flags so the #33072 __proto__ hardening is preserved, and that the reordered JSC__JSValue__toISOString signature has no other callers. All 31 prior review threads are resolved.

Extended reasoning...

Overview

This PR replaces Bun's TOML parser with a from-scratch TOML v1.1.0 implementation (~1,860 lines in src/parsers/toml.rs), adds a new public Bun.TOML.stringify API (~530 lines in TOMLObject.rs), extracts a shared expr_to_js conversion into src/runtime/api.rs (now used by both TOML and JSON5), extracts the bake dev-server's JS-escape decoder into a standalone module, deletes the old TOML lexer and three now-unused E::Object rope helpers, fixes a dead-write bug in the pre-existing JSC__JSValue__toISOString C++ binding while reordering its parameters, and adds ~8,500 lines of generated conformance tests plus hand-written coverage, docs, and .d.ts updates.

Security risks

The parser processes untrusted input (config files, module imports). The relevant surfaces — UTF-8 validation via simdutf before any byte scanning, bounded peek_at, checked integer accumulation with explicit overflow flags, StackCheck on recursive value parsing, and __proto__ handled as an own property via own_key_property_flags — were reviewed and look correct. The O(N²) duplicate-key check on very wide tables (documented in the thread) is a pre-existing shape and not a regression. No injection or auth surface.

Level of scrutiny

High. The TOML parser backs bunfig.toml, so a defect here can break Bun's own startup. The PR also introduces a new public API and a deliberate behavioral tightening (~1% of sampled real-world bunfigs will newly error). These are product-level decisions a maintainer should sign off on, independent of correctness.

Other factors

The PR has been through 31 review threads (all resolved), including a maintainer review from Jarred whose date-conversion concern was addressed by routing through JSC's dateCache. Test coverage is unusually strong: the full official toml-test suite (708 cases, every rejection asserting the exact message), 217 round-trip assertions, and targeted GC/recursion/boundary tests. The bug-hunting pass on the current head found nothing new. None of that changes the fact that a ~2,400-line native rewrite plus a new public API is outside the bar for auto-approval.

@dylan-conway
dylan-conway merged commit aca54d5 into main Jul 16, 2026
16 of 35 checks passed
@dylan-conway
dylan-conway deleted the claude/toml-test-suite branch July 16, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants