diff --git a/src/runtime/node/node_process.rs b/src/runtime/node/node_process.rs index d021b241ea51..b351cf049f5e 100644 --- a/src/runtime/node/node_process.rs +++ b/src/runtime/node/node_process.rs @@ -315,67 +315,68 @@ mod _impl { }, ); + // OneOptional params (-c, --inspect) never take the next token, so they are left out. + static CONSUMES_NEXT_ARG: std::sync::LazyLock = + std::sync::LazyLock::new(|| { + let mut set = bun_collections::StringSet::new(); + for param in crate::cli::arguments::AUTO_PARAMS.iter() { + if matches!( + param.takes_value, + bun_clap::Values::One | bun_clap::Values::Many + ) { + if let Some(name) = param.names.long { + let mut k = Vec::with_capacity(2 + name.len()); + k.extend_from_slice(b"--"); + k.extend_from_slice(name); + bun_core::handle_oom(set.insert(&k)); + } + if let Some(name) = param.names.short { + bun_core::handle_oom(set.insert(&[b'-', name])); + } + } + } + // Aliases are not params; one takes a value iff its target does. + for (from, to) in crate::cli::arguments::NODE_SHORT_ALIASES { + if set.contains(to) { + bun_core::handle_oom(set.insert(from)); + } + } + set + }); + let mut seen_run = false; - let mut prev: Option<&[u8]> = None; + let mut awaiting_value = false; // we re-parse the process argv to extract execArgv, since this is a very uncommon operation // it isn't worth doing this as a part of the CLI let mut iter = argv.iter(); let _ = iter.next(); // skip argv[0] for arg in iter { - // emulate `defer prev = arg` by setting at end of each iteration body let arg: &[u8] = arg; + // The previous option's value, whatever it looks like (`--conditions -`). + if awaiting_value { + args.push(BunString::clone_utf8(arg)); + awaiting_value = false; + continue; + } + + // `-` (the stdin script) and `--` are positionals, so they end execArgv too. + if arg == b"-" || arg == b"--" { + break; + } + if arg.len() >= 1 && arg[0] == b'-' { args.push(BunString::clone_utf8(arg)); - prev = Some(arg); + awaiting_value = CONSUMES_NEXT_ARG.contains(arg); continue; } if !seen_run && arg == b"run" { seen_run = true; - prev = Some(arg); continue; } - // A set of execArgv args consume an extra argument, so we do not want to - // confuse these with script names. - // Build the set lazily at runtime from the `AUTO_PARAMS` table: - // `--long` / `-s` for every param with a value. - static MAP: std::sync::LazyLock = - std::sync::LazyLock::new(|| { - let mut set = bun_collections::StringSet::new(); - for param in crate::cli::arguments::AUTO_PARAMS.iter() { - if param.takes_value != bun_clap::Values::None { - if let Some(name) = param.names.long { - let mut k = Vec::with_capacity(2 + name.len()); - k.extend_from_slice(b"--"); - k.extend_from_slice(name); - bun_core::handle_oom(set.insert(&k)); - } - if let Some(name) = param.names.short { - bun_core::handle_oom(set.insert(&[b'-', name])); - } - } - } - // Node's whole-token aliases are not params, so they never - // land above; an alias takes a value iff its target does. - for (from, to) in crate::cli::arguments::NODE_SHORT_ALIASES { - if set.contains(to) { - bun_core::handle_oom(set.insert(from)); - } - } - set - }); - - if let Some(p) = prev { - if MAP.contains(p) { - args.push(BunString::clone_utf8(arg)); - prev = Some(arg); - continue; - } - } - // we hit the script name break; } diff --git a/test/js/node/child_process/child_process.test.ts b/test/js/node/child_process/child_process.test.ts index a7bf59ba32ac..d93a3bd7038f 100644 --- a/test/js/node/child_process/child_process.test.ts +++ b/test/js/node/child_process/child_process.test.ts @@ -1,7 +1,18 @@ import { semver, write } from "bun"; import { afterAll, beforeEach, describe, expect, it } from "bun:test"; import fs from "fs"; -import { bunEnv, bunExe, isLinux, isPosix, isWindows, nodeExe, runBunInstall, shellExe, tmpdirSync } from "harness"; +import { + bunEnv, + bunExe, + isLinux, + isPosix, + isWindows, + nodeExe, + runBunInstall, + shellExe, + tempDir, + tmpdirSync, +} from "harness"; import { ChildProcess, exec, execFile, execFileSync, execSync, fork, spawn, spawnSync } from "node:child_process"; import { getEventListeners, once, setMaxListeners } from "node:events"; import { promisify } from "node:util"; @@ -194,6 +205,32 @@ describe("fork() IPC", () => { }); expect(exitCode).toBe(0); }); + + // fork() launches the child with process.execArgv in front of the module path. For a parent + // started as `bun run -`, execArgv used to contain that "-", so the child ran stdin instead. + it("works from a parent script piped into `bun run -`", async () => { + using dir = tempDir("fork-from-stdin", { + "child.js": `console.log("child " + JSON.stringify(process.argv.slice(2)));`, + }); + const parent = ` + const child = require("child_process").fork("./child.js", ["x"], { stdio: ["ignore", "inherit", "inherit", "ipc"] }); + child.on("exit", code => console.log("child exit " + code)); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "-", "parent-arg"], + cwd: String(dir), + env: bunEnv, + stdin: Buffer.from(parent), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.split("\n").filter(Boolean), stderr }).toEqual({ + stdout: ['child ["x"]', "child exit 0"], + stderr: "", + }); + expect(exitCode).toBe(0); + }); }); describe("spawn()", () => { diff --git a/test/js/node/process/process.test.js b/test/js/node/process/process.test.js index b2e793ed3f77..b4d17c9c7174 100644 --- a/test/js/node/process/process.test.js +++ b/test/js/node/process/process.test.js @@ -1542,17 +1542,39 @@ it("process.hasUncaughtExceptionCaptureCallback", () => { }); it("process.execArgv", async () => { + const script = join(__dirname, "print-process-execArgv.js"); + // Every command below also gets `script` on stdin, so a bare `-` in the script + // position runs the same fixture from stdin. `argv` is process.argv.slice(2). const fixtures = [ ["index.ts --bun -a -b -c", [], ["--bun", "-a", "-b", "-c"]], ["--bun index.ts index.ts", ["--bun"], ["index.ts"]], ["run -e bruh -b index.ts foo -a -b -c", ["-e", "bruh", "-b"], ["foo", "-a", "-b", "-c"]], + // `-` is the stdin script, not an exec arg, and nothing after it is either. + ["run - a b", [], ["a", "b"]], + ["--smol run - -x --foo", ["--smol"], ["-x", "--foo"]], + ["run --smol - a", ["--smol"], ["a"]], + ["--smol - foo", ["--smol"], ["foo"]], + // `--` ends the exec args (node drops it from execArgv too). + ["--smol -- index.ts a", ["--smol"], ["a"]], + ["run --smol -- - a", ["--smol"], ["a"]], + ["--bun node --no-warnings -- index.ts a", ["--no-warnings"], ["a"]], + // ...unless they are the value of an option that takes one (and so is a value spelled `run`). + ["--conditions - index.ts", ["--conditions", "-"], []], + ["--conditions -- index.ts", ["--conditions", "--"], []], + ["--conditions run index.ts", ["--conditions", "run"], []], + // `-c`/`--config` only take a value as `--config=path`, so the next arg is the script. + ["-c index.ts a", ["-c"], ["a"]], ]; - for (const [cmd, execArgv, argv] of fixtures) { - const replacedCmd = cmd.replace("index.ts", Bun.$.escape(join(__dirname, "print-process-execArgv.js"))); - const result = await Bun.$`${bunExe()} ${{ raw: replacedCmd }}`.json(); - expect(result, `bun ${cmd}`).toEqual({ execArgv, argv }); - } + const results = await Promise.all( + fixtures.map(async ([cmd]) => { + const replacedCmd = cmd.replace("index.ts", Bun.$.escape(script)); + return [cmd, await Bun.$`${bunExe()} ${{ raw: replacedCmd }} < ${script}`.json()]; + }), + ); + expect(Object.fromEntries(results)).toEqual( + Object.fromEntries(fixtures.map(([cmd, execArgv, argv]) => [cmd, { execArgv, argv }])), + ); }); describe("process.exitCode", () => {