Skip to content

shell: only treat digit as fd-prefix redirect at word boundary - #27257

Closed
robobun wants to merge 4 commits into
mainfrom
claude/fix-shell-redirect-digit-stripping
Closed

robobun wants to merge 4 commits into
mainfrom
claude/fix-shell-redirect-digit-stripping

Conversation

@robobun

@robobun robobun commented Feb 20, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes the shell lexer treating a digit at the end of a word as an fd-prefix when followed by > / <.

Repro

$`echo abc1>file`       # wrote "abc\n" — lexed as `echo abc` + `1>file`
$`echo test100>out`     # wrote nothing — lexed as `echo test10` + `0>out` (stdin redirect)
$`echo "abc"1>file`     # wrote "abc\n" — lexed as `echo "abc"` + `1>file`
$`./script1<input`      # ran `./script` with `1<input`

POSIX only recognizes an IO number when the digits form their own word. bash writes abc1\n, test100\n, abc1\n, and runs ./script1 respectively.

Root cause

In the '0'...'9' arm of the lexer (src/shell/shell.zig), eat_redirect() was attempted unconditionally whenever a digit appeared in Normal state, then break_word() was called after the redirect was accepted — silently splitting the trailing digit off whatever word preceded it.

Fix

Before entering the fd-redirect path, require that the digit begins a new word:

  1. self.word_start == self.j — no text currently being accumulated, and
  2. the preceding token (if any) is not a word-continuation token (Text, Var, SingleQuotedText, DoubleQuotedText, CmdSubstEnd, Asterisk, brace tokens) — same classification break_word_impl already uses.

Digits that genuinely begin a word — after whitespace (echo 2>file), at start of input (2>file), or after an operator (echo foo;2>file) — still parse as fd prefixes.

How did you verify your code works?

  • USE_SYSTEM_BUN=1 bun test test/js/bun/shell/lex.test.ts -t "op_redirect digit" → fail
  • bun bd test test/js/bun/shell/lex.test.ts → 30/30 pass
  • bun bd test test/js/bun/shell/parse.test.ts → 17/17 pass
  • bun bd test test/js/bun/shell/file-io.test.ts → 25/25 pass
  • bun bd test test/js/bun/shell/bunshell.test.ts -t redirect → 28/28 pass
  • bun bd test test/regression/issue/12602.test.ts → 3/3 pass

Closes #12602

@coderabbitai

coderabbitai Bot commented Feb 20, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

A control-flow change in the Zig shell lexer prevents digits that are part of an in-progress word from being parsed as file-descriptor redirections. New regression tests validate parsing of command names with trailing digits versus standalone FD redirects.

Changes

Cohort / File(s) Summary
Shell Parsing Logic
src/shell/shell.zig
Added a check that an accumulating word is not in-progress (word_start != j) before attempting to parse a leading digit as an fd redirection, avoiding misinterpretation of digits inside words.
Regression Tests
test/regression/issue/12602.test.ts
Added three tests (two skipped on Windows) that verify commands like ./script1<input.txt keep the trailing digit in the command name, while constructs like cat 0<input.txt are treated as FD redirects.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR successfully addresses issue #12602 by fixing the tokenizer to check whether digits are part of an already-accumulating word before parsing as fd redirects, with comprehensive regression tests validating the fix.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the shell redirect parsing issue described in #12602; no unrelated modifications are present.
Title check ✅ Passed The title accurately describes the main change: preventing digits from being treated as fd-prefix redirects when they're part of an existing word, which directly addresses the core fix.
Description check ✅ Passed The description includes both required sections: detailed explanation of what the PR does with specific examples and repro steps, plus comprehensive verification with test results.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

@robobun

robobun commented Feb 20, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 8:53 PM PT - May 1st, 2026

❌ @robobun, your commit 58599ca has 3 failures in Build #49939 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 27257

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

bun-27257 --bun

@claude

claude Bot commented Feb 20, 2026 •

Copy link
Copy Markdown
Contributor

Newest first

✅ 0a2a7 — Looks good!

Reviewed 2 files across src/shell/ and test/regression/issue/: Fixes the shell tokenizer to correctly preserve trailing digits in command names (e.g., ./script1) when followed by redirect operators, preventing the digit from being incorrectly stripped and misinterpreted as a file descriptor redirect number.

Previous reviews

✅ 08afc — Looks good!

Reviewed 2 files across src/shell/ and test/regression/issue/: Fixes the shell tokenizer to correctly preserve trailing digits in command names when followed by redirect operators, preventing incorrect parsing where commands like ./script1 before < or > would have their trailing digit stripped and misinterpreted as a file descriptor redirect.

Previous reviews

✅ d9f72 — Looks good!

Reviewed 2 files across src/shell/ and test/regression/issue/: Fixes the shell tokenizer to correctly preserve trailing digits in command names when followed by redirect operators, preventing incorrect parsing like ./script1<file being misinterpreted as a file descriptor redirect.

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/12602.test.ts`:
- Around line 15-20: Capture and assert the exit code of the setup chmod command
and add one negative redirect case: assign the chmod invocation to a variable
(e.g., chmodRes = await $`chmod +x ${dir}/script1`.quiet()) and assert
chmodRes.exitCode === 0; keep the existing successful run (result = await $`cd
${dir} && ./script1<input.txt`.quiet()) and its output assertion, then add a
failing redirect test by running the command against a missing file with
.nothrow() (e.g., bad = await $`cd ${dir} && ./script1<missing.txt`.nothrow())
and assert bad.exitCode !== 0 to cover the error path.

Comment thread test/regression/issue/12602.test.ts
robobun added 2 commits May 1, 2026 18:54
The lexer treated any digit 0-2 followed by > or < as an fd-prefix
redirect, even when the digit was part of a larger word. This caused
`echo abc1>file` to lex as `echo abc` + `1>file` (writing "abc\n"
instead of "abc1\n"), and `echo test100>out` to lex as
`echo test10` + `0>out`.

POSIX only recognizes an IO number when the digits form a separate
word. Add a word-boundary check before entering the fd-redirect path:
the digit must not extend an in-progress text fragment (word_start==j)
and must not immediately follow a word-continuation token (Text, Var,
quoted text, CmdSubstEnd, etc.). Digits that begin a word, including
after operators/semicolons or at start of input, are still parsed as
fd prefixes.
@robobun
robobun force-pushed the claude/fix-shell-redirect-digit-stripping branch from 0a2a7ca to 2934757 Compare May 1, 2026 19:14
@robobun robobun changed the title fix(shell): don't strip trailing digits from command names before redirects shell: only treat digit as fd-prefix redirect at word boundary May 1, 2026
@robobun

robobun commented May 1, 2026 •

Copy link
Copy Markdown
Collaborator Author

Addressed review feedback in 7ac36fe + 58599ca:

  • Added .DoubleAsterisk to the compound-word guard so **2>file keeps the 2 in the glob word
  • Moved .DoubleAsterisk into the => true arm of break_word_impl so whitespace after ** emits a .Delimit (matching *) — fixes echo ** 2>file lexing as stderr redirect, and stops echo ** foo from gluing into one compound word
  • eat_redirect's '<' arm now consumes the < before checking for <<, so N<file no longer lexes with a spurious append=true and N<<file lexes as a single redirect token
  • Dropped the unnecessary Windows skip on the cat 0<input.txt regression test

All shell tests pass on every platform (lex 31/31, parse 17/17, 12602 3/3 on darwin and 1+2skip on Windows).

CI reds are unrelated flakes also hitting other open PRs: no-orphans.test.ts perl-setsid timeout on macOS (from #29930, also failing on #30089), bake/dev-and-prod.test.ts HMR race on one Windows shard (#28211/#29575), plus several darwin shards that expired waiting for runners.

Comment thread src/shell/shell.zig
Comment thread src/shell/shell.zig
…redirect

- Treat a digit following `**` as part of the compound word, same as `*`,
  so `**2>file` lexes as glob `**2` + `>file` instead of `**` + `2>file`.
- In eat_redirect's '<' arm, eat the '<' before checking for a second one so
  `N<file` no longer sets append=true and `N<<file` lexes as one token.
Comment thread src/shell/shell.zig
Comment thread test/regression/issue/12602.test.ts Outdated
…sterisk

Whitespace after `**` didn't emit a Delimit (unlike `*`), so adding
`.DoubleAsterisk` to the fd-prefix guard made `echo ** 2>file` glue the
`2` onto the glob word and parse `>` as a stdout redirect instead of
`2>` as stderr. Move `.DoubleAsterisk` into the `=> true` arm of
break_word_impl so it delimits like `.Asterisk` does — this also keeps
`echo ** foo` from gluing into a single compound word.

Also drop the unnecessary Windows skip on the `cat 0<input.txt` regression
test since it only uses shell builtins.
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.

Bun shell parses some redirections incorrectly

2 participants