Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<usize>,
Expand Down Expand Up @@ -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();
Expand Down
11 changes: 0 additions & 11 deletions src/jsc/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
10 changes: 10 additions & 0 deletions src/jsc/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Box<[u8]>>,
name: bun_core::ZBox,
Expand Down Expand Up @@ -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;
Expand All @@ -362,6 +365,7 @@ impl WebWorker {
exec_argv_len,
))
};
has_eval_string = parsed.is_some_and(|flags| flags.has_eval_string);
Comment thread
robobun marked this conversation as resolved.
if let Some(flags) = parsed {
if let Some(invalid) = flags.invalid {
use bun_core::WTFStringImplExt as _;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions src/runtime/cli/Arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
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))
}
Comment on lines +791 to +800

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.

🟡 nit (optional): maintainers get two helpers answering whether a bun flag takes the next token as its value, and the new one ignores short chains. src/runtime/cli/Arguments.rs:795 takes_value matches only a two-byte -x, while script_or_command.rs:54 has_value_in_next_argument walks chains like -be that bun's clap accepts. So bun -be '<code>' reports process.execArgv as ["-be"] without the code (pre-existing), and a Worker given execArgv: ["-be", "1"] slices at 2 where the main thread slices at 1. Fix: one chain-aware helper in Arguments.rs used by node_process.rs:309 and jsc_hooks.rs:1670,1676, so every spelling bun's CLI accepts gets the same answer. [also at: src/runtime/cli/Arguments.rs:799 - nit, pre-existing: users who chain short flags, e.g. bun -be '<code>', get process.execArgv of ["-be"] with the code token missing, and a Worker walk that never sees the code.]

Why this was flagged

Bun's CLI parser chains short flags and lets the last letter take the next token (src/clap/streaming.rs:265-299), so bun -be '<code>' zzz is --bun -e <code> and run_command.rs:1092 sets has_eval_string true for the main thread. The new takes_value at src/runtime/cli/Arguments.rs:794-795 only recognises [b'-', b'-', long] and the exact two-byte [b'-', short], so takes_value(b"-be") is None. In node_process.rs:309 that None != Some(true) ends the execArgv scan at the code token, so process.execArgv is ["-be"]; the base branch's MAP had the same gap, so that part is pre-existing. In src/runtime/jsc_hooks.rs:1675-1677 the (name, None) arm turns that None into value_may_follow = true, so a Worker made with execArgv: ["-be", "1"] consumes "1" as an unknown flag's value, has_eval_string stays false, and parseArgs() in that Worker slices at 2 while the main thread given the same spelling slices at 1; the base Worker scan also sliced at 2, so this is a parity gap, not a regression. script_or_command.rs:39-65 already has the chain-aware version of this question (has_value_in_next_argument), so the repo now carries two implementations that disagree on -be.

Verification: Trigger: a chained short flag, e.g. a Worker created with execArgv: ["-be", "<code>"]. The new takes_value at src/runtime/cli/Arguments.rs:790-800 only matches [b'-', b'-', long @ ..] and the exact two-byte [b'-', short]; takes_value(b"-be") falls to _ => None. In exec_argv_has_eval_string (src/runtime/jsc_hooks.rs:1619-1681) -be lands in (name, None), so has_eval_string stays false and the Worker slices at 2 while the parent slices at 1.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changed in this PR. The facts are right: takes_value reads --name and -n, and a chain such as -be is not read.

  • The set that create_exec_argv had on main has the same gap, so takes_value gives what main gives: bun -be <code> has ["-be"] as process.execArgv on main 9b4284f and on 1.4.2, and bun -br ./x.js file.js has ["-br"].
  • A chain-aware answer there changes process.execArgv for those launches. That is a change to another API, with its own tests in process.test.js.
  • One helper for all three callers also needs a decision for -pe: has_value_in_next_argument reads it as -p with the attached value e, and the CLI reads -pe <code> as -p <code> (NODE_SHORT_ALIASES).
  • For a Worker, main and this branch agree on these spellings: execArgv: ["-be", "1"] slices at 2 on both.

The description has the case under "Not in this change" (spellings of bun's CLI that the walk does not read).


/// `--disallow-code-generation-from-strings[=<value>]`. Raises the process's level; never lowers it.
fn disallow_code_generation_from_strings(value: &[u8]) {
match bun_core::CodeGenerationFromStrings::from_flag_value(value) {
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/cli/run_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,10 @@ Full documentation is available at <magenta>https://bun.com/docs/cli/run<r>
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 <code> <file>` 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;
}
Comment thread
claude[bot] marked this conversation as resolved.
Expand Down
7 changes: 1 addition & 6 deletions src/runtime/cli/script_or_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <command>` runs.
Expand All @@ -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.
Expand Down
71 changes: 71 additions & 0 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1577,9 +1577,15 @@ unsafe fn apply_standalone_runtime_flags(
unsafe fn parse_worker_exec_argv_flags(
exec_argv: &[bun_core::WTFStringImpl],
) -> Option<WorkerExecArgvFlags> {
// 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() {
Expand Down Expand Up @@ -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<Item = &'a bun_core::WTFStringImplStruct>,
) -> 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=<code>` and `-p=<code>` 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=<x>` 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
Expand Down
49 changes: 5 additions & 44 deletions src/runtime/node/node_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ─────────────────────────────
Expand Down Expand Up @@ -58,10 +56,6 @@ pub(crate) fn get_argv(global: &JSGlobalObject) -> JsResult<JSValue> {
bun_jsc::call_zero_is_throw(global, || Bun__Process__getArgv(global))
}

pub(crate) fn get_exec_argv(global: &JSGlobalObject) -> JsResult<JSValue> {
bun_jsc::call_zero_is_throw(global, || Bun__Process__getExecArgv(global))
}

// ───────────────────────────── exit ─────────────────────────────

// @190n: this may need to be noreturn
Expand Down Expand Up @@ -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<bun_collections::StringSet> =
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
Expand Down
30 changes: 8 additions & 22 deletions src/runtime/node/util/parse_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ArgsSlice> {
// 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,
});
}
Expand Down
7 changes: 0 additions & 7 deletions test/expected-durations.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions test/js/node/test/fixtures/parse-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const { parseArgs } = require('util');

const parsedArgs = parseArgs({ strict: false }).values;

process.stdout.write(JSON.stringify(parsedArgs));
Loading
Loading