toml: fix string escapes, multiline trimming, date-times, integers, and dotted keys - #32994
toml: fix string escapes, multiline trimming, date-times, integers, and dotted keys#32994robobun wants to merge 2 commits into
Conversation
…nd dotted keys The TOML lexer is a copy of the JavaScript lexer, not a TOML lexer, and silently corrupted several classes of valid documents: - \UXXXXXXXX (the 8-digit Unicode escape) was unimplemented; it and every other reserved escape fell into a catch-all that emits the literal character after the backslash, so "\U000003B4" loaded as "U000003B4". Reserved escapes now error, and \u/\U reject non-scalar code points. - The newline after an opening """/''' was not trimmed, a line-ending backslash did not consume the following whitespace, and quotes adjacent to a closing delimiter were mis-tokenized. - Integers were accumulated in an f64 at lex time, so 9223372036854775807 loaded as 9223372036854776000. They now go through a checked u64 and error when out of the 64-bit range or not exactly representable. - Date-times had no lexer branch, so every document with one was rejected. All four RFC 3339 shapes now lex; the value surfaces as a string. - Float-looking bare keys were not split on '.', and a dotted key continuing with a digit segment (a.1 = 1) was a hard error. toml-lang/toml-test 1.0 corpus via Bun.TOML.parse: valid 150/209 -> 188/209 (silently mis-parsed 15 -> 1), invalid accepted 148/497 -> 126/497, with no valid document regressing. Fixes #28680 Fixes #28687
|
Warning Review limit reached
More reviews will be available in 51 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
|
Updated 10:33 PM PT - Jun 27th, 2026
❌ @robobun, your commit ba0ca99 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 32994That installs a local version of the PR into your bun-32994 --bun |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
The canonical example from https://toml.io/en/v1.0.0 is the reproduction in issue 22426; it was rejected at the offset date-time.
|
Closing in favor of #32953, which was already open when this one was created and which my duplicate search missed (it only looked for robobun-authored PRs). #32953 is a ground-up rewrite that reaches 708/708 on the official toml-test suite and covers every fix here: the The per-class analysis and the before/after toml-test breakdown in the description may still be useful as a reference for the old parser's failure modes. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| } else { | ||
| let mut int_value: u64 = 0; | ||
| let mut int_overflow = false; | ||
| for &c in text { | ||
| match int_value | ||
| .checked_mul(10) | ||
| .and_then(|v| v.checked_add(u64::from(c - b'0'))) | ||
| { | ||
| Some(v) => int_value = v, | ||
| None => { | ||
| int_overflow = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| // The sign is a separate token, so the magnitude of | ||
| // i64::MIN (2^63) must lex; `parse_value` applies the minus. | ||
| self.number = self.check_exact_integer(int_value, int_overflow, 1u64 << 63)?; | ||
| } |
There was a problem hiding this comment.
🔴 The new check_exact_integer guard runs at lex time, before the parser knows the token is a key, so a valid TOML bare key of 16+ digits that isn't an exact f64 (e.g. 9223372036854775807 = 1 or a Snowflake ID like 1719554400000000001 = "event") now fails the whole parse with "cannot be represented exactly". This is a regression — pre-PR the ≥10-digit path went through the lossy-but-Ok parse_double, and parse_key_segment only ever read raw(), so these keys parsed correctly. The prefixed path has the same issue: 0xFFFFFFFFFFFFFFFF = 1 is a valid bare key (every char is alphanumeric) that now hits "outside the 64-bit range".
Extended reasoning...
What the bug is. TOML bare keys are arbitrary [A-Za-z0-9_-]+ strings — a run of 19 digits is a perfectly valid key, and its numeric value is irrelevant. But check_exact_integer is called from inside parse_numeric_literal_or_dot, which runs as part of Lexer::next() with no key/value context. So when the first token of a line is 9223372036854775807, the lexer rejects it as an unrepresentable integer before parse_key ever gets a chance to use raw() as a string.
Concrete walkthrough. Take 9223372036854775807 = 1:
Lexer::next()sees a leading digit and dispatches toparse_numeric_literal_or_dot(lexer.rs:928-931).scan_date_timedoesn't match.- The decimal-integer branch hits
!has_dot_or_exponentwithtext.len() == 19 ≥ 16, so it accumulates into a checkedu64(int_value = 9223372036854775807, no overflow) and callscheck_exact_integer(int_value, false, 1u64 << 63)(lexer.rs:480-498). - The
value > maxcheck passes (it's < 2^63). Thenvalue as f64rounds to9223372036854775808.0, and(9223372036854775808.0 as u64) != 9223372036854775807, soadd_syntax_error("cannot be represented exactly...")fires. LexerLog::add_syntax_error(src/ast/lexer_log.rs:109-112) logs and unconditionally returnsErr(syntax_err()). The?propagates out ofcheck_exact_integer→parse_numeric_literal_or_dot→next(), and the whole parse aborts.
parse_key never runs. It would have happily taken self.lexer.raw() — the source bytes "9223372036854775807" — and ignored self.number entirely, just as the new T::t_numeric_literal arm in parse_key does for shorter keys.
Why this is a regression. Before this PR, the same input took the else branch (self.end - self.start < 10 was false) and fell through to bun_core::wtf::parse_double(text), which returns a lossy f64 but never errors. The old parse_key_segment then matched T::t_numeric_literal and built the key from self.lexer.raw(), producing {"9223372036854775807": 1}. The PR's own toml-test diff ("no valid document regresses") missed it because the corpus doesn't include a 16+-digit non-exactly-representable bare key.
Why nothing prevents it. The lexer is context-free; the only place that knows the token is in key position is parse_key, which runs after next() has already returned Err. The new key tests in this PR (10e3, 2018_10, 1) all stay under 16 post-filter digits.
Second instance — prefixed integers. 0xFFFFFFFFFFFFFFFF = 1 is also a valid bare key (every character is in A-Za-z0-9). The base-16 path accumulates u64::MAX and calls check_exact_integer(u64::MAX, false, i64::MAX as u64) (lexer.rs:354-356), which fails the value > max check with "outside the 64-bit range". Pre-PR this path accumulated lossily into self.number via * base + digit and returned Ok.
Impact. Real-world TOML using Snowflake IDs, nanosecond timestamps, or other 64-bit identifiers as bare keys (e.g. [1719554400000000001]) will stop loading. The error message ("quote it to load it as a string") is misleading in key position.
Fix. The integer-precision/range error needs to be deferred until the token is actually consumed as a value. One approach: have check_exact_integer record the diagnostic on the lexer (e.g. self.inexact_integer = Some(msg)) and return Ok(as_float); then have parse_value's T::t_numeric_literal arm raise it, while parse_key ignores it (it only reads raw()). Alternatively, make the lexer emit the check as a non-fatal logged error and have the key path clear it — but that would interact badly with the existing log.has_errors() check on the Ok path. Deferring is cleaner.
| } | ||
| // The sign is a separate token, so the magnitude of | ||
| // i64::MIN (2^63) must lex; `parse_value` applies the minus. | ||
| self.number = self.check_exact_integer(int_value, int_overflow, 1u64 << 63)?; |
There was a problem hiding this comment.
🟡 Using max = 1u64 << 63 here lets the magnitude of i64::MIN lex (so -9223372036854775808 works), but it also lets bare/+-prefixed 9223372036854775808 (= 2^63) through: value > max is false at equality, and 2^63 is a power of two so it round-trips f64↔u64 exactly. parse_value's t_numeric_literal/t_plus arms don't re-check the bound after the sign is known, so Bun.TOML.parse("a = 9223372036854775808") succeeds even though TOML's signed-64 range tops out at 2^63−1. Harmless (one boundary value, lossless, not a regression) — just noting it since the PR description says "values outside the 64-bit range … are errors".
Extended reasoning...
What happens
The decimal-integer slow path calls check_exact_integer(int_value, int_overflow, 1u64 << 63). The comment explains why: the sign is a separate token, so the lexer must accept the magnitude 2^63 to allow -9223372036854775808 (i64::MIN) — parse_value applies the minus afterward. The side effect is that the same magnitude without a minus also lexes.
Step-by-step trace for a = 9223372036854775808
9223372036854775808has 19 digits (≥ 16), so it goes through the checked-u64accumulation. 2^63 fits in au64, soint_overflow = falseandint_value = 9223372036854775808.check_exact_integer(2^63, false, 1u64 << 63)runs:overflowed || value > max→false || 2^63 > 2^63→ false. No "outside the 64-bit range" error.as_float = 2^63 as f64is exact (2^63 has one significant bit), and(2^63 as f64) as u64 == 2^63. No "cannot be represented exactly" error.- Returns
Ok(9.223372036854776e18)(which is exactly 2^63).
parse_value'sT::t_numeric_literalarm readsself.lexer.numberand emits it directly; theT::t_plusarm does the same after consuming the+. Neither re-checks the upper bound now that the sign is known.
So both a = 9223372036854775808 and a = +9223372036854775808 succeed with the value 9223372036854775808, even though TOML §Integer specifies the range −2^63 .. 2^63−1 and the PR description says "Values outside the 64-bit range … are errors".
Why the existing checks don't catch it
The range check uses strict >, and max is set exactly to 2^63 so that the negated form survives. The exact-representation check is independent of sign and 2^63 happens to be one of the few values above i64::MAX that an f64 represents exactly, so neither guard fires for this one value. Every other out-of-i64 positive integer is caught by either u64 overflow or the round-trip check.
Addressing the refutation
One verifier argued this is an intentional documented tradeoff and not a regression. All of that is accurate — the inline comment names the tradeoff, pre-PR Bun also accepted this input (via parse_double, with the same numeric result), there's no precision loss, and the PR explicitly leaves 126 invalid documents in the "accepted" bucket. That's exactly why this is filed as a nit rather than a blocking bug: the only thing it contradicts is the PR description's wording, and a one-line note is enough for the author to decide whether to tighten it or leave it.
Possible fix (if desired)
Either re-check the bound in parse_value once the sign is known (reject number == 2^63 in the t_numeric_literal/t_plus arms), or lex with max = i64::MAX as u64 and special-case the t_minus path to allow exactly 2^63. Both are small; leaving it as-is is also defensible since the value is lossless.
|
Correction to the description for anyone using this as a reference: the "no valid document regresses" claim is wrong. A later review pass found that because 9007199254740993 = 1 # valid TOML (bare keys are always strings); Bun 1.4.0 parses it; this diff errors
[175928847299117063]
t = { 99999999999999999999 = 1 }toml-test has no such case (its |
1 participant
Fixes #28680
Fixes #28687
Bun's TOML parser silently corrupts several classes of valid documents and rejects every document that contains a date-time. All of these reproduce on 1.4.0 and current
main, through both the module loader (import x from "./f.toml",with { type: "toml" }) andBun.TOML.parse:Cause
src/parsers/toml/lexer.rsis a copy of the JavaScript lexer, not a TOML lexer.decode_escape_sequencesuses the JavaScript escape table. Its catch-all emits the literal character after the backslash, so the unimplemented\UXXXXXXXX(one of the two Unicode escapes TOML defines) becameUplus eight hex digits, and every other reserved escape (\a,\e,\/,\) was silently dropped to its trailing character."""/''', so the newline the spec says to trim stays in the value. A line-ending backslash dropped only its own newline, not the following whitespace, and\with trailing spaces before the newline (valid per the ABNF) was treated as an escape of a space. One or two quotes directly against a closing delimiter (""""x"""") were mis-tokenized.f64(self.number * base + digit), so precision is lost at lex time. TOML 1.0 requires 64-bit integers be handled losslessly or error.1979lexed as an integer and the-became a stray token.parse_key_segmentused the raw text of a numeric-literal token as the key, so3.14159never split on the.. The same greedy lexing madea.1 = 1a hard error: the.1lexes as a fractional literal instead of.followed by the bare key1.Fix
All in
src/parsers/toml/lexer.rsandsrc/parsers/toml.rs.\UXXXXXXXXdecodes as the 8-digit Unicode escape. Both\uand\Unow reject surrogates and code points past U+10FFFF.Invalid escape sequence "\X" in TOML string) instead of silently emitting the literal character, and the JavaScript-only legacy octal escape machinery is deleted.\xHHand\u{...}are kept: they are pre-existing lenient extensions no valid document can contain (\xis also TOML 1.1), and an existing test depends on\u{...}."""/'''is trimmed. A line-ending backslash (including\+ trailing whitespace + newline, permlb-escaped-nl) consumes all whitespace and newlines up to the next non-whitespace character;\followed by non-whitespace on the same line is an error. Up to two quotes against a closing delimiter belong to the content.u64. Values outside the 64-bit range or not exactly representable as a JavaScript number are errors, and hex, octal, and binary literals go through the same check. Floats are unchanged.2020-01-01xand1997-09-0909:09:09still fail. There is no date-time node in the TOML AST, so the value surfaces as the verbatim source string.parse_keysplits numeric-literal key tokens on.and treats a.-prefixed numeric literal after a segment as a dotted-key continuation, so3.14159 = "pi",a.1 = 1,[3.14], anda.2001-02-08 = 1all produce the right keys. A trailing dot (a. = 1) or empty segment (a..5 = 1, previously the key".5") is now an error.Verification
Against the official toml-lang/toml-test 1.0 corpus (209 valid, 497 invalid documents) through
Bun.TOML.parse:No valid document regresses (the before and after file lists were diffed). The one remaining mis-parse is
inf/nanloading as strings, a separate pre-existing class.New tests in
test/js/bun/resolve/toml/toml-parse.test.tscover each class againstBun.TOML.parse, andtest/js/bun/resolve/toml/toml.test.jsgains an end-to-end test throughimport "./f.toml"andwith { type: "toml" }. 16 of the new tests fail on an unpatched build.Behavior changes
Integer "9223372036854775807" cannot be represented exactly as a JavaScript number; quote it to load it as a string. The spec mandates lossless-or-error. BigInt was considered, butBun.TOML.parseconverts its AST by printing JSON text and runningJSON.parseover it, which cannot carry a BigInt, so that path would need a rewrite first. Erroring is the other behavior TOML permits and is contained to the lexer."C:\Users\x"was already corrupted (\UbecameU,\tbecame a tab); it now errors at the escape. The TOML spelling for a backslash path is a literal string:'C:\Users\x'.Pre-existing issues this does not touch
Bun.TOML.parsethrowsFailed to parse JSONfor any string value containing U+0000 through U+001F without a JSON short escape;Bun.TOML.parse('a = "\\u0000"')already fails on 1.4.0 becauseprint_jsoninsrc/js_printerdoes not escape those code points. The module loader path handles them. That is a printer bug, not a parser bug.-or mix digits and-without being a date (-key,34-11) are still rejected; the lexer tokenizes the leading-or digit run before it knows it is in key position.inf/nanfloats, leading-zero integer rejection, bare-word values, duplicate-key and table-redefinition enforcement, newline requirements between statements, and calendar validation of dates (Feb 30) are unchanged. toml: restrict bare keys and values to the TOML 1.0.0 spec #31256 and toml: reject bare identifiers at value position #31253 cover some of those.#28682 fixes the two multi-line string issues here; this change covers those two plus the cases that PR does not reach (
\with trailing whitespace before the newline,\followed by non-whitespace, and the closing-delimiter quote counting), so it can supersede it.