Skip to content

Commit ffbe236

Browse files
bartlomiejuclaude
andauthored
fix(ext/node): support numeric FDs in child_process stdio array (#32959)
## Summary - Node.js allows passing raw file descriptors as numbers in the `stdio` array for `child_process.spawn()`, e.g. `spawn('cmd', { stdio: ['ignore', 'pipe', 'pipe', fd] })`. This previously failed with `serde_v8 error: invalid type; expected: enum, got: Number` - Refactor `StdioOrRid` → `StdioOrFdOrRid` with a new `Fd(i32)` variant for raw file descriptors - Numbers in `extra_stdio` now deserialize as `Fd` and are dup2'd to the target fd in the child process on Unix - `as_stdio()` also handles `Fd` by dup'ing it into a `std::fs::File` for use with stdin/stdout/stderr Towards #32846 --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4fc7ee2 commit ffbe236

2 files changed

Lines changed: 165 additions & 70 deletions

File tree

ext/process/lib.rs

Lines changed: 132 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ use std::collections::HashMap;
77
use std::ffi::OsString;
88
use std::io::Write;
99
#[cfg(unix)]
10+
use std::os::fd::FromRawFd;
11+
#[cfg(unix)]
1012
use std::os::unix::prelude::ExitStatusExt;
1113
#[cfg(unix)]
1214
use std::os::unix::process::CommandExt;
@@ -79,12 +81,14 @@ impl Stdio {
7981
}
8082

8183
#[derive(Copy, Clone, Eq, PartialEq)]
82-
pub enum StdioOrRid {
84+
pub enum StdioOrFdOrRid {
8385
Stdio(Stdio),
86+
/// A raw file descriptor (e.g. from Node's `fs.openSync`).
87+
Fd(i32),
8488
Rid(ResourceId),
8589
}
8690

87-
impl<'de> Deserialize<'de> for StdioOrRid {
91+
impl<'de> Deserialize<'de> for StdioOrFdOrRid {
8892
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
8993
where
9094
D: serde::Deserializer<'de>,
@@ -93,38 +97,69 @@ impl<'de> Deserialize<'de> for StdioOrRid {
9397
let value = Value::deserialize(deserializer)?;
9498
match value {
9599
Value::String(val) => match val.as_str() {
96-
"inherit" => Ok(StdioOrRid::Stdio(Stdio::Inherit)),
97-
"piped" => Ok(StdioOrRid::Stdio(Stdio::Piped)),
98-
"null" => Ok(StdioOrRid::Stdio(Stdio::Null)),
100+
"inherit" => Ok(StdioOrFdOrRid::Stdio(Stdio::Inherit)),
101+
"piped" => Ok(StdioOrFdOrRid::Stdio(Stdio::Piped)),
102+
"null" => Ok(StdioOrFdOrRid::Stdio(Stdio::Null)),
99103
"ipc_for_internal_use" => {
100-
Ok(StdioOrRid::Stdio(Stdio::IpcForInternalUse))
104+
Ok(StdioOrFdOrRid::Stdio(Stdio::IpcForInternalUse))
101105
}
102106
val => Err(serde::de::Error::unknown_variant(
103107
val,
104108
&["inherit", "piped", "null"],
105109
)),
106110
},
107-
Value::Number(val) => match val.as_u64() {
108-
Some(val) if val <= ResourceId::MAX as u64 => {
109-
Ok(StdioOrRid::Rid(val as ResourceId))
111+
Value::Number(val) => match val.as_i64() {
112+
Some(val) if val >= 0 && val <= i32::MAX as i64 => {
113+
Ok(StdioOrFdOrRid::Fd(val as i32))
110114
}
111-
_ => Err(serde::de::Error::custom("Expected a positive integer")),
115+
_ => Err(serde::de::Error::custom("Expected a non-negative integer")),
112116
},
113117
_ => Err(serde::de::Error::custom(
114-
r#"Expected a resource id, "inherit", "piped", or "null""#,
118+
r#"Expected a file descriptor, "inherit", "piped", or "null""#,
115119
)),
116120
}
117121
}
118122
}
119123

120-
impl StdioOrRid {
124+
impl StdioOrFdOrRid {
121125
pub fn as_stdio(
122126
&self,
123127
state: &mut OpState,
124128
) -> Result<StdStdio, ProcessError> {
125129
match &self {
126-
StdioOrRid::Stdio(val) => Ok(val.as_stdio()),
127-
StdioOrRid::Rid(rid) => {
130+
StdioOrFdOrRid::Stdio(val) => Ok(val.as_stdio()),
131+
StdioOrFdOrRid::Fd(fd) => {
132+
// Convert a raw FD to a Stdio by dup'ing it into a std::fs::File.
133+
#[cfg(unix)]
134+
{
135+
// SAFETY: libc::dup is safe to call with any fd; returns -1 on error.
136+
let duped = unsafe { libc::dup(*fd) };
137+
if duped < 0 {
138+
return Err(ProcessError::Io(std::io::Error::last_os_error()));
139+
}
140+
// SAFETY: duped is a valid fd we own (just created by dup).
141+
Ok(unsafe { std::fs::File::from_raw_fd(duped) }.into())
142+
}
143+
#[cfg(windows)]
144+
{
145+
// SAFETY: get_osfhandle converts a CRT fd to a Windows HANDLE; returns -1 on error.
146+
let handle = unsafe { libc::get_osfhandle(*fd) };
147+
if handle == -1 {
148+
return Err(ProcessError::Io(std::io::Error::last_os_error()));
149+
}
150+
// SAFETY: handle is a valid Windows HANDLE (verified non-negative above).
151+
// BorrowedHandle does not take ownership.
152+
let borrowed = unsafe {
153+
std::os::windows::io::BorrowedHandle::borrow_raw(
154+
handle as *mut std::ffi::c_void,
155+
)
156+
};
157+
let owned =
158+
borrowed.try_clone_to_owned().map_err(ProcessError::Io)?;
159+
Ok(std::fs::File::from(owned).into())
160+
}
161+
}
162+
StdioOrFdOrRid::Rid(rid) => {
128163
Ok(FileResource::with_file(state, *rid, |file| {
129164
file.as_stdio().map_err(deno_error::JsErrorBox::from_err)
130165
})?)
@@ -133,7 +168,7 @@ impl StdioOrRid {
133168
}
134169

135170
pub fn is_ipc(&self) -> bool {
136-
matches!(self, StdioOrRid::Stdio(Stdio::IpcForInternalUse))
171+
matches!(self, StdioOrFdOrRid::Stdio(Stdio::IpcForInternalUse))
137172
}
138173
}
139174

@@ -248,7 +283,7 @@ pub struct SpawnArgs {
248283

249284
input: Option<JsBuffer>,
250285

251-
extra_stdio: Vec<Stdio>,
286+
extra_stdio: Vec<StdioOrFdOrRid>,
252287
detached: bool,
253288
needs_npm_process_state: bool,
254289
#[cfg(unix)]
@@ -349,9 +384,9 @@ pub enum ProcessError {
349384
#[derive(Deserialize)]
350385
#[serde(rename_all = "camelCase")]
351386
pub struct ChildStdio {
352-
stdin: StdioOrRid,
353-
stdout: StdioOrRid,
354-
stderr: StdioOrRid,
387+
stdin: StdioOrFdOrRid,
388+
stdout: StdioOrFdOrRid,
389+
stderr: StdioOrFdOrRid,
355390
}
356391

357392
#[derive(ToV8)]
@@ -518,11 +553,15 @@ fn create_command(
518553
}
519554

520555
command.stdout(match args.stdio.stdout {
521-
StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(1).as_stdio(state)?,
556+
StdioOrFdOrRid::Stdio(Stdio::Inherit) => {
557+
StdioOrFdOrRid::Rid(1).as_stdio(state)?
558+
}
522559
value => value.as_stdio(state)?,
523560
});
524561
command.stderr(match args.stdio.stderr {
525-
StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(2).as_stdio(state)?,
562+
StdioOrFdOrRid::Stdio(Stdio::Inherit) => {
563+
StdioOrFdOrRid::Rid(2).as_stdio(state)?
564+
}
526565
value => value.as_stdio(state)?,
527566
});
528567

@@ -578,26 +617,36 @@ fn create_command(
578617
// index 0 in `extra_stdio` actually refers to fd 3
579618
// because we handle stdin,stdout,stderr specially
580619
let fd = (i + 3) as i32;
581-
// TODO(nathanwhit): handle inherited, but this relies on the parent process having
582-
// fds open already. since we don't generally support dealing with raw fds,
583-
// we can't properly support this
584-
if matches!(stdio, Stdio::Piped) {
585-
let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
586-
fds_to_dup.push((fd2, fd));
587-
fds_to_close.push(fd2);
588-
let rid = state.resource_table.add(
589-
match deno_io::BiPipeResource::from_raw_handle(fd1) {
590-
Ok(v) => v,
591-
Err(e) => {
592-
log::warn!("Failed to open bidirectional pipe for fd {fd}: {e}");
593-
extra_pipe_rids.push(None);
594-
continue;
595-
}
596-
},
597-
);
598-
extra_pipe_rids.push(Some(rid));
599-
} else {
600-
extra_pipe_rids.push(None);
620+
match stdio {
621+
StdioOrFdOrRid::Stdio(Stdio::Piped) => {
622+
let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
623+
fds_to_dup.push((fd2, fd));
624+
fds_to_close.push(fd2);
625+
let rid = state.resource_table.add(
626+
match deno_io::BiPipeResource::from_raw_handle(fd1) {
627+
Ok(v) => v,
628+
Err(e) => {
629+
log::warn!(
630+
"Failed to open bidirectional pipe for fd {fd}: {e}"
631+
);
632+
extra_pipe_rids.push(None);
633+
continue;
634+
}
635+
},
636+
);
637+
extra_pipe_rids.push(Some(rid));
638+
}
639+
StdioOrFdOrRid::Fd(raw_fd) => {
640+
// Raw file descriptor (from fs.openSync etc.)
641+
// dup2 it to the target fd in the child process.
642+
// The caller retains ownership of raw_fd so we don't
643+
// add it to fds_to_close.
644+
fds_to_dup.push((raw_fd, fd));
645+
extra_pipe_rids.push(None);
646+
}
647+
_ => {
648+
extra_pipe_rids.push(None);
649+
}
601650
}
602651
}
603652

@@ -670,28 +719,41 @@ fn create_command(
670719
// index 0 in `extra_stdio` actually refers to fd 3
671720
// because we handle stdin,stdout,stderr specially
672721
let fd = (i + 3) as i32;
673-
// TODO(nathanwhit): handle inherited, but this relies on the parent process having
674-
// fds open already. since we don't generally support dealing with raw fds,
675-
// we can't properly support this
676-
if matches!(stdio, Stdio::Piped) {
677-
let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
678-
handles_to_close.push(fd2);
679-
let rid = state.resource_table.add(
680-
match deno_io::BiPipeResource::from_raw_handle(fd1) {
681-
Ok(v) => v,
682-
Err(e) => {
683-
log::warn!("Failed to open bidirectional pipe for fd {fd}: {e}");
684-
extra_pipe_rids.push(None);
685-
continue;
686-
}
687-
},
688-
);
689-
command.extra_handle(Some(fd2));
690-
extra_pipe_rids.push(Some(rid));
691-
} else {
692-
// no handle, push an empty handle so we need get the right fds for following handles
693-
command.extra_handle(None);
694-
extra_pipe_rids.push(None);
722+
match stdio {
723+
StdioOrFdOrRid::Stdio(Stdio::Piped) => {
724+
let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
725+
handles_to_close.push(fd2);
726+
let rid = state.resource_table.add(
727+
match deno_io::BiPipeResource::from_raw_handle(fd1) {
728+
Ok(v) => v,
729+
Err(e) => {
730+
log::warn!(
731+
"Failed to open bidirectional pipe for fd {fd}: {e}"
732+
);
733+
extra_pipe_rids.push(None);
734+
continue;
735+
}
736+
},
737+
);
738+
command.extra_handle(Some(fd2));
739+
extra_pipe_rids.push(Some(rid));
740+
}
741+
StdioOrFdOrRid::Fd(raw_fd) => {
742+
// SAFETY: get_osfhandle converts a CRT fd to a Windows HANDLE; returns -1 on error.
743+
let handle = unsafe { libc::get_osfhandle(raw_fd) };
744+
if handle != -1 {
745+
command.extra_handle(Some(handle as _));
746+
} else {
747+
command.extra_handle(None);
748+
}
749+
extra_pipe_rids.push(None);
750+
}
751+
_ => {
752+
// No handle -- push an empty slot so subsequent handles
753+
// get the right fd indices.
754+
command.extra_handle(None);
755+
extra_pipe_rids.push(None);
756+
}
695757
}
696758
}
697759

@@ -1115,8 +1177,8 @@ fn op_spawn_sync(
11151177
state: &mut OpState,
11161178
#[serde] args: SpawnArgs,
11171179
) -> Result<SpawnOutput, ProcessError> {
1118-
let stdout = matches!(args.stdio.stdout, StdioOrRid::Stdio(Stdio::Piped));
1119-
let stderr = matches!(args.stdio.stderr, StdioOrRid::Stdio(Stdio::Piped));
1180+
let stdout = matches!(args.stdio.stdout, StdioOrFdOrRid::Stdio(Stdio::Piped));
1181+
let stderr = matches!(args.stdio.stderr, StdioOrFdOrRid::Stdio(Stdio::Piped));
11201182
let input = args.input.clone();
11211183
let (mut command, _, _, _) =
11221184
create_command(state, args, "Deno.Command().outputSync()")?;
@@ -1213,11 +1275,11 @@ mod deprecated {
12131275
cwd: Option<String>,
12141276
env: Vec<(String, String)>,
12151277
#[from_v8(serde)]
1216-
stdin: StdioOrRid,
1278+
stdin: StdioOrFdOrRid,
12171279
#[from_v8(serde)]
1218-
stdout: StdioOrRid,
1280+
stdout: StdioOrFdOrRid,
12191281
#[from_v8(serde)]
1220-
stderr: StdioOrRid,
1282+
stderr: StdioOrFdOrRid,
12211283
}
12221284

12231285
struct ChildResource {
@@ -1293,14 +1355,14 @@ mod deprecated {
12931355
c.stdin(run_args.stdin.as_stdio(state)?);
12941356
c.stdout(
12951357
match run_args.stdout {
1296-
StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(1),
1358+
StdioOrFdOrRid::Stdio(Stdio::Inherit) => StdioOrFdOrRid::Rid(1),
12971359
value => value,
12981360
}
12991361
.as_stdio(state)?,
13001362
);
13011363
c.stderr(
13021364
match run_args.stderr {
1303-
StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(2),
1365+
StdioOrFdOrRid::Stdio(Stdio::Inherit) => StdioOrFdOrRid::Rid(2),
13041366
value => value,
13051367
}
13061368
.as_stdio(state)?,

tests/unit_node/child_process_test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import CP from "node:child_process";
44
import { Buffer } from "node:buffer";
5+
import * as fs from "node:fs";
56
import {
67
assert,
78
assertEquals,
@@ -1344,3 +1345,35 @@ Deno.test(async function stdoutWriteMultipleChunksNotTruncated() {
13441345
await Deno.remove(script);
13451346
}
13461347
});
1348+
1349+
Deno.test({
1350+
name: "[node/child_process spawn] supports numeric fd in stdio array",
1351+
// On Windows, Deno's fs.openSync returns a resource ID, not a CRT file
1352+
// descriptor, so libc::get_osfhandle receives an invalid fd and crashes.
1353+
ignore: Deno.build.os === "windows",
1354+
async fn() {
1355+
const tmpFile = await Deno.makeTempFile();
1356+
try {
1357+
const fd = fs.openSync(tmpFile, "r");
1358+
try {
1359+
// Passing a numeric fd at stdio index 3 should not throw.
1360+
// Previously this caused: serde_v8 error: invalid type; expected: enum, got: Number
1361+
const child = spawn(Deno.execPath(), [
1362+
"eval",
1363+
"/* no-op */",
1364+
], {
1365+
stdio: ["ignore", "ignore", "ignore", fd],
1366+
});
1367+
1368+
const deferred = withTimeout<number>();
1369+
child.on("exit", (code: number) => deferred.resolve(code));
1370+
const code = await deferred.promise;
1371+
assertEquals(code, 0);
1372+
} finally {
1373+
fs.closeSync(fd);
1374+
}
1375+
} finally {
1376+
await Deno.remove(tmpFile);
1377+
}
1378+
},
1379+
});

0 commit comments

Comments
 (0)