diff --git a/src/clap/comptime.rs b/src/clap/comptime.rs index 345ddad22196..dae4b5c1e1ff 100644 --- a/src/clap/comptime.rs +++ b/src/clap/comptime.rs @@ -504,13 +504,14 @@ pub fn convert_params(params: &[Param]) -> (Vec>, usize, us // ───────────────────────────────────────────────────────────────────────────── /// Deprecated: Use `parse_ex` instead -pub struct ComptimeClap { - // Inner `&'static [u8]` slices borrow argv (process-lifetime). - pub single_options: Box<[Option<&'static [u8]>]>, - pub multi_options: Box<[Box<[&'static [u8]]>]>, +pub struct ComptimeClap<'a, Id> { + // Inner `&'a [u8]` slices borrow argv. `'a` is `'static` for `OsIterator` + // (process argv); a borrowed `SliceIterator` gets its own lifetime. + pub single_options: Box<[Option<&'a [u8]>]>, + pub multi_options: Box<[Box<[&'a [u8]]>]>, pub flags: Box<[bool]>, - pub pos: Box<[&'static [u8]]>, - pub passthrough_positionals: Box<[&'static [u8]]>, + pub pos: Box<[&'a [u8]]>, + pub passthrough_positionals: Box<[&'a [u8]]>, // The converted params are // carried as a `&'static` table — either rodata (`comptime_table!`) or @@ -520,9 +521,9 @@ pub struct ComptimeClap { _id: PhantomData, } -impl ComptimeClap { - /// `iter` must yield `&'static [u8]` (process-lifetime args, e.g. `OsIterator`) - /// because parsed values are stored by reference. +impl<'a, Id> ComptimeClap<'a, Id> { + /// Parsed values are stored by reference into the `&'a [u8]` slices + /// yielded by `iter`; `'a` is `'static` for `OsIterator` (process argv). /// /// `params` must be `'static` (every in-tree table is a `static`/`const` /// item); the converted form is interned once per unique slice. @@ -540,7 +541,7 @@ impl ComptimeClap { opt: ParseOptions<'_>, ) -> Result where - I: ArgIter<'static>, + I: ArgIter<'a>, { Self::parse_with_table(ConvertedTable::for_params(params), iter, opt) } @@ -553,15 +554,15 @@ impl ComptimeClap { opt: ParseOptions<'_>, ) -> Result where - I: ArgIter<'static>, + I: ArgIter<'a>, { // `opt.allocator` dropped — global mimalloc. - let mut multis: Vec> = (0..table.n_multi).map(|_| Vec::new()).collect(); + let mut multis: Vec> = (0..table.n_multi).map(|_| Vec::new()).collect(); - let mut pos: Vec<&'static [u8]> = Vec::new(); - let mut passthrough_positionals: Vec<&'static [u8]> = Vec::new(); + let mut pos: Vec<&'a [u8]> = Vec::new(); + let mut passthrough_positionals: Vec<&'a [u8]> = Vec::new(); - let mut single_options: Box<[Option<&'static [u8]>]> = + let mut single_options: Box<[Option<&'a [u8]>]> = vec![None; table.n_single].into_boxed_slice(); let mut flags: Box<[bool]> = vec![false; table.n_flags].into_boxed_slice(); @@ -637,7 +638,7 @@ impl ComptimeClap { } #[inline] - pub fn option(&self, name: &[u8]) -> Option<&'static [u8]> { + pub fn option(&self, name: &[u8]) -> Option<&'a [u8]> { let param = self.table.find(name); debug_assert!( param.takes_value != Values::None, @@ -653,7 +654,7 @@ impl ComptimeClap { } #[inline] - pub fn options(&self, name: &[u8]) -> &[&'static [u8]] { + pub fn options(&self, name: &[u8]) -> &[&'a [u8]] { let param = self.table.find(name); debug_assert!( param.takes_value != Values::None, @@ -675,19 +676,19 @@ impl ComptimeClap { self.flags[self.table.converted[converted_idx].id] } #[inline] - pub fn option_at(&self, converted_idx: usize) -> Option<&'static [u8]> { + pub fn option_at(&self, converted_idx: usize) -> Option<&'a [u8]> { self.single_options[self.table.converted[converted_idx].id] } #[inline] - pub fn options_at(&self, converted_idx: usize) -> &[&'static [u8]] { + pub fn options_at(&self, converted_idx: usize) -> &[&'a [u8]] { &self.multi_options[self.table.converted[converted_idx].id] } - pub fn positionals(&self) -> &[&'static [u8]] { + pub fn positionals(&self) -> &[&'a [u8]] { &self.pos } - pub fn remaining(&self) -> &[&'static [u8]] { + pub fn remaining(&self) -> &[&'a [u8]] { &self.passthrough_positionals } diff --git a/src/clap/lib.rs b/src/clap/lib.rs index 768234df2d61..7b8fc2e84003 100644 --- a/src/clap/lib.rs +++ b/src/clap/lib.rs @@ -455,29 +455,29 @@ fn get_value_simple(param: &Param) -> &'static [u8] { param.id.value } -pub struct Args { - pub clap: ComptimeClap, - pub exe_arg: Option<&'static [u8]>, +pub struct Args<'a, Id: 'static> { + pub clap: ComptimeClap<'a, Id>, + pub exe_arg: Option<&'a [u8]>, } -impl Args { +impl<'a, Id: 'static> Args<'a, Id> { pub fn flag(&self, name: &'static [u8]) -> bool { self.clap.flag(name) } - pub fn option(&self, name: &'static [u8]) -> Option<&'static [u8]> { + pub fn option(&self, name: &'static [u8]) -> Option<&'a [u8]> { self.clap.option(name) } - pub fn options(&self, name: &'static [u8]) -> &[&'static [u8]] { + pub fn options(&self, name: &'static [u8]) -> &[&'a [u8]] { self.clap.options(name) } - pub fn positionals(&self) -> &[&'static [u8]] { + pub fn positionals(&self) -> &[&'a [u8]] { self.clap.positionals() } - pub fn remaining(&self) -> &[&'static [u8]] { + pub fn remaining(&self) -> &[&'a [u8]] { self.clap.remaining() } @@ -497,7 +497,7 @@ impl Args { pub fn parse( params: &'static [Param], opt: ParseOptions<'_>, -) -> Result, bun_core::Error> { +) -> Result, bun_core::Error> { let mut iter = args::OsIterator::init(); let exe_arg = iter.exe_arg; @@ -518,7 +518,7 @@ pub fn parse( pub fn parse_with_table( table: &'static ConvertedTable, opt: ParseOptions<'_>, -) -> Result, bun_core::Error> { +) -> Result, bun_core::Error> { let mut iter = args::OsIterator::init(); let exe_arg = iter.exe_arg; let clap = ComptimeClap::::parse_with_table( @@ -538,13 +538,13 @@ pub fn parse_with_table( /// **Cold path** — see [`parse`]; the startup hot path is [`parse_with_table`]. #[cold] #[inline(never)] -pub fn parse_ex( +pub fn parse_ex<'a, Id: 'static, I>( params: &'static [Param], iter: &mut I, opt: ParseOptions<'_>, -) -> Result, bun_core::Error> +) -> Result, bun_core::Error> where - I: args::ArgIter<'static>, + I: args::ArgIter<'a>, { ComptimeClap::::parse(params, iter, opt) } diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index a56936359a69..5114fb0cdf32 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1451,14 +1451,7 @@ unsafe fn apply_standalone_runtime_flags( /// `RunCommand` param table and return `!args.flag("--no-addons")`, or `None` /// on parse error. /// -/// Note: the Rust `bun_clap::parse_ex` port currently constrains -/// `ArgIter<'static>` (parsed values are stored by reference), which would -/// force leaking the per-call UTF-8 copies of `exec_argv`. Spec only ever -/// reads the single `--no-addons` flag from the result (per the in-tree -/// `// TODO: currently this only checks for --no-addons`), so this body scans -/// the converted argv directly with the same `stop_after_positional_at = 1` -/// short-circuit. Full clap routing can return when `ComptimeClap` grows a -/// borrowed-lifetime variant. +/// Currently only honours `--no-addons`. /// /// # Safety /// Each `WTFStringImpl` in `exec_argv` is a live WTF string (the C++ @@ -1466,27 +1459,27 @@ unsafe fn apply_standalone_runtime_flags( unsafe fn parse_worker_exec_argv_allow_addons( exec_argv: &[bun_core::WTFStringImpl], ) -> Option { - let mut no_addons = false; - for &arg in exec_argv { - if arg.is_null() { - continue; - } - // SAFETY: per fn contract — `arg` is a live `WTFStringImpl*`. - let owned = unsafe { &*arg }.to_owned_slice_z(); - let bytes = owned.as_bytes(); - // `stop_after_positional_at = 1` — first non-flag token ends parsing. - if bytes.first() != Some(&b'-') { - break; - } - if bytes == b"--" { - break; - } - if bytes == b"--no-addons" { - no_addons = true; - } - } - // Override `allow_addons` unconditionally on successful parse. - Some(!no_addons) + let owned: Vec<_> = exec_argv + .iter() + .filter(|a| !a.is_null()) + // SAFETY: per fn contract — each `arg` is a live `WTFStringImpl*`. + .map(|&a| unsafe { &*a }.to_owned_slice_z()) + .collect(); + let argv: Vec<&[u8]> = owned.iter().map(|s| s.as_bytes()).collect(); + + let mut iter = bun_clap::args::SliceIterator::init(&argv); + let args = bun_clap::ComptimeClap::::parse_with_table( + crate::cli::arguments::RUN_TABLE, + &mut iter, + bun_clap::ParseOptions { + diagnostic: None, + stop_after_positional_at: 1, + }, + ) + .ok()?; + + // override the existing even if it was set + Some(!args.flag(b"--no-addons")) } /// `jsc.API.cron.CronJob.clearAllForVM(vm, .teardown)` — diff --git a/test/js/node/no-addons.test.ts b/test/js/node/no-addons.test.ts index 0df8cf5155e2..e58026be2ff6 100644 --- a/test/js/node/no-addons.test.ts +++ b/test/js/node/no-addons.test.ts @@ -1,6 +1,7 @@ import { spawnSync } from "bun"; -import { expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { bunExe, bunEnv as env } from "harness"; +import { Worker } from "node:worker_threads"; test("--no-addons throws an error on process.dlopen", () => { const { stdout, stderr, exitCode } = spawnSync({ @@ -15,3 +16,40 @@ test("--no-addons throws an error on process.dlopen", () => { expect(out).toBeEmpty(); expect(err).toContain("\nerror: Cannot load native addon because loading addons is disabled."); }); + +describe("worker execArgv --no-addons parsing matches RunCommand clap", () => { + // A value token following a value-taking flag (`-r x`, `--title x`, ...) + // must not be treated as the first positional when scanning execArgv, and + // the value is consumed regardless of its own content (even `--` or a + // `-`-prefixed string). + const body = `try { process.dlopen({ exports: {} }, "/nonexistent.node"); } catch (e) { require("node:worker_threads").parentPort.postMessage(e.code); }`; + test.each([ + [["--no-addons"], "ERR_DLOPEN_DISABLED"], + [["-r", "./preload.js", "--no-addons"], "ERR_DLOPEN_DISABLED"], + [["--require", "./preload.js", "--no-addons"], "ERR_DLOPEN_DISABLED"], + [["--title", "foo", "--no-addons"], "ERR_DLOPEN_DISABLED"], + [["--port", "3000", "--no-addons"], "ERR_DLOPEN_DISABLED"], + [["-e", "void 0", "--no-addons"], "ERR_DLOPEN_DISABLED"], + [["--no-addons", "-r", "./preload.js"], "ERR_DLOPEN_DISABLED"], + // chained shorts ending in a value-taking short pull the next token + [["-br", "./preload.js", "--no-addons"], "ERR_DLOPEN_DISABLED"], + // the value is consumed via raw iter.next(), even if it is `--` + [["-r", "--", "--no-addons"], "ERR_DLOPEN_DISABLED"], + // here `--no-addons` is `-r`'s value, not a flag; addons stay enabled + [["-r", "--no-addons"], "ERR_DLOPEN_FAILED"], + // bare `-` is a positional; parsing stops before `--no-addons` + [["-", "--no-addons"], "ERR_DLOPEN_FAILED"], + ])("%j", async (execArgv, expected) => { + const worker = new Worker(body, { eval: true, execArgv }); + try { + const code = await new Promise((resolve, reject) => { + worker.once("message", resolve); + worker.once("error", reject); + worker.once("exit", exitCode => reject(new Error(`worker exited (code=${exitCode}) before posting a message`))); + }); + expect(code).toBe(expected); + } finally { + await worker.terminate(); + } + }); +});