Repository navigation
Conversation
Inside double quotes a backslash escapes only $, `, ", \ and newline (POSIX 2.2.3). The lexer also listed #, so "a\#b" printed a#b where bash and dash print a\#b.
|
Updated 4:05 AM PT - Oct 17th, 2026
❌ @robobun, your commit 8d339b8 has some failures in 🧪 To try this PR locally: bunx bun-pr 45496That installs a local version of the PR into your bun-45496 --bun |
|
Reproduced on 1.4.3-canary.1+367d939d9, beside bash 5.2.37 and dash 0.5.12: bun exec 'echo "a\#b"' # Bun: a#b bash, dash: a\#b
bun exec 'echo "\# Title"' # Bun: # Title bash, dash: \# TitleThe fix is this PR, #45496. CI on 8d339b8 (build 127295): 179 of 181 jobs passed and no test failed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe shell parser now preserves a backslash before ChangesShell Quoting
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: preserving the backslash before # inside double quotes. |
| Description check | |
The description explains the problem, fix, behavior change, standards basis, test coverage, verification results, and potential downsides. It does not use the exact template headings, but it provides … |
Full details: Description check
-
Explanation
The description explains the problem, fix, behavior change, standards basis, test coverage, verification results, and potential downsides. It does not use the exact template headings, but it provides the required information in greater detail.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding (a pre-existing gap on a trailing backslash, not introduced here), I checked the one-line change for sibling sites and ruled out: the lexer's # arm at src/shell_parser/parse.rs:2518 already treats # as literal in both quote states, so the removed entry had no other consumer; the ASCII fast path lists \ in SPECIAL_CHARS, so both Src::Ascii and Src::Unicode reach read_char; BACKSLASHABLE_CHARS (line 3937) has no #, so $.escape and interpolated values are unaffected; and src/js/builtins/shell.ts passes strings.raw, so the new "\#" TestBuilder cases exercise the fix rather than a cooked #.
Extended reasoning...
The change removes # from the set of characters a backslash escapes inside double quotes in the Bun shell lexer and adds seven tests, including an exhaustive printable-ASCII table and a non-ASCII { raw } case. It touches no security-sensitive surface; it is a parser behaviour change aligned with POSIX 2.2.3 and bash/dash. The only confirmed finding is a pre-existing truncation on a trailing backslash in an open double quote that the PR does not address, so a human should weigh whether it belongs in this PR; the core change itself has no sibling sites left unfixed.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/shell_parser/parse.rs— A double-quoted word that ends the script with a trailing backslash is silently truncated instead of producing an error, and this PR rewrites the same escape rule without closing that gap. At src/shell_parser/parse.rs:3749 the?on index_next returns None when the backslash is the last byte, so eat() at parse.rs:2383-2388 treats it as end of input and pushes Eof; no "unterminated" error is raised for the open double quote (only parse.rs:2892-2894 report unclosed constructs). Bash reports an unexpected EOF forecho "a\. Fix: when a backslash is the final character inside double quotes, return it as literal text (parse.rs:3745-3749) and have the lexer report an unclosed double quote at parse.rs:2889 rather than ending silently.Why this was flagged
Trigger: a script whose last character is a backslash inside an open double quote, e.g.
echo "abc\arriving through Bun.$,bun exec, or a.shfile. src/shell_parser/parse.rs:3749 uses?on index_next, so read_char returns None; parse.rs:3707-3717 eat() then returns None and parse.rs:2383-2388 calls break_word and exits the loop; parse.rs:2889-2900 only errors for an open subshell and pushes Eof. The result is the command running withabcas its argument and exit code 0, where bash and dash fail with an unexpected EOF. The base commit has the same?(confirmed in git show of 5866b5f lines 3740-3748), so this is pre-existing, but the PR edits the adjacent escape rule in the name of POSIX 2.2.3 and leaves this sibling case of the same backslash-in-double-quotes rule wrong. Remedy: treat the trailing backslash as literal and report the unterminated quote.Verification: Pre-existing. Trigger: any script whose input ends with a backslash inside an open double quote. /home/claude/bun/src/shell_parser/parse.rs:3745-3749 does
index_next().map(...)?, so 2889-2899 pushesToken::Eof; no "unterminated quote" diagnostic exists, so the command runs with exit 0 where bash exits 2. The?at 3749 is byte-identical on the base, so merging this PR makes nothing worse.
…l-dquote-backslash-hash
|
On the review finding about a backslash as the last byte inside an open double quote (
|
Problem
#.echo "a\#b"printsa#b. Bash 5.2 and dash printa\#b.read_char(src/shell_parser/parse.rs:3757). It has the five POSIX characters and also#, since the first shell commit (feat: Bun shell #7748).Fix
#from the list. The backslash stays in the word, as it does in"\x"and"\!".$,`,",\and newline inside double quotes. A#inside quotes never starts a comment, so the escape had no use.test/js/bun/shell/bunshell.test.ts(7 new tests, 5 fail without the fix). Also ranlex,parse,brace,execandrun-shell.Background
read_charreturns the next character of a shell script and anescapedflag. Every shell source passes through it: aBun.$template, a.shfile andbun exec.$.escapedo not change. The escaper table (BACKSLASHABLE_CHARS) has no#.BACKSLASHABLE_CHARSin place of the list. Then the quoting rule depends on the escaper table. One new test pins all 95 printable ASCII characters instead.Downsides
"\#"to get#now gets\#, as in bash."#"prints#before and after.Notes
Repro (1.4.3-canary.1+367d939d9, beside bash 5.2.37 and dash 0.5.12)
Unquoted text (
echo a\#bprintsa#b) and single-quoted text (echo 'a\#b'printsa\#b) are right without the fix and do not change. Two of the new tests pin that.Checked against bash
echowith a backslash before each of the 95 printable ASCII characters, in 7 double-quote shapes, throughBun.$and through bash 5.2.37. The shapes: the middle, the start and the end of the quotes, after a non-ASCII character, after a variable, inside$(...), and in quotes around$(...).#, one per shape. With the fix, 0 of 665 differ.a#b,a\#b,#,\#,a \# b,# leadandx\\#y, each bare, in double quotes, and through$.escapewith{ raw }. 21 of 21 print the value, without and with the fix..shfile andbun execprint the same asBun.$. They share the lexer.History
#is in the list in the first shell commit,1b1760a9c9(feat: Bun shell #7748). In that commit the comment arm of the lexer already skips a#inside quotes, so the entry never had a function.#.Tests
"\#"cases and the table of 95 characters. The unquoted case and the single-quoted case pass on both builds.{ raw }. A template literal gives the shell\u00E9foré(raw tagged template literals show escapes for non ascii text #8745).bunshell.test.ts458 pass (after the merge with main),lex.test.ts43 pass,parse.test.ts18 pass,brace.test.ts47 pass,exec.test.ts19 pass,run-shell.test.ts3 pass.Open PRs on the same lines
\rto this list and keeps#. The two branches conflict in one hunk. The resolution keeps the\rline and drops the#line.$.braceslexer. Its list has the five POSIX characters and no#.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/bunshell.test.ts