Skip to content

Commit 75bc1f6

Browse files
bartlomiejuclaude
andcommitted
fix: use quote-aware parser for NODE_OPTIONS and add argv overwrite justification
Address review feedback: replace naive whitespace splitting of NODE_OPTIONS with a shell-like tokenizer that handles single/double quoted values (e.g. --title="hello world"). Add comments explaining the argv buffer overwrite technique with references to libuv/Node.js. Add spec tests for process.title including quoted NODE_OPTIONS values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 465b71e commit 75bc1f6

File tree

8 files changed

+102
-5
lines changed

8 files changed

+102
-5
lines changed

ext/node/ops/process.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ use nix::unistd::User;
1717
use crate::ExtNodeSys;
1818

1919
// --- process.title support ---
20+
//
21+
// The argv buffer overwrite technique used here is the standard approach for
22+
// setting the process title visible in `ps`. This is the same technique used by
23+
// Node.js (via libuv's uv_setup_args/uv_set_process_title), nginx, PostgreSQL,
24+
// and many other programs. The OS allocates argv as a contiguous buffer; we
25+
// save its bounds at startup, then overwrite it with the new title.
26+
//
27+
// References:
28+
// - libuv: https://github.com/libuv/libuv/blob/v1.x/src/unix/proctitle.c
29+
// - Node.js: uses uv_setup_args() in node_main.cc, uv_set_process_title() in node.cc
2030

2131
#[cfg(target_os = "linux")]
2232
mod argv_store {

ext/node/polyfills/internal_binding/node_options.ts

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@ import { primordials } from "ext:core/mod.js";
44
const {
55
SafeMap,
66
ArrayPrototypeForEach,
7-
SafeRegExp,
7+
ArrayPrototypePush,
88
StringPrototypeSlice,
9-
StringPrototypeSplit,
109
StringPrototypeStartsWith,
1110
} = primordials;
1211

@@ -15,6 +14,39 @@ const {
1514
// - https://github.com/nodejs/node/blob/master/src/node_options.cc
1615
// - https://github.com/nodejs/node/blob/master/src/node_options.h
1716

17+
// Quote-aware tokenizer for NODE_OPTIONS. Node.js uses a shell-like parser
18+
// that respects single and double quotes, so `--title="hello world"` is a
19+
// single token whose value is `hello world`, not two tokens.
20+
function splitNodeOptions(input: string): string[] {
21+
const args: string[] = [];
22+
let current = "";
23+
let inDouble = false;
24+
let inSingle = false;
25+
26+
for (let i = 0; i < input.length; i++) {
27+
const ch = input[i];
28+
if (ch === '"' && !inSingle) {
29+
inDouble = !inDouble;
30+
} else if (ch === "'" && !inDouble) {
31+
inSingle = !inSingle;
32+
} else if (
33+
(ch === " " || ch === "\t" || ch === "\n" || ch === "\r") && !inDouble &&
34+
!inSingle
35+
) {
36+
if (current.length > 0) {
37+
ArrayPrototypePush(args, current);
38+
current = "";
39+
}
40+
} else {
41+
current += ch;
42+
}
43+
}
44+
if (current.length > 0) {
45+
ArrayPrototypePush(args, current);
46+
}
47+
return args;
48+
}
49+
1850
/** Gets the all options for Node.js
1951
* This function is expensive to execute. `getOptionValue` in `internal/options.ts`
2052
* should be used instead to get a specific option. */
@@ -26,9 +58,7 @@ export function getOptions() {
2658
]);
2759

2860
const nodeOptions = Deno.env.get("NODE_OPTIONS");
29-
const args = nodeOptions
30-
? StringPrototypeSplit(nodeOptions, new SafeRegExp("\\s"))
31-
: [];
61+
const args = nodeOptions ? splitNodeOptions(nodeOptions) : [];
3262
ArrayPrototypeForEach(args, (arg) => {
3363
if (StringPrototypeStartsWith(arg, "--title=")) {
3464
options.set("--title", { value: StringPrototypeSlice(arg, 8) });
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
{
2+
"tests": {
3+
"default_title_is_exec_path": {
4+
"args": "run -A main.ts default",
5+
"output": "default.out"
6+
},
7+
"set_title": {
8+
"args": "run -A main.ts set",
9+
"output": "set.out"
10+
},
11+
"node_options_title": {
12+
"envs": {
13+
"NODE_OPTIONS": "--title=custom-title"
14+
},
15+
"args": "run -A main.ts node_options",
16+
"output": "node_options.out"
17+
},
18+
"node_options_title_quoted_spaces": {
19+
"envs": {
20+
"NODE_OPTIONS": "--title=\"hello world\""
21+
},
22+
"args": "run -A main.ts node_options",
23+
"output": "node_options_quoted.out"
24+
}
25+
}
26+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
PASS
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import process from "node:process";
2+
3+
const mode = Deno.args[0];
4+
5+
switch (mode) {
6+
case "default":
7+
// Default title should be the execPath
8+
console.log(
9+
process.title === process.execPath
10+
? "PASS"
11+
: `FAIL: expected execPath, got ${process.title}`,
12+
);
13+
break;
14+
case "set":
15+
// Setting title should work
16+
process.title = "my-custom-title";
17+
console.log(
18+
process.title === "my-custom-title"
19+
? "PASS"
20+
: `FAIL: expected my-custom-title, got ${process.title}`,
21+
);
22+
break;
23+
case "node_options":
24+
// Title should come from NODE_OPTIONS --title=...
25+
console.log(`title=${process.title}`);
26+
break;
27+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
title=custom-title
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
title=hello world
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
PASS

0 commit comments

Comments
 (0)