diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts index ed70f054cd77..7f12e2be705a 100644 --- a/src/js/node/child_process.ts +++ b/src/js/node/child_process.ts @@ -1691,9 +1691,6 @@ function isInternalIpcMessage(message) { } function streamFdOf(item): number | undefined { - const itemFd = ObjectHasOwn(item, "fd") ? item.fd : undefined; - if (typeof itemFd === "number") return itemFd; - const handle = item._handle; const handleFd = handle ? handle.fd : undefined; if (typeof handleFd === "number") return handleFd; @@ -1709,7 +1706,7 @@ function streamFdOf(item): number | undefined { return undefined; } -function nodeToBun(item: string, index: number): string | number | null | NodeJS.TypedArray | ArrayBufferView { +function nodeToBun(item, index: number): string | number | null | NodeJS.TypedArray | ArrayBufferView { // If not defined, use the default. // For stdin/stdout/stderr, it's pipe. For others, it's ignore. if (item == null) { @@ -1720,6 +1717,11 @@ function nodeToBun(item: string, index: number): string | number | null | NodeJS if (typeof item === "number") { return item; } + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/child_process.js#L1058 + if (typeof item === "object") { + const fd = item.fd; + if (typeof fd === "number") return fd; + } if (isNodeStreamReadable(item) || isNodeStreamWritable(item)) { const fd = streamFdOf(item); if (fd !== undefined) return fd; @@ -1785,7 +1787,7 @@ function getBunStdioFromOptions(stdio) { // overlapped -- same as pipe on Unix based systems // inherit -- 'inherit': equivalent to ['inherit', 'inherit', 'inherit'] or [0, 1, 2] // ignore -- > /dev/null, more or less same as null option for Bun.spawn stdio - // TODO: Stream -- use this stream + // Stream, handle or { fd } object -- its underlying FD is shared with the child // number -- used as FD // null, undefined: Use default value. Not same as ignore, which is Bun.spawn null. // null/undefined: For stdio fds 0, 1, and 2 (in other words, stdin, stdout, and stderr) a pipe is created. For fd 3 and up, the default is 'ignore' @@ -1800,7 +1802,7 @@ function getBunStdioFromOptions(stdio) { // overlapped -> pipe // ignore -> null // inherit -> inherit (stdin/stdout/stderr) - // Stream -> throw err for now + // Stream / handle / { fd } -> fd (a stream without an fd throws) const bunStdio = normalizedStdio.map(nodeToBun); return bunStdio; } @@ -1820,9 +1822,7 @@ function normalizeStdio(stdio): string[] { throw ERR_INVALID_OPT_VALUE("stdio", stdio); } } else if ($isJSArray(stdio)) { - // Validate if each is a valid stdio type - // TODO: Support wrapped types here - + // Each entry is validated and translated by nodeToBun. let processedStdio; if (stdio.length === 0) processedStdio = ["pipe", "pipe", "pipe"]; else if (stdio.length === 1) processedStdio = [stdio[0], "pipe", "pipe"]; diff --git a/test/expectations.txt b/test/expectations.txt index 87c8b99dc0f6..e30a559aeae9 100644 --- a/test/expectations.txt +++ b/test/expectations.txt @@ -35,3 +35,15 @@ # only hold on the distributing model. Passes on macOS and Windows. # (Verified still failing on every Linux lane, build 87834.) [ LINUX ] test/js/node/test/sequential/test-net-listen-shared-ports.js [ FAIL ] # SO_REUSEPORT shared-listener semantics on Linux + +# Both files pass `server._handle` to a detached child, which then serves the +# inherited socket with http.createServer().listen({ fd: 3 }). The spawn half +# works; http.Server#listen ignores `fd` (it listens on a fresh port), so the +# test's request to the inherited port is never answered: the file hangs until +# the runner kills it and leaves the detached child running. Before #39220 the +# spawn threw inside the intermediate process and both files exited 0 without +# testing anything. Remove these entries when http listen({ fd }) lands +# (#36491 / #34659); the Bun-side coverage of the spawn half is in +# test/js/node/child_process/child-process-stdio.test.js. +test/js/node/test/parallel/test-listen-fd-detached.js [ TIMEOUT ] # child's http listen({ fd: 3 }) ignores the fd +test/js/node/test/parallel/test-listen-fd-detached-inherit.js [ TIMEOUT ] # child's http listen({ fd: 3 }) ignores the fd diff --git a/test/js/node/child_process/child-process-stdio.test.js b/test/js/node/child_process/child-process-stdio.test.js index 78b6454f9243..8af19f9e3f49 100644 --- a/test/js/node/child_process/child-process-stdio.test.js +++ b/test/js/node/child_process/child-process-stdio.test.js @@ -1,7 +1,11 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe } from "harness"; -import { execSync, spawn } from "node:child_process"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { execSync, spawn, spawnSync } from "node:child_process"; import { once } from "node:events"; +import { closeSync, openSync, readFileSync } from "node:fs"; +import { open } from "node:fs/promises"; +import { connect, createServer } from "node:net"; +import { join } from "node:path"; const CHILD_PROCESS_FILE = import.meta.dir + "/spawned-child.js"; const OUT_FILE = import.meta.dir + "/stdio-test-out.txt"; @@ -166,3 +170,174 @@ describe("child.stdin", () => { }); }); }); + +// Node shares the descriptor of any stdio entry that has a numeric `fd` +// property (lib/internal/child_process.js, getValidStdio): handle wraps, +// FileHandles, fs/tty streams, plain { fd } objects. Its own test-listen-fd-* +// tests rely on that to hand `server._handle` to a child as fd 3; in Bun that +// handle is the Bun.listen() Listener, which exposes `fd`. Socket descriptors +// cannot be inherited as stdio on Windows (same as Node), so the cases that +// actually inherit a socket are POSIX only. +describe("stdio entries carrying a file descriptor", () => { + async function listeningServer(onConnection) { + const server = createServer(onConnection); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + return { server, port: server.address().port }; + } + + it.skipIf(isWindows)("a listening server's handle is inherited by the child as that fd", async () => { + const { server, port } = await listeningServer(conn => conn.end("hello from parent")); + let child; + try { + child = spawn( + bunExe(), + [ + "-e", + `require("net").createServer(c => c.end("hello from child")).listen({ fd: 3 }, () => console.log("listening"));`, + ], + { env: bunEnv, stdio: ["ignore", "pipe", "inherit", server._handle] }, + ); + // Like Node, a shared descriptor gets no stream in child.stdio. + expect(child.stdio[3]).toBeNull(); + + // Close the parent's copy so the child's inherited descriptor is the only + // thing keeping the port open. + const closed = once(server, "close"); + server.close(); + await closed; + + let out = ""; + await new Promise((resolve, reject) => { + child.stdout.setEncoding("utf8"); + child.stdout.on("data", chunk => { + out += chunk; + if (out.includes("listening")) resolve(); + }); + child.once("error", reject); + child.once("exit", (code, signal) => + reject(new Error(`child exited (${code ?? signal}) before listening: ${out}`)), + ); + }); + + const reply = await new Promise((resolve, reject) => { + const socket = connect({ port, host: "127.0.0.1" }); + let data = ""; + socket.setEncoding("utf8"); + socket.on("data", chunk => (data += chunk)); + socket.once("end", () => resolve(data)); + socket.once("error", reject); + }); + expect(reply).toBe("hello from child"); + } finally { + child?.kill(); + server.close(); + } + }); + + it.skipIf(isWindows)("spawnSync shares listening and connected socket handles with the child", async () => { + // The server leaves the connection open so the client's handle is still + // live while the child is spawned. + const { server, port } = await listeningServer(() => {}); + const client = connect({ port, host: "127.0.0.1" }); + try { + await once(client, "connect"); + const { stdout, status } = spawnSync( + bunExe(), + [ + "-e", + `const { fstatSync } = require("fs"); console.log(JSON.stringify([3, 4].map(fd => fstatSync(fd).isSocket())));`, + ], + { + env: bunEnv, + encoding: "utf8", + stdio: ["ignore", "pipe", "inherit", server._handle, client._handle], + }, + ); + expect(stdout).toBe("[true,true]\n"); + expect(status).toBe(0); + } finally { + client.destroy(); + server.close(); + } + }); + + it.each([ + [ + "a plain { fd } object", + file => { + const fd = openSync(file, "w"); + return { entry: { fd }, close: () => closeSync(fd) }; + }, + ], + [ + "a FileHandle", + async file => { + const handle = await open(file, "w"); + return { entry: handle, close: () => handle.close() }; + }, + ], + ])("%s as stdout makes the child write to that descriptor", async (_label, prepare) => { + using dir = tempDir("stdio-fd-object", {}); + const file = join(String(dir), "stdout.txt"); + const { entry, close } = await prepare(file); + let status; + try { + ({ status } = spawnSync(bunExe(), ["-e", `console.log("written through the shared fd")`], { + env: bunEnv, + stdio: ["ignore", entry, "inherit"], + })); + } finally { + await close(); + } + expect(readFileSync(file, "utf8")).toBe("written through the shared fd\n"); + expect(status).toBe(0); + }); + + it.each([ + ["an object without an fd", {}], + ["an object whose fd is not a number", { fd: "3" }], + ])("%s is still rejected before anything is spawned", (_label, entry) => { + const options = { env: bunEnv, stdio: ["ignore", "ignore", "ignore", entry] }; + expect(() => spawn(bunExe(), ["-e", "0"], options)).toThrow(/stdio/); + expect(() => spawnSync(bunExe(), ["-e", "0"], options)).toThrow(/stdio/); + }); + + it("a handle whose descriptor is already closed is refused, like passing its fd directly", async () => { + const { server } = await listeningServer(() => {}); + const handle = server._handle; + const closed = once(server, "close"); + server.close(); + await closed; + expect(handle.fd).toBe(-1); + + // The handle is only sugar for its `.fd`, so this takes the path a bare -1 + // takes: Bun.spawn refuses the descriptor (spawn() throws, spawnSync() + // returns the error). Node's libuv layer instead spawns the child with + // the slot left closed for the negative errno a closed wrap reports + // (EINVAL only for exactly -1); silently dropping the socket is not worth + // reproducing. + const optionsFor = entry => ({ env: bunEnv, stdio: ["ignore", "ignore", "ignore", entry] }); + const spawnFailure = entry => { + try { + spawn(bunExe(), ["-e", "0"], optionsFor(entry)); + } catch (error) { + return { code: error.code, message: error.message }; + } + }; + const spawnSyncFailure = entry => { + const { error } = spawnSync(bunExe(), ["-e", "0"], optionsFor(entry)); + return error && { code: error.code, message: error.message }; + }; + + const viaHandle = spawnFailure(handle); + expect(viaHandle).toBeDefined(); + expect(viaHandle).toEqual(spawnFailure(handle.fd)); + + const viaHandleSync = spawnSyncFailure(handle); + expect(viaHandleSync).toBeDefined(); + expect(viaHandleSync).toEqual(spawnSyncFailure(handle.fd)); + }); +});