diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 4fbe9c6944be..4f7529e34b29 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -237,6 +237,8 @@ pub struct VirtualMachine { // `transpiler.resolver.standalone_module_graph` without a downcast. pub standalone_module_graph: Option<&'static dyn bun_resolver::StandaloneModuleGraph>, pub smol: bool, + /// Node's `getOptionValue('--eval')` has a value for this VM. + pub has_eval_string: bool, // LAYERING: real type is `bun_runtime::dns_jsc::Order` (forward // dep); stored as its `u8` repr. pub dns_result_order: u8, @@ -2816,13 +2818,15 @@ extern crate alloc; /// casts back on the other side of each hook. pub type RuntimeState = *mut c_void; -/// Runtime flags a Worker's `execArgv` can set. `true` means allowed. +/// Runtime flags a Worker's `execArgv` can set. #[derive(Copy, Clone, Debug)] pub struct WorkerExecArgvFlags { /// `!--no-addons` pub allow_addons: bool, /// `!--no-ffi-cc` pub allow_ffi_cc: bool, + /// `--eval` has a value. See [`VirtualMachine::has_eval_string`]. + pub has_eval_string: bool, /// Where a flag is that is the process's, which a Worker cannot be given /// (`ERR_WORKER_INVALID_EXEC_ARGV`): `--disallow-code-generation-from-strings`. pub invalid: Option, @@ -4909,6 +4913,7 @@ impl VirtualMachine { // executable) resolve against the real filesystem and fail. vm_ref.transpiler.resolver.standalone_module_graph = opts.graph; vm_ref.hot_reload = worker.hot_reload(); + vm_ref.has_eval_string = worker.has_eval_string(); vm_ref.initial_script_execution_context_identifier = worker.execution_context_id() as i32; if opts.graph.is_none() { vm_ref.transpiler.configure_linker(); diff --git a/src/jsc/bindings/BunProcess.cpp b/src/jsc/bindings/BunProcess.cpp index 1d0155876744..9f648e1c0193 100644 --- a/src/jsc/bindings/BunProcess.cpp +++ b/src/jsc/bindings/BunProcess.cpp @@ -3279,17 +3279,6 @@ JSC_DEFINE_CUSTOM_SETTER(setProcessArgv, (JSGlobalObject * globalObject, Encoded return true; } -extern "C" EncodedJSValue Bun__Process__getExecArgv(JSGlobalObject* lexicalGlobalObject) -{ - auto* globalObject = defaultGlobalObject(lexicalGlobalObject); - auto* process = globalObject->processObject(); - if (!process) { - return JSValue::encode(jsUndefined()); - } - - return JSValue::encode(process->getExecArgv(globalObject)); -} - JSC_DEFINE_CUSTOM_GETTER(processExecArgv, (JSGlobalObject * globalObject, EncodedJSValue thisValue, PropertyName)) { Process* process = getProcessObject(globalObject, JSValue::decode(thisValue)); diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index d8b2f4e161bc..0c74a01d44bd 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -81,6 +81,8 @@ pub struct WebWorker { exec_argv_ptr: *const WTFStringImpl, exec_argv_len: usize, inherit_exec_argv: bool, + /// The worker VM's [`VirtualMachine::has_eval_string`]. + has_eval_string: bool, unresolved_specifier: Box<[u8]>, preloads: Vec>, name: bun_core::ZBox, @@ -352,6 +354,7 @@ impl WebWorker { // SAFETY: `parent` is the calling thread's live VM. let parent_ref = unsafe { &*parent }; let mut transform_options = (*parent_ref.transpiler.options.transform_options).clone(); + let mut has_eval_string = parent_ref.has_eval_string; if !inherit_exec_argv { let hooks = runtime_hooks().expect("RuntimeHooks not installed"); // SAFETY: caller passed valid (ptr,len) borrowed from C++ WorkerOptions; @@ -362,6 +365,7 @@ impl WebWorker { exec_argv_len, )) }; + has_eval_string = parsed.is_some_and(|flags| flags.has_eval_string); if let Some(flags) = parsed { if let Some(invalid) = flags.invalid { use bun_core::WTFStringImplExt as _; @@ -420,6 +424,7 @@ impl WebWorker { exec_argv_ptr, exec_argv_len, inherit_exec_argv, + has_eval_string, unresolved_specifier: spec_slice.slice().to_vec().into_boxed_slice(), preloads, name: if name_str.is_empty() { @@ -586,6 +591,11 @@ impl WebWorker { self.hot_reload } + #[inline] + pub(crate) fn has_eval_string(&self) -> bool { + self.has_eval_string + } + #[inline] pub(crate) fn arm_test_gate(&self) -> bool { self.arm_test_gate diff --git a/src/runtime/cli/Arguments.rs b/src/runtime/cli/Arguments.rs index d2a105468ec6..f4152049c597 100644 --- a/src/runtime/cli/Arguments.rs +++ b/src/runtime/cli/Arguments.rs @@ -781,6 +781,24 @@ pub(crate) use bun_bunfig::arguments::{load_config_path, load_config_with_cmd_ar /// the attached value `e`. Bun's `-p` takes the code, so `-pe X` is `-p X`. pub(crate) const NODE_SHORT_ALIASES: &[(&[u8], &[u8])] = &[(b"-pe", b"-p")]; +/// What the flag of `bun` takes that `is_it` says yes to. `None`: there is no such flag. +pub(crate) fn values_of(is_it: &dyn Fn(&clap::Names) -> bool) -> Option<&'static clap::Values> { + let mut flags = AUTO_PARAMS.iter(); + flags.find(|it| is_it(&it.names)).map(|it| &it.takes_value) +} + +/// Whether the flag `name`, written `--name` or `-n`, has its value in the argument after it. `None`: `bun` has no such flag. +pub(crate) fn takes_value(name: &[u8]) -> Option { + let alias = NODE_SHORT_ALIASES.iter().find(|(from, _)| *from == name); + let values = match alias.map_or(name, |(_, to)| *to) { + [b'-', b'-', long @ ..] => values_of(&|names| names.matches_long(long)), + [b'-', short] => values_of(&|names| names.short == Some(*short)), + _ => None, + }?; + // An optional value is only ever written `--name=value`. + Some(matches!(values, clap::Values::One | clap::Values::Many)) +} + /// `--disallow-code-generation-from-strings[=]`. Raises the process's level; never lowers it. fn disallow_code_generation_from_strings(value: &[u8]) { match bun_core::CodeGenerationFromStrings::from_flag_value(value) { diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index 5c1ec5c0497b..5e9097f03e1b 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -1086,6 +1086,10 @@ Full documentation is available at https://bun.com/docs/cli/run Some(Box::new(bun_ast::Source::init_path_string(entry, script))); vm.module_loader.interactive_eval_script = ctx.runtime_options.eval.interactive_script.take(); + let interactive = vm.module_loader.interactive_eval_script.as_deref(); + let has_code = interactive.is_none_or(|code| !code.is_empty()); + // `bun run -e ` runs the file, and `--interactive` can have no `-e` code. + vm.has_eval_string = entry.ends_with(EVAL_TRIGGER) && has_code; if ctx.runtime_options.eval.eval_and_print { vm.transpiler.options.dead_code_elimination = false; } diff --git a/src/runtime/cli/script_or_command.rs b/src/runtime/cli/script_or_command.rs index 3cd52f7b858b..fff7168bb1a6 100644 --- a/src/runtime/cli/script_or_command.rs +++ b/src/runtime/cli/script_or_command.rs @@ -6,6 +6,7 @@ use bun_core::{Global, UnwrapOrOom, env_var}; +use super::arguments::values_of; use super::check_command::working_directory; /// The kind of thing `bun ` runs. @@ -32,12 +33,6 @@ impl Unparsable { } } -/// What the flag of `bun` takes that `is_it` says yes to. `None`: there is no such flag. -fn values_of(is_it: &dyn Fn(&bun_clap::Names) -> bool) -> Option<&'static bun_clap::Values> { - let mut flags = super::arguments::AUTO_PARAMS.iter(); - flags.find(|it| is_it(&it.names)).map(|it| &it.takes_value) -} - /// Where the first of `shorts`, the letters of a chain like `-be`, is that takes a value: the rest of the chain, as in /// `-Fpattern`, or else the next argument. `None` also where the value is optional, which is never the next argument. What /// follows that letter in `-bc=bunfig.toml` is a value too, not more letters. diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 4a9da411e5f9..052ec2a0afb7 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1577,9 +1577,15 @@ unsafe fn apply_standalone_runtime_flags( unsafe fn parse_worker_exec_argv_flags( exec_argv: &[bun_core::WTFStringImpl], ) -> Option { + // SAFETY: per fn contract, a non-null entry is a live `WTFStringImpl*`. + let tokens = exec_argv + .iter() + .filter(|arg| !arg.is_null()) + .map(|&arg| unsafe { &*arg }); let mut flags = WorkerExecArgvFlags { allow_addons: true, allow_ffi_cc: true, + has_eval_string: exec_argv_has_eval_string(tokens), invalid: None, }; for (index, &arg) in exec_argv.iter().enumerate() { @@ -1609,6 +1615,71 @@ unsafe fn parse_worker_exec_argv_flags( Some(flags) } +/// Whether `--eval` has a value in `tokens`. The walk follows Node's option parser, with bun's CLI table for the flags that take a value. +fn exec_argv_has_eval_string<'a>( + tokens: impl Iterator, +) -> bool { + fn name_and_value(flag: &[u8]) -> (&[u8], Option<&[u8]>) { + match bun_core::strings::split_once_char(flag, b'=') { + Some((name, value)) => (name, Some(value)), + None => (flag, None), + } + } + let starts_with_dash = |token: &bun_core::WTFStringImplStruct| { + if token.is_8bit() { + token.latin1_slice().first() == Some(&b'-') + } else { + token.utf16_slice().first() == Some(&u16::from(b'-')) + } + }; + // Node's parser stops at a lone `-`, so it is not a flag. + let is_flag = + |token: &bun_core::WTFStringImplStruct| token.length() > 1 && starts_with_dash(token); + let mut tokens = tokens.peekable(); + let mut has_eval_string = false; + let mut value_may_follow = false; + while let Some(token) = tokens.next() { + if !is_flag(token) { + if !core::mem::take(&mut value_may_follow) { + break; + } + continue; + } + let flag = token.to_utf8(); + let flag: &[u8] = &flag; + if flag == b"--" { + break; + } + value_may_follow = false; + match name_and_value(flag) { + // `-e=` and `-p=` are bun's spellings. Node rejects them. + (b"--eval" | b"-e" | b"-p", Some(code)) => has_eval_string = !code.is_empty(), + // Node rejects a code that starts with `-`. Bun's CLI takes it, so it is the code here too. + (b"--eval" | b"-e" | b"-pe", None) => { + has_eval_string = tokens.next().is_some_and(|code| code.length() != 0); + } + // As in Node, `--print=` is `--print`, and a print flag only prints before `-`, `--` or an option. Bun's CLI takes a token like `-1` as the code. + (b"--print", _) | (b"-p", None) => { + let is_code = |next: &bun_core::WTFStringImplStruct| { + if !starts_with_dash(next) { + return next.length() != 0; + } + let next = next.to_utf8(); + let (name, _) = name_and_value(&next); + !matches!(name, b"-" | b"--") + && crate::cli::arguments::takes_value(name).is_none() + }; + has_eval_string |= tokens.next_if(|next| is_code(next)).is_some(); + } + (_, Some(_)) => {} + (name, None) => { + value_may_follow = crate::cli::arguments::takes_value(name) != Some(false); + } + } + } + has_eval_string +} + /// `jsc.API.cron.CronJob.clearAllForVM(vm, .teardown)` — /// stops every in-process `Bun.cron()` job registered on /// this VM and releases the pending-promise ref so the struct frees (the event diff --git a/src/runtime/node/node_process.rs b/src/runtime/node/node_process.rs index 219c4afbf37d..3a68bc1a8b95 100644 --- a/src/runtime/node/node_process.rs +++ b/src/runtime/node/node_process.rs @@ -7,11 +7,9 @@ use bun_core::env_var; use bun_core::{self, Environment, Global}; use bun_jsc::{EncodedSliceJsc as _, JSGlobalObject, JSValue, JsResult}; -// Both materialize the array on first access through `Bun__Process__createArgv` -// / `createExecArgv` below, which return zero with the exception pending. +// Filled on first access by `Bun__Process__createArgv` below. Zero means a pending exception. unsafe extern "C" { safe fn Bun__Process__getArgv(global: &JSGlobalObject) -> JSValue; - safe fn Bun__Process__getExecArgv(global: &JSGlobalObject) -> JSValue; } // ───────────────────────────── argv0 / execPath ───────────────────────────── @@ -58,10 +56,6 @@ pub(crate) fn get_argv(global: &JSGlobalObject) -> JsResult { bun_jsc::call_zero_is_throw(global, || Bun__Process__getArgv(global)) } -pub(crate) fn get_exec_argv(global: &JSGlobalObject) -> JsResult { - bun_jsc::call_zero_is_throw(global, || Bun__Process__getExecArgv(global)) -} - // ───────────────────────────── exit ───────────────────────────── // @190n: this may need to be noreturn @@ -312,43 +306,10 @@ mod _impl { // 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() { - // An optional value is only ever written `--name=value`. - if param.takes_value != bun_clap::Values::None - && param.takes_value != bun_clap::Values::OneOptional - { - 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; - } + if prev.is_some_and(|prev| crate::cli::arguments::takes_value(prev) == Some(true)) { + args.push(BunString::clone_utf8(arg)); + prev = Some(arg); + continue; } // we hit the script name diff --git a/src/runtime/node/util/parse_args.rs b/src/runtime/node/util/parse_args.rs index 227b2a918319..075427917ced 100644 --- a/src/runtime/node/util/parse_args.rs +++ b/src/runtime/node/util/parse_args.rs @@ -184,33 +184,19 @@ fn find_option_by_long_name(long_name: &String, options: &[OptionDefinition]) -> /// Gets the default args from the process argv fn get_default_args(global: &JSGlobalObject) -> JsResult { - // Work out where to slice process.argv for user supplied arguments - - let exec_argv = super::process::get_exec_argv(global)?; let argv = super::process::get_argv(global)?; - if argv.is_array() && exec_argv.is_array() { + if argv.is_array() { let end = u32::try_from(argv.get_length(global)?).expect("int cast"); - let mut iter = exec_argv.array_iterator(global)?; - while let Some(item) = iter.next()? { - if item.is_string() { - let str = item.to_bun_string(global)?; - if str.eq_ascii(b"-e") - || str.eq_ascii(b"--eval") - || str.eq_ascii(b"-p") - || str.eq_ascii(b"--print") - { - return Ok(ArgsSlice { - array: argv, - start: end.min(1), - end, - }); - } - } - } + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/util/parse_args/parse_args.js#L58-L67 + let start = if global.bun_vm().has_eval_string { + 1 + } else { + 2 + }; // `process.argv` can have fewer than 2 items. Clamp the start like `ArrayPrototypeSlice` does. return Ok(ArgsSlice { array: argv, - start: end.min(2), + start: end.min(start), end, }); } diff --git a/test/expected-durations.json b/test/expected-durations.json index c75f1bd0293b..76aa1c4971ef 100644 --- a/test/expected-durations.json +++ b/test/expected-durations.json @@ -34981,13 +34981,6 @@ "windows": 150, "windows-aarch64": 140 }, - "js/node/util/parse_args/default-args.test.mjs": { - "default": 200, - "asan": 1590, - "musl": 150, - "windows": 200, - "windows-aarch64": 330 - }, "js/node/util/parse_args/parse-args-null-config.test.ts": { "default": 40, "asan": 130, diff --git a/test/js/node/test/fixtures/parse-args.js b/test/js/node/test/fixtures/parse-args.js new file mode 100644 index 000000000000..104cabbf679e --- /dev/null +++ b/test/js/node/test/fixtures/parse-args.js @@ -0,0 +1,5 @@ +const { parseArgs } = require('util'); + +const parsedArgs = parseArgs({ strict: false }).values; + +process.stdout.write(JSON.stringify(parsedArgs)); diff --git a/test/js/node/test/parallel/test-parse-args.mjs b/test/js/node/test/parallel/test-parse-args.mjs index e79434bdc6bb..c5fffcf82a50 100644 --- a/test/js/node/test/parallel/test-parse-args.mjs +++ b/test/js/node/test/parallel/test-parse-args.mjs @@ -1,6 +1,7 @@ -import '../common/index.mjs'; +import { spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; import assert from 'node:assert'; -import { test } from 'node:test'; +import { suite, test } from 'node:test'; import { parseArgs } from 'node:util'; test('when short option used as flag then stored as flag', () => { @@ -211,73 +212,40 @@ test('order of option and positional does not matter (per README)', () => { ); }); -test('correct default args when use node -p', () => { - const holdArgv = process.argv; - process.argv = [process.argv0, '--foo']; - const holdExecArgv = process.execArgv; - process.execArgv = ['-p', '0']; - const result = parseArgs({ strict: false }); - - const expected = { values: { __proto__: null, foo: true }, - positionals: [] }; - assert.deepStrictEqual(result, expected); - process.argv = holdArgv; - process.execArgv = holdExecArgv; -}); - -test('correct default args when use node --print', () => { - const holdArgv = process.argv; - process.argv = [process.argv0, '--foo']; - const holdExecArgv = process.execArgv; - process.execArgv = ['--print', '0']; - const result = parseArgs({ strict: false }); - - const expected = { values: { __proto__: null, foo: true }, - positionals: [] }; - assert.deepStrictEqual(result, expected); - process.argv = holdArgv; - process.execArgv = holdExecArgv; -}); - -test('correct default args when use node -e', () => { - const holdArgv = process.argv; - process.argv = [process.argv0, '--foo']; - const holdExecArgv = process.execArgv; - process.execArgv = ['-e', '0']; - const result = parseArgs({ strict: false }); - - const expected = { values: { __proto__: null, foo: true }, - positionals: [] }; - assert.deepStrictEqual(result, expected); - process.argv = holdArgv; - process.execArgv = holdExecArgv; -}); - -test('correct default args when use node --eval', () => { - const holdArgv = process.argv; - process.argv = [process.argv0, '--foo']; - const holdExecArgv = process.execArgv; - process.execArgv = ['--eval', '0']; - const result = parseArgs({ strict: false }); - const expected = { values: { __proto__: null, foo: true }, - positionals: [] }; - assert.deepStrictEqual(result, expected); - process.argv = holdArgv; - process.execArgv = holdExecArgv; -}); - -test('correct default args when normal arguments', () => { - const holdArgv = process.argv; - process.argv = [process.argv0, 'script.js', '--foo']; - const holdExecArgv = process.execArgv; - process.execArgv = []; - const result = parseArgs({ strict: false }); +suite('correct default args', () => { + const expected = { code: 0, signal: null, stderr: '', stdout: { foo: true, bar: true } }; + + suite('with CLI flags', () => { + const evalCode = "JSON.stringify(require('util').parseArgs({ strict: false }).values)"; + const evalCodePrinted = `process.stdout.write(${evalCode})`; + const execArgsTests = { + '-e