diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 36c22bde2be3..5bf696bfcdf7 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -1,9 +1,10 @@ -use crate::shell::EnvStr; use crate::shell::builtin::{Builtin, BuiltinState, IoKind}; use crate::shell::interpreter::{Interpreter, NodeId}; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; +use crate::shell::{EnvStr, ExitCode}; use bun_collections::index_sort; +use bun_shell_parser::is_valid_var_name; #[derive(Default)] pub(crate) struct Export { @@ -15,24 +16,45 @@ enum State { #[default] Idle, WaitingIo, + Err, Done, } impl Export { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let argc = Builtin::of(interp, cmd).args_slice().len(); - if argc == 0 { - // No args: print all exported vars. - return Self::print_all(interp, cmd); - } + let mut has_operand = false; + let mut errors = Vec::new(); for i in 0..argc { let s = Builtin::of(interp, cmd).arg_bytes(i); + if i == 0 && s.len() > 1 && s[0] == b'-' { + if s == b"--" { + continue; + } + // No option is supported. `export -n NAME` must not reach NAME. + let end = bun_core::strings::index_of_char_usize(s, b'=').unwrap_or(s.len()); + let msg = [b"export: ".as_slice(), &s[..end], b": invalid option\n"].concat(); + return Self::fail(interp, cmd, &msg); + } + has_operand = true; if s.is_empty() { continue; } let (name, value) = match bun_core::strings::index_of_char_usize(s, b'=') { - Some(eq) => (&s[..eq], &s[eq + 1..]), - None => (s, &b""[..]), + Some(eq) if eq > 0 => (&s[..eq], &s[eq + 1..]), + None if is_valid_var_name(s) => { + // A bare NAME stores an empty value (#45371). Do not do that after a refused word. + if !errors.is_empty() { + continue; + } + (s, &b""[..]) + } + refused => { + errors.extend_from_slice(b"export: `"); + errors.extend_from_slice(if refused.is_some() { b"=" } else { s }); + errors.extend_from_slice(b"`: not a valid identifier\n"); + continue; + } }; // The argv backing is freed when the Cmd retires, // so the key/value MUST be duplicated into ref-counted storage — @@ -48,7 +70,20 @@ impl Export { label.deref(); val.deref(); } - Builtin::done(interp, cmd, 0) + if !has_operand { + // No operand: print all exported vars. + return Self::print_all(interp, cmd); + } + if errors.is_empty() { + return Builtin::done(interp, cmd, 0); + } + Self::fail(interp, cmd, &errors) + } + + /// Write `msg` to stderr, then finish with exit code 1. + fn fail(interp: &Interpreter, cmd: NodeId, msg: &[u8]) -> Yield { + Self::state_mut(interp, cmd).state = State::Err; + Builtin::write_failing_error(interp, cmd, msg, 1) } fn print_all(interp: &Interpreter, cmd: NodeId) -> Yield { @@ -84,7 +119,15 @@ impl Export { _: usize, err: Option, ) -> Yield { + let failed = matches!(Self::state_mut(interp, cmd).state, State::Err); Self::state_mut(interp, cmd).state = State::Done; - Builtin::done(interp, cmd, err.map_or(0, |_| 1)) + Builtin::done( + interp, + cmd, + match err { + Some(_err) => 1, + None => ExitCode::from(failed), + }, + ) } } diff --git a/src/shell_parser/lib.rs b/src/shell_parser/lib.rs index 5034c759041a..163dcca56573 100644 --- a/src/shell_parser/lib.rs +++ b/src/shell_parser/lib.rs @@ -17,6 +17,6 @@ pub mod parse; pub mod json_fmt; pub use parse::{ - LexResult, LexerError, ParseError, Parser, ast, escape_8bit, escape_bun_str, + LexResult, LexerError, ParseError, Parser, ast, escape_8bit, escape_bun_str, is_valid_var_name, needs_escape_bunstr, needs_escape_utf8_ascii_latin1, }; diff --git a/src/shell_parser/parse.rs b/src/shell_parser/parse.rs index 534aba820824..34ec015ba7ea 100644 --- a/src/shell_parser/parse.rs +++ b/src/shell_parser/parse.rs @@ -3776,7 +3776,7 @@ impl<'a, const ENCODING: StringEncoding> ShellCharIter<'a, ENCODING> { /// - a-zA-Z /// - _ /// - 0-9 (but can't be first char) -pub(crate) fn is_valid_var_name(var_name: &[u8]) -> bool { +pub fn is_valid_var_name(var_name: &[u8]) -> bool { if is_all_ascii(var_name) { return is_valid_var_name_ascii(var_name); } diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index f2831d30d204..99c77225653c 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1668,6 +1668,158 @@ describe("deno_task", () => { TestBuilder.command`export $(echo "SPLIT=hello world") && echo "SPLIT=[$SPLIT]"` .stdout("SPLIT=[hello]\n") .runAsTest("export splits a non-assignment word"); + + describe("export refuses a name that is not an identifier", () => { + const notValid = (name: string) => `export: \`${name}\`: not a valid identifier\n`; + const invalidOption = (word: string) => `export: ${word}: invalid option\n`; + const env = { A: "one" }; + + // [script, stderr, exit code, the exported variables after the script] + const rows: [string, string, number, Record][] = [ + ["export a-b", notValid("a-b"), 1, { A: "one" }], + ["export 1a", notValid("1a"), 1, { A: "one" }], + ['export "a b"', notValid("a b"), 1, { A: "one" }], + ["export a.b", notValid("a.b"), 1, { A: "one" }], + ["export é", notValid("é"), 1, { A: "one" }], + ["export aé", notValid("aé"), 1, { A: "one" }], + ["export -", notValid("-"), 1, { A: "one" }], + ["X=a-b; export $X", notValid("a-b"), 1, { A: "one" }], + ["export $(echo a-b)", notValid("a-b"), 1, { A: "one" }], + ["export b{-c,=1}", notValid("b-c"), 1, { A: "one", b: "1" }], + ['"export" a-b', notValid("a-b"), 1, { A: "one" }], + ["E=export; $E a-b", notValid("a-b"), 1, { A: "one" }], + ["(export a-b)", notValid("a-b"), 1, { A: "one" }], + ["export a-b | cat", notValid("a-b"), 0, { A: "one" }], + ["export a-b 2> /dev/null", "", 1, { A: "one" }], + + // An empty name is refused. The message does not have the value. + ["export =x", notValid("="), 1, { A: "one" }], + ["export $UNSET=1", notValid("="), 1, { A: "one" }], + ['export =$(echo "x B=2")', notValid("="), 1, { A: "one", B: "2" }], + + // A refused word does not stop the `NAME=value` operands around it. + ["export B=1 a-b D=2", notValid("a-b"), 1, { A: "one", B: "1", D: "2" }], + ["export a-b A=", notValid("a-b"), 1, { A: "" }], + + // A bare NAME stores an empty value. After a refused word it is left alone. + ["export a-b A", notValid("a-b"), 1, { A: "one" }], + ["export B=1 -n A", notValid("-n"), 1, { A: "one", B: "1" }], + ["X=5; export a-b X", notValid("a-b"), 1, { A: "one" }], + // Before a refused word it is stored, as it is without one. + ["export A a-b", notValid("a-b"), 1, { A: "" }], + + // `export` has no options. A leading option refuses the whole command. + ["export -x", invalidOption("-x"), 1, { A: "one" }], + ["export -n A", invalidOption("-n"), 1, { A: "one" }], + ["export -x B=1", invalidOption("-x"), 1, { A: "one" }], + ["export -X=1", invalidOption("-X"), 1, { A: "one" }], + ["export -- B=1", "", 0, { A: "one", B: "1" }], + ["export -- -n A", notValid("-n"), 1, { A: "one" }], + + // Valid input. The name of a `NAME=value` operand is not checked, only that it is not empty. + ["export _b=1 B2=2", "", 0, { A: "one", _b: "1", B2: "2" }], + ["export B= C=a=b", "", 0, { A: "one", B: "", C: "a=b" }], + ["export a-b=1 a.b=c=d", "", 0, { A: "one", "a-b": "1", "a.b": "c=d" }], + ['export "" B=1', "", 0, { A: "one", B: "1" }], + ['export ""', "", 0, { A: "one" }], + ['export -- ""', "", 0, { A: "one" }], + ]; + + test.each(rows)("%s", async (script, stderr, exitCode, exported) => { + // Without `.quiet()` stderr is a file descriptor: the builtin finishes + // in the callback of the write. With `.quiet()` stderr is a buffer. + for (const quiet of [false, true]) { + const shell = $`${{ raw: script }}`.env(env).nothrow(); + const result = await (quiet ? shell.quiet() : shell); + expect({ + stdout: result.stdout.toString(), + stderr: result.stderr.toString(), + exitCode: result.exitCode, + }).toEqual({ stdout: "", stderr, exitCode }); + } + + // `export` with no operand prints each exported variable as `NAME=value`. + // PWD is in the list because this file calls `$.cwd()`. + const { stdout } = await $`${{ raw: script }}; export`.env(env).nothrow().quiet(); + const { PWD, ...listed } = Object.fromEntries( + stdout + .toString() + .split("\n") + .filter(line => line.length > 0) + .map(line => [line.slice(0, line.indexOf("=")), line.slice(line.indexOf("=") + 1)]), + ); + expect(listed).toEqual(exported); + }); + + test("a child process does not get a refused name", async () => { + const printEnv = `console.log(JSON.stringify(["a-b", "a b", "-n", "A", "D"].map(name => [name, process.env[name]])))`; + const { stdout, stderr } = await $`export a-b D=1 ${"a b"}; export -n A; ${BUN} -e ${printEnv}` + .env({ ...bunEnv, A: "one" }) + .nothrow() + .quiet(); + expect(stderr.toString()).toBe(notValid("a-b") + notValid("a b") + invalidOption("-n")); + expect(JSON.parse(stdout.toString())).toEqual([ + ["a-b", null], + ["a b", null], + ["-n", null], + ["A", "one"], + ["D", "1"], + ]); + }); + + test("the message follows a redirect of stderr", async () => { + using dir = tempDir("export-stderr", {}); + const toStdout = await $`export a-b 2>&1`.env(env).nothrow().quiet(); + expect({ + stdout: toStdout.stdout.toString(), + stderr: toStdout.stderr.toString(), + exitCode: toStdout.exitCode, + }).toEqual({ stdout: notValid("a-b"), stderr: "", exitCode: 1 }); + + const substituted = await $`echo "$(export a-b 2>&1)"`.env(env).nothrow().quiet(); + expect(substituted.stdout.toString()).toBe(notValid("a-b")); + + const toFile = await $`export a-b 2> err.txt`.env(env).cwd(String(dir)).nothrow().quiet(); + expect(await Bun.file(join(String(dir), "err.txt")).text()).toBe(notValid("a-b")); + expect(toFile.exitCode).toBe(1); + }); + + test("an empty word from a JavaScript value is skipped", async () => { + const { stdout, stderr } = await $`export ${""} B=1 && echo $B`.env(env).quiet(); + expect({ stdout: stdout.toString(), stderr: stderr.toString() }).toEqual({ stdout: "1\n", stderr: "" }); + }); + + test("`export --` prints the exported variables", async () => { + const { stdout, stderr } = await $`export --`.env(env).quiet(); + expect(stdout.toString()).toStartWith("A=one\n"); + expect(stderr.toString()).toBe(""); + }); + + TestBuilder.command`export 1abc` + .stderr(notValid("1abc")) + .exitCode(1) + .testMini({ onlyMini: true }) + .runAsTest("in a script that `bun run` executes"); + + TestBuilder.command`export A=1 2B C=3 3D || echo "failed A=$A C=$C"` + .stdout("failed A=1 C=3\n") + .stderr(notValid("2B") + notValid("3D")) + .runAsTest("the next command of an || list runs"); + + TestBuilder.command`export NODE_ENV = test && echo ran` + .stderr(notValid("=")) + .exitCode(1) + .runAsTest("spaces around = stop an && list"); + + TestBuilder.command`B=2 export -x B; echo "[$B]"` + .stdout("[]\n") + .stderr(invalidOption("-x")) + .runAsTest("a refused option does not apply a prefix assignment"); + + TestBuilder.command`export _ok OK2=1 && echo done` + .stdout("done\n") + .runAsTest("a valid bare name and a valid assignment exit 0"); + }); }); describe("pipeline", async () => {