toml: reject bare identifiers at value position

[robobun]

Fixes #31251.

Repro

$ bun -e 'console.log(JSON.stringify(Bun.TOML.parse("a = @")))'
{"a":"@"}

Per the TOML spec the right-hand side of key = value must be a quoted string, number, boolean, datetime, array, or inline table. @ is none of those.

Cause

The TOML lexer classifies @, a-z, A-Z, $, _ followed by identifier-part characters as t_identifier — this is deliberately broad so bare keys like foo-bar work in key position. But parse_value_inner was also accepting t_identifier and wrapping it in an E.String, so any bare identifier-ish token on the right of = silently became a string. Same arm existed in the legacy Zig (src/parsers/toml.zig L262-267).

Additionally, the TOML 1.0.0 spec (§Float) lists inf, +inf, -inf, nan, +nan, -nan as valid floats, but the lexer classified bare inf / nan as t_identifier (only true / false got length-based promotion). Pre-fix they silently became the strings "inf" / "nan"; the signed forms produced 0 because t_plus / t_minus read self.lexer.number before expect(t_numeric_literal) failed on the identifier.

Fix

1. Drop the t_identifier arm in parse_value_inner. Falls through to the existing catch-all self.lexer.unexpected(), which produces e.g. Unexpected @ at the right location. The t_identifier arm in parse_key_segment is untouched, so bare keys still work.
2. Promote inf / nan to t_numeric_literal in the lexer with f64::INFINITY / f64::NAN in self.number. Covers all six spec forms: bare inf / nan hit the numeric-literal value arm, and +inf / -inf / +nan / -nan flow through t_plus / t_minus. Bare keys spelled inf / nan keep working because parse_key_segment's numeric-literal arm uses self.lexer.raw() (source text) for the key name, not self.number.
3. Switch TOMLObject::parse from print_json → JSC JSONParse to ExprJsc::to_js (the same direct Expr → JSValue walker JSONCObject and the import foo.toml module loader already use). Strict JSON has no Infinity / NaN, so the round-trip was both wasteful and unable to carry those values — Bun.TOML.parse("a = inf") would otherwise fail at JSONParse with Failed to parse JSON.

Rebase conflict resolution

Rebased onto main to pick up #31255 (the fix for #31252, which added a log.has_errors() recovery check to TOMLObject::parse). Both PRs touch the same function. The resolution keeps both: my new ExprJsc::to_js call happens after the log.has_errors() surface-errors check, so lexer error-recovery diagnostics (a = [1 2]) still throw correctly.

Verification

$ bun -e 'try { Bun.TOML.parse("a = @") } catch (e) { console.log(e.message) }'
Unexpected @
$ bun -e 'try { Bun.TOML.parse("a = foo") } catch (e) { console.log(e.message) }'
Unexpected foo
$ bun -e 'console.log(Bun.TOML.parse("a = inf").a === Infinity)'
true
$ bun -e 'console.log(Number.isNaN(Bun.TOML.parse("a = -nan").a))'
true
$ bun -e 'console.log(JSON.stringify(Bun.TOML.parse("@foo = 1")))'  # bare key still ok
{"@foo":1}
$ bun -e 'try { Bun.TOML.parse("a = [1 2]") } catch (e) { console.log(e.message) }'  # #31252 regression check
Expected t_comma but found 2

test/js/bun/resolve/toml/toml-parse.test.ts covers:

• @, foo, @foo, $, _bar on the RHS all throw (bare identifier rejection)
• Identifier inside an inline table (a = { x = foo }) throws
• Identifier inside an array (a = [foo]) throws
• true / false still parse as booleans
• Bare keys @foo, $bar, foo-bar still parse as keys
• inf, +inf, -inf, nan, +nan, -nan return Infinity / NaN / -Infinity correctly
• Bare keys spelled inf / nan still work
• infinity, nan1, inf-foo at value position still throw (they only match the identifier arm, not the promoted keywords)
• a = [1 2] (from TOML arrays accept missing commas in values like a = [1 2] #31252) still throws on the new ExprJsc::to_js path

[coderabbitai]

Warning

Review limit reached

@robobun, we couldn't start this review because you've used your available PR reviews for now.

Your plan currently allows 1 review/hour. Refill in 49 minutes and 42 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6c55492f-dbc3-456b-9161-acf520982b79

📥 Commits

Reviewing files that changed from the base of the PR and between f161e03 and a4e823d.

📒 Files selected for processing (4)

• src/parsers/toml.rs
• src/parsers/toml/lexer.rs
• src/runtime/api/TOMLObject.rs
• test/js/bun/resolve/toml/toml-parse.test.ts

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[robobun]

^{Updated 4:15 PM PT - May 23rd, 2026}

✅ Your commit a4e823d8 has passed in Build #57373! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 31253

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

bun-31253 --bun

[claude]

Thanks for the inf/nan fix — that resolves my earlier concern. The follow-up also swaps Bun.TOML.parse's output pipeline from print_json → JSONParse to ExprJsc::to_js, which changes how every parsed value is materialized; it mirrors JSONCObject exactly and looks right, but it's enough of a scope expansion beyond the original fix that a maintainer should give it a quick glance.

Extended reasoning...

Overview

Four files: src/parsers/toml.rs drops the t_identifier arm from parse_value_inner (the original #31251 fix); src/parsers/toml/lexer.rs adds a len-3 keyword arm promoting inf/nan to t_numeric_literal (addressing my prior review); src/runtime/api/TOMLObject.rs replaces the print_json → to_js_by_parse_json round-trip with ExprJsc::to_js so Infinity/NaN survive; and ~70 lines of new tests in toml-parse.test.ts.

Security risks

None. TOML parsing is a config-file format with no auth/crypto/permission surface; the change tightens validation (rejecting previously-accepted invalid input) and swaps one internal AST→JSValue path for another already used by JSONC and the .toml module loader.

Level of scrutiny

Medium. The parser/lexer edits are mechanical and follow existing patterns exactly (the new len-3 arm is structurally identical to the len-4/len-5 true/false arms; parse_key_segment's t_numeric_literal arm already uses raw() so bare keys named inf/nan keep working, and there's a test for it). The TOMLObject.rs change is a byte-for-byte copy of JSONCObject.rs:31-38 including the ToJSError mapping and the unreachable!() for non-literal variants — and the TOML parser only emits E::Object/E::Array/E::String/E::Number/E::Boolean, so that's sound.

Other factors

My earlier inf/nan concern is fully resolved per option (a). The reason I'm not auto-approving is the TOMLObject.rs pipeline swap: it's correct and well-motivated (strict JSON can't round-trip Infinity/NaN), but it changes the materialization path for all Bun.TOML.parse output, not just inf/nan, and it grew out of a follow-up rather than being part of the original PR scope/description. Existing toml.test.js fixture tests do exercise Bun.TOML.parse through the new path, and the new tests cover all six spec float forms plus negative cases (infinity, nan1, inf-foo), so coverage is good — this just feels like the one piece a human should explicitly sign off on.

[claude]

Thanks for the inf/nan follow-up — the lexer promotion and tests look right. I'm deferring rather than auto-approving because the TOMLObject::parse change swaps the whole output pipeline (print_json→JSONParse → ExprJsc::to_js); it mirrors JSONCObject so it's almost certainly fine, but it's a runtime-API behavior change worth a quick human glance, and robobun still shows a ❌ on fbd5dd8.

Extended reasoning...

Overview

Four files: src/parsers/toml.rs drops the t_identifier arm from parse_value_inner so bare identifiers on the RHS of = now error per the TOML spec; src/parsers/toml/lexer.rs adds a length-3 keyword arm promoting inf/nan to t_numeric_literal (mirroring the existing true/false arms); src/runtime/api/TOMLObject.rs replaces the print_json → JSONParse round-trip with ExprJsc::to_js; and test/js/bun/resolve/toml/toml-parse.test.ts adds seven targeted tests.

Security risks

None. This is parser correctness for a data-format API; no auth, crypto, permissions, filesystem, or network surface is touched. Input remains bounded by the existing arena/stack-check machinery.

Level of scrutiny

Medium. The lexer and parser changes are mechanical and pattern-matched against existing code (true/false handling, parse_key_segment's t_numeric_literal arm using raw()). The TOMLObject.rs change, however, swaps the entire Expr→JSValue materialization path for Bun.TOML.parse. It is a byte-for-byte copy of JSONCObject.rs:31-38 and the TOML parser only emits E::{Object,Array,String,Number,Boolean} (all handled by data_to_js), so the unreachable!() is sound — but it's still a runtime-API pipeline change rather than a localized bug fix, and the kind of thing a maintainer should sign off on.

Other factors

• My earlier inline concern (inf/nan regressing to a hard error) was fully addressed in fbd5dd8 via option (a), with tests covering all six spec forms plus bare-key and near-miss cases.
• No CODEOWNERS entries cover these paths.
• robobun's status comment still shows a ❌ on Build #57129 for fbd5dd8; 1cac7bb is a CI retrigger but I don't see a green status yet, so I'd rather not auto-approve over a red build.
• Test coverage for the new behavior is thorough; existing toml-parse.test.ts cases also exercise the new ExprJsc::to_js path indirectly (string escapes, objects), which gives some confidence the pipeline swap doesn't regress encoding.

[claude]

LGTM — the inf/nan lexer promotion and ExprJsc::to_js swap fully address the earlier feedback, and the TOMLObject error-mapping is a verbatim copy of the proven JSONCObject path.

Extended reasoning...

Overview

Four files: drops the t_identifier arm from parse_value_inner (src/parsers/toml.rs, 6 lines removed), adds a length-3 keyword arm promoting inf/nan to t_numeric_literal in the lexer (src/parsers/toml/lexer.rs), swaps Bun.TOML.parse's output pipeline from print_json → JSONParse to ExprJsc::to_js (src/runtime/api/TOMLObject.rs), and adds ~70 lines of tests.

Security risks

None. This is TOML parsing of caller-provided strings; worst case is a thrown SyntaxError or a wrong value. No auth, crypto, filesystem, or network surface. The new unreachable!() in TOMLObject.rs covers only CannotConvertArgumentTypeToJS / CannotConvertIdentifierToJS / MacroError, which the TOML parser cannot produce — it only constructs E::Object, E::Array, E::String, E::Number, E::Boolean (and this PR removes the only arm that ever did anything else).

Level of scrutiny

Low-to-moderate. The parser and lexer changes are mechanical: the removed arm falls through to an existing catch-all, and the new 3 => lexer arm follows the exact pattern of the adjacent 4 => (true) / 5 => (false) arms. The TOMLObject.rs change is the largest, but the match parse_result.to_js(global) block is byte-identical to JSONCObject.rs:31-38, so it's a copy of an already-shipping pattern rather than a new design. The log.has_errors() guard from #31255 is correctly preserved before the new call, and the #31252 regression test still passes on this path.

Other factors

• My prior review flagged that removing the t_identifier arm would regress spec-valid inf/nan. The author implemented exactly the suggested fix (option a: lexer promotion) and went further to fix the JSValue pipeline so Infinity/NaN actually surface — strict JSON can't round-trip them. That comment is resolved.
• Test coverage is thorough: bare identifier rejection (top-level, in arrays, in inline tables), all six inf/nan spec forms, inf/nan as bare keys, near-miss identifiers (infinity, nan1, inf-foo), booleans, and bare-key alphabet preservation.
• No CODEOWNERS entries match these paths.
• The bug-hunting system found no issues on this revision.

[robobun]

The TOML parser was rewritten for v1.1.0 conformance in #32953, which landed on main (aca54d5). It deletes src/parsers/toml/lexer.rs and replaces src/parsers/toml.rs, so this PR no longer applies, and #31251 was closed as a duplicate of #31250.

The behavior this PR adds is covered on current main. Running the tests added here against main: every rejection case passes (a = @, a = foo, a = $, a = _bar, { x = foo }, [foo], infinity, nan1, inf-foo all throw a SyntaxError), and inf/nan with signs parse as floats while inf = "ok" and nan = "ok" still work as keys.

The one test here that does not pass on main is the one asserting that @foo = "ok" and $bar = "ok" are still accepted as bare keys. Main rejects those on purpose (Expected a key but found '@'): TOML limits bare keys to A-Za-z0-9_-, which is what #31256 and the toml-test suite (invalid/key/bare-invalid-character-*) assert, so that test was pinning old behavior rather than something worth keeping. Closing.