diff --git a/src/runtime/shell/Builtin.rs b/src/runtime/shell/Builtin.rs index 9be291a1f006..320725e32d2b 100644 --- a/src/runtime/shell/Builtin.rs +++ b/src/runtime/shell/Builtin.rs @@ -799,8 +799,8 @@ impl Builtin { } } None if redirect.duplicate_out() => { - // `2>&1` (stderr=true,dup_out=true) → stderr := stdout - // `1>&2` (stdout=true,dup_out=true) → stdout := stderr + // `2>&1` (stdout=true,dup_out=true) → stderr := stdout + // `1>&2` (stderr=true,dup_out=true) → stdout := stderr let me = Self::of_mut(interp, cmd); if redirect.stdout() { me.stderr = me.stdout.dup_ref(); diff --git a/src/shell_parser/parse.rs b/src/shell_parser/parse.rs index 40d6f5bd000c..eb8062c06b8c 100644 --- a/src/shell_parser/parse.rs +++ b/src/shell_parser/parse.rs @@ -586,8 +586,8 @@ pub mod ast { const STDOUT = 1 << 1; const STDERR = 1 << 2; const APPEND = 1 << 3; - /// 1>&2 === stdout=true and duplicate_out=true - /// 2>&1 === stderr=true and duplicate_out=true + /// 2>&1 === stdout=true and duplicate_out=true + /// 1>&2 === stderr=true and duplicate_out=true const DUPLICATE_OUT = 1 << 4; } } @@ -1571,10 +1571,26 @@ impl<'bump> Parser<'bump> { let mut name_and_args = bun_alloc::ArenaVec::new_in(self.alloc); name_and_args.push(name); - while let Some(arg) = self.parse_atom()? { - name_and_args.push(arg); + let mut parsed_redirect = ParsedRedirect::default(); + let mut has_redirect = false; + loop { + if let Some(arg) = self.parse_atom()? { + name_and_args.push(arg); + continue; + } + if self.check(TokenTag::Redirect) { + if has_redirect { + self.add_error(format_args!( + "Multiple redirects are not supported yet. Please open a GitHub issue." + ))?; + return Err(ParseError::Unsupported.into()); + } + parsed_redirect = self.parse_redirect()?; + has_redirect = true; + continue; + } + break; } - let parsed_redirect = self.parse_redirect()?; Ok(ast::CmdOrAssigns::Cmd(ast::Cmd { assigns: assigns.into_bump_slice(), @@ -1596,6 +1612,11 @@ impl<'bump> Parser<'bump> { }; let redirect_file: Option> = 'redirect_file: { if has_redirect { + // `2>&1` / `1>&2` are complete on their own; the next word is an + // argument of the command, not a file operand. + if redirect.duplicate_out() { + break 'redirect_file None; + } if self.r#match(TokenTag::JSObjRef) { let Token::JSObjRef(obj_ref) = self.prev() else { unreachable!() @@ -1606,9 +1627,6 @@ impl<'bump> Parser<'bump> { let file = match self.parse_atom()? { Some(f) => f, None => { - if redirect.duplicate_out() { - break 'redirect_file None; - } self.add_error(format_args!("Redirection with no file"))?; return Err(ParseError::Expected.into()); } diff --git a/test/js/bun/shell/file-io.test.ts b/test/js/bun/shell/file-io.test.ts index 5b10f0351a8e..2312435ce792 100644 --- a/test/js/bun/shell/file-io.test.ts +++ b/test/js/bun/shell/file-io.test.ts @@ -23,6 +23,25 @@ describe("IOWriter file output redirection", () => { .runAsTest("zero-length write should trigger onIOWriterChunk callback"); }); + describe("fd-dup redirect followed by a word", () => { + // `2>&1` has no file operand, so `b` must stay an argument of echo. + TestBuilder.command`echo a 2>&1 b` + .ensureTempDir() + .exitCode(0) + .stdout("a b\n") + .stderr("") + .doesNotExist("b") + .runAsTest("2>&1 does not consume the next word as a file"); + + TestBuilder.command`echo a 1>&2 b` + .ensureTempDir() + .exitCode(0) + .stdout("") + .stderr("a b\n") + .doesNotExist("b") + .runAsTest("1>&2 does not consume the next word as a file"); + }); + describe("drainBufferedData edge cases", () => { TestBuilder.command`echo -n ${"x".repeat(1024 * 10)} > large.txt` .exitCode(0) diff --git a/test/js/bun/shell/parse.test.ts b/test/js/bun/shell/parse.test.ts index 2da7a674bc12..9ce256e832b3 100644 --- a/test/js/bun/shell/parse.test.ts +++ b/test/js/bun/shell/parse.test.ts @@ -59,6 +59,31 @@ describe("parse shell", () => { expect(result).toEqual(expected); }); + test("fd-dup redirect does not consume the next word", () => { + // `2>&1` / `1>&2` are complete redirects; the following word is an + // argument of the command (bash: `echo a 2>&1 b` prints "a b"). + const cmd = { + assigns: [], + name_and_args: [{ simple: { Text: "echo" } }, { simple: { Text: "a" } }, { simple: { Text: "b" } }], + redirect: redirect({ stdout: true, duplicate_out: true }), + redirect_file: null, + }; + expect(JSON.parse(parse`echo a 2>&1 b`)).toEqual({ stmts: [{ exprs: [{ cmd }] }] }); + + const cmd2 = { + assigns: [], + name_and_args: [{ simple: { Text: "echo" } }, { simple: { Text: "a" } }, { simple: { Text: "b" } }], + redirect: redirect({ stderr: true, duplicate_out: true }), + redirect_file: null, + }; + expect(JSON.parse(parse`echo a 1>&2 b`)).toEqual({ stmts: [{ exprs: [{ cmd: cmd2 }] }] }); + + // With the trailing word absent the redirect still stands on its own. + expect(JSON.parse(parse`echo a 2>&1`)).toEqual({ + stmts: [{ exprs: [{ cmd: { ...cmd, name_and_args: cmd.name_and_args.slice(0, 2) } }] }], + }); + }); + test("single atom", () => { expect(JSON.parse(parse`ls`)).toEqual({ stmts: [ @@ -1066,5 +1091,9 @@ describe("parse shell invalid input", () => { await TestBuilder.command`echo (echo foo && echo hi)`.error("Unexpected token: `(`").run(); await TestBuilder.command`echo foo >`.error("Redirection with no file").run(); + + await TestBuilder.command`echo a 2>&1 b > f` + .error("Multiple redirects are not supported yet. Please open a GitHub issue.") + .run(); }); });