Skip to content
Open
43 changes: 22 additions & 21 deletions src/clap/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,13 +504,14 @@ pub fn convert_params<Id>(params: &[Param<Id>]) -> (Vec<Param<usize>>, usize, us
// ─────────────────────────────────────────────────────────────────────────────

/// Deprecated: Use `parse_ex` instead
pub struct ComptimeClap<Id> {
// 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
Expand All @@ -520,9 +521,9 @@ pub struct ComptimeClap<Id> {
_id: PhantomData<Id>,
}

impl<Id> ComptimeClap<Id> {
/// `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.
Expand All @@ -540,7 +541,7 @@ impl<Id> ComptimeClap<Id> {
opt: ParseOptions<'_>,
) -> Result<Self, bun_core::Error>
where
I: ArgIter<'static>,
I: ArgIter<'a>,
{
Self::parse_with_table(ConvertedTable::for_params(params), iter, opt)
}
Expand All @@ -553,15 +554,15 @@ impl<Id> ComptimeClap<Id> {
opt: ParseOptions<'_>,
) -> Result<Self, bun_core::Error>
where
I: ArgIter<'static>,
I: ArgIter<'a>,
{
// `opt.allocator` dropped — global mimalloc.
let mut multis: Vec<Vec<&'static [u8]>> = (0..table.n_multi).map(|_| Vec::new()).collect();
let mut multis: Vec<Vec<&'a [u8]>> = (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();

Expand Down Expand Up @@ -637,7 +638,7 @@ impl<Id> ComptimeClap<Id> {
}

#[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,
Expand All @@ -653,7 +654,7 @@ impl<Id> ComptimeClap<Id> {
}

#[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,
Expand All @@ -675,19 +676,19 @@ impl<Id> ComptimeClap<Id> {
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
}

Expand Down
26 changes: 13 additions & 13 deletions src/clap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,29 +455,29 @@ fn get_value_simple(param: &Param<Help>) -> &'static [u8] {
param.id.value
}

pub struct Args<Id: 'static> {
pub clap: ComptimeClap<Id>,
pub exe_arg: Option<&'static [u8]>,
pub struct Args<'a, Id: 'static> {
pub clap: ComptimeClap<'a, Id>,
pub exe_arg: Option<&'a [u8]>,
}

impl<Id: 'static> Args<Id> {
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()
}

Expand All @@ -497,7 +497,7 @@ impl<Id: 'static> Args<Id> {
pub fn parse<Id: 'static>(
params: &'static [Param<Id>],
opt: ParseOptions<'_>,
) -> Result<Args<Id>, bun_core::Error> {
) -> Result<Args<'static, Id>, bun_core::Error> {
let mut iter = args::OsIterator::init();
let exe_arg = iter.exe_arg;

Expand All @@ -518,7 +518,7 @@ pub fn parse<Id: 'static>(
pub fn parse_with_table<Id: 'static>(
table: &'static ConvertedTable,
opt: ParseOptions<'_>,
) -> Result<Args<Id>, bun_core::Error> {
) -> Result<Args<'static, Id>, bun_core::Error> {
let mut iter = args::OsIterator::init();
let exe_arg = iter.exe_arg;
let clap = ComptimeClap::<Id>::parse_with_table(
Expand All @@ -538,13 +538,13 @@ pub fn parse_with_table<Id: 'static>(
/// **Cold path** — see [`parse`]; the startup hot path is [`parse_with_table`].
#[cold]
#[inline(never)]
pub fn parse_ex<Id: 'static, I>(
pub fn parse_ex<'a, Id: 'static, I>(
params: &'static [Param<Id>],
iter: &mut I,
opt: ParseOptions<'_>,
) -> Result<ComptimeClap<Id>, bun_core::Error>
) -> Result<ComptimeClap<'a, Id>, bun_core::Error>
where
I: args::ArgIter<'static>,
I: args::ArgIter<'a>,
{
ComptimeClap::<Id>::parse(params, iter, opt)
}
Expand Down
51 changes: 22 additions & 29 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1451,42 +1451,35 @@ 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++
/// `Worker::create` array, kept alive for the worker's lifetime).
unsafe fn parse_worker_exec_argv_allow_addons(
exec_argv: &[bun_core::WTFStringImpl],
) -> Option<bool> {
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::<bun_clap::Help>::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)` —
Expand Down
40 changes: 39 additions & 1 deletion test/js/node/no-addons.test.ts
Original file line number Diff line number Diff line change
@@ -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({
Expand All @@ -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`)));
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(code).toBe(expected);
} finally {
await worker.terminate();
}
});
});
Loading