Skip to content

shell: reject !, brace groups, here-documents and stray then/elif/else/fi at parse time - #43922

Open
robobun wants to merge 2 commits into
robobun/7610aab4/shell-reject-reserved-wordsfrom
robobun/7610aab4/shell-reject-bang-braces-heredoc
Open

robobun wants to merge 2 commits into
robobun/7610aab4/shell-reject-reserved-wordsfrom
robobun/7610aab4/shell-reject-bang-braces-heredoc

Conversation

@robobun

@robobun robobun commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #12602

Part of #43860. Related to #10465. Stacked on #43921.

Problem

  • Bun Shell does not implement !, brace groups or here-documents (<<). if ! false; then echo THEN; else echo ELSE; fi prints ELSE. The body of false && {⏎ echo BODY⏎} runs. Here-document lines run as commands.
  • A stray then, elif, else or fi runs as a missing command.
  • Cause: the fallthrough in Parser::parse_compound_cmd (src/shell_parser/parse.rs). parse_redirect reads << as <.

Fix

  • ! joins the reserved-word table. A delimited { or } gets the same error. A stray if-family word fails with Unexpected token: `fi` . An escaped word (\fi) stays plain.
  • parse_redirect fails the parse for <<. The lexer reads a digit before < as a file descriptor only for fd 0 (Bun shell parses some redirections incorrectly #12602).
  • Verified: test/js/bun/shell/bunshell.test.ts (25 new cases fail on the base branch), exec.test.ts, test/cli/run/run-shell.test.ts.
  • Self-reviewed: 13 concerns raised, 12 addressed. Rejected: a separate PR for the lexer change (Notes).

Background

  • Brace expansion ({a,b}) also starts with BraceBegin. The new arm needs a delimiter after the brace, so {a,b} stays.
  • parse_if_clause consumes each if-family word of an if.
  • Considered a word list alone. It cannot see <<, which is a redirect.

Downsides

  • Behaviour change: these scripts now throw before they start. ok || { …; } exited 0 when ok passed. \if no longer opens an if.
  • cmd << file no longer reads file. In cmd 1< file, 1 is now an argument.
  • Parse: -9 instructions per command, +4 per redirect. With 400 interpolated values: 2,057,261 to 1,093,667. Binary: +768 bytes.
Notes

Measurements. Release builds of the base branch (ca7e023) and of this PR. Instruction counts from gdb single-stepping over a window on bun-profile (entry of a function to its return, second call). valgrind, perf and strace are not available in the build container.

measurement base branch this PR
createParsedShellScript, echo a; echo b | cat; true && echo c (4 commands) 72,383 72,347 (-9 per command)
one parse_compound_cmd call, echo a 1,044 1,037
one parse_redirect call with a redirect 307 311
Interpreter::parse, 64 commands echo x > /dev/null 245,719 245,463 (-4 per command)
shell_cmd_from_js, 8 interpolated values (3-char, 4-char, UTF-16, 12-char) 10,474 / 10,606 / 12,569 / 11,871 the same
$.escape("hello") 217 216
allocator calls in createParsedShellScript, the 4-command script 43 43
Interpreter::parse, K commands with M by-reference values, K=M=100 / 200 / 400 377,902 / 815,906 / 2,057,261 317,002 / 574,106 / 1,093,667
.text + .rodata (size -A) 77,982,128 77,982,896

parse_compound_cmd reads the reserved-word table once per command. The base branch reads it twice: once for if and once for the unsupported words. reserved_word_at looks the word up first. It scans the interpolated ranges and the escaped positions only on a hit. main scans the interpolated ranges for each command, so its cost grows with commands times values (2,048,084 at K=M=400). The K=200 and K=400 windows contain 1 and 3 vDSO clock_gettime calls, which run at full speed and are not counted. Two measurements of such a window differ by 6 to 18 instructions. Each other window repeats exactly.

Before and after, on the base branch and on this PR:

script before after
if ! false; then echo THEN; else echo ELSE; fi prints ELSE, exit 0 "!" is a reserved word that Bun Shell does not support yet. …
false && {⏎ echo IN_GROUP⏎}⏎echo tail prints IN_GROUP, tail "{" is a reserved word …
cat <<-EOF⏎<TAB>echo BODY_LINE_RAN⏎<TAB>EOF⏎echo tail No such file or directory: -EOF, then each body line runs Here-documents "<<" are not supported yet.
cat 1<<EOF⏎echo BODY_LINE_RAN⏎EOF Redirection with no file Here-documents "<<" are not supported yet.
echo a; fi; echo b prints a, b, exit 0 Unexpected token: `fi`
if true; then⏎fi Expected "else", "elif", or "fi" but got: Eof Unexpected token: `fi`
cat 0< input.txt reads the file reads the file
echo hi 1< notes.txt appends hi to notes.txt, prints nothing prints hi 1, the file stays as it is
echo hi 2< notes.txt prints hi prints hi 2
\fi, f\i, \then bun: command not found: fi (or then) the same

A digit before < (#12602). eat_redirect peeked the < after the digit and did not eat it, so eat_simple_redirect_operator counted the same < as a second one and set APPEND. 0< file had the flags of <<, and 1< file had the flags of 1>> file. The lexer now eats the < and accepts only fd 0 in front of it. For 1< and 2< the digit joins the word and < is a plain redirect of stdin. That is what #12602 asks for and what 3< already did. The here-document check needs this change: without it parse_redirect cannot tell 0< file from <<. The first version of this change ate the < for every digit. Then 1< file had the flags of 1> file and truncated the file. The review of this PR found it.

script bash base branch this PR
./script1<file (the script of #12602) runs ./script1 command not found: ./script runs ./script1
echo hi1< notes.txt prints hi1 appends hi to notes.txt prints hi1
echo hi 1< notes.txt write error: Bad file descriptor appends hi to notes.txt prints hi 1
echo hi 2< notes.txt prints hi prints hi prints hi 2
echo hi 3< notes.txt prints hi prints hi 3 prints hi 3

bash reads a digit as a file descriptor only when the digit is a word of its own. Bun Shell has no read redirect for a file descriptor other than 0, so the last three rows differ from bash. The tests pin the output of each row and that the file stays as it is. A comment in the test marks the rows that differ from bash. #27257 and #33997 (both closed without a merge) had the word-boundary rule for < and >.

An escaped if-family word, checked against bash 5.2. The lexer records the position of each char that a backslash escaped (#43921). reserved_word_at reads that record, and every if-family check passes through it.

script bash base branch this PR
\fi fi: command not found the same the same
\if true; then echo yes; fi syntax error near then prints yes Unexpected token: `then`
if true; then echo yes; \fi syntax error: unexpected end of file prints yes Expected "else", "elif", or "fi" but got: Eof
if true; \then echo yes; fi syntax error near fi prints yes Unexpected token: `fi`

Who can see the behaviour change. A script whose construct sits on a branch that does not run. true || { echo failed; exit 1; }; echo after printed after and exited 0 before, with bun: command not found: } on stderr. With a failing guard (false || { echo warn; }) the fallback did not run and the script still exited 0. Both forms now fail the parse. package.json scripts and the lifecycle scripts of bun install run under Bun Shell by default on Windows, so a script of this shape fails there now. A script that escapes a word of an if (\if, \then, \fi) parsed as an if before and fails the parse now. No user reported !, a brace group or a stray if-family word. A read of the parser for #43860 found them. #10465 asks for here-documents. This PR does not add them: it replaces the run of the body lines with an error.

Self-review. A direct read raised eight concerns, all addressed: one lookup for the if family and the unsupported words must not route a word into the panic! of expect_if_clause_text_token (each call site follows a check of the same token, and four new tests pin glued words such as fi$x), the order of the lookup and the delimiter check (lookup first), \if changes for scripts that main accepted (stated, matches bash), escaped words inside $( ), backticks and subshells (16 scripts checked against bash), IfClauseTok::from_text had no caller left (deleted), a brace group on a branch that does not run (stated above), <<< gets the here-document error (stated below), and the changed message for an empty then-body (in the table above). A longer review raised five more. Addressed: the body did not link #12602 and #10465, the body did not say that nobody reported ! or brace groups, the test pinned hi 1 and hi 2 with no note that bash gives other output (now a comment in the test), and the base branch needs a maintainer decision first (this PR is stacked on it). Rejected: a separate PR for the lexer change, because the here-document check depends on it.

Not in this PR.

  • <<< (here-string) gets the here-document error, because the lexer reads it as << and <.
  • A digit that ends a longer word is read as a file descriptor before > and before 0<: echo a2> out.txt prints a and redirects stderr. bash writes a2 to the file. main does the same.

no test proof · iteration 2 · 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, test/cli/run/run-shell.test.ts

@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-reserved-words branch from fe49fb0 to 4a9aa17 Compare September 24, 2026 21:07
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-bang-braces-heredoc branch from 61e9254 to fbb1dae Compare September 24, 2026 21:07
@robobun

robobun commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Reproduced on 1.4.3-canary.1 and on the base branch (Linux x64):

import { $ } from "bun";
const r = await $`if ! false; then echo THEN; else echo ELSE; fi`.nothrow().quiet();
console.log(JSON.stringify(r.stdout.toString()), r.exitCode);
  • Before: "ELSE\n" 0, with bun: command not found: ! on stderr. The wrong branch ran.
  • With this branch: the $ call throws "!" is a reserved word that Bun Shell does not support yet. To run a command named "!", quote it. No command runs.

PR: #43922. It is stacked on #43921. #43861 is stacked on it.

@robobun

robobun commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 8:51 PM PT - Sep 24th, 2026

✅ @robobun, your commit 761e8c8202fca3cabd9fa97d6abfb68c72950767 passed in Build #120524! 🎉


🧪   To try this PR locally:

bunx bun-pr 43922

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

bun-43922 --bun

@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.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread src/shell_parser/parse.rs
Comment thread test/js/bun/shell/bunshell.test.ts Outdated
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-reserved-words branch from 4a9aa17 to b3f2624 Compare September 24, 2026 22:25
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-bang-braces-heredoc branch from fbb1dae to b29d9fc Compare September 24, 2026 22:25
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-reserved-words branch from b3f2624 to fe3bfa6 Compare September 24, 2026 22:37
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-bang-braces-heredoc branch from b29d9fc to 440ca75 Compare September 24, 2026 22:37

@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.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread src/shell_parser/parse.rs
Comment thread src/shell_parser/parse.rs Outdated
Comment thread test/js/bun/shell/bunshell.test.ts
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-reserved-words branch from fe3bfa6 to ca7e023 Compare September 25, 2026 00:57
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-bang-braces-heredoc branch from 440ca75 to 70b1dac Compare September 25, 2026 00:57

@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.

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-bang-braces-heredoc branch from 70b1dac to 2e45e47 Compare September 25, 2026 02:39

@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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread test/js/bun/shell/bunshell.test.ts
…y then/elif/else/fi

Bun Shell does not implement pipeline negation, brace groups or
here-documents. Each one ran as plain commands: `if ! cmd` took the wrong
branch, the body of `guard && { ...; }` ran when the guard failed, and the
lines of a here-document body ran as commands. A then/elif/else/fi with no
open if ran as a command that does not exist, and the script continued.

parse_compound_cmd now fails the parse for `!`, for a delimited `{` or `}`
in command position, and for a word of the if family that no if clause
consumed. It reads the reserved-word table once for the if family and for
the unsupported words. parse_redirect fails the parse for `<<`.

A backslash-escaped word of the if family is a plain word in every
position, as an escaped unsupported word already is.

The lexer reads a digit before `<` as a file descriptor only for fd 0.
`0< file` had the flags of `<<`. `1< file` and `2< file` had the flags of
`1>> file` and `2>> file`, so they wrote to the file.
@robobun
robobun force-pushed the robobun/7610aab4/shell-reject-bang-braces-heredoc branch from 2e45e47 to cee8742 Compare September 25, 2026 03:02

@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.

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant