Fix error when null/undefined is passed to stdio in spawn/spawnSync (o…

…ven-sh#8110) * Fix error when null/undefined is passed to stdio in spawn/spawnSync * Change conditional
AdamBear · Jan 12, 2024 · 31db26b · 31db26b
1 parent f567528
commit 31db26b
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 12 deletions.
diff --git a/src/js/node/child_process.js b/src/js/node/child_process.js
@@ -571,18 +571,22 @@ function spawnSync(file, args, options) {
     }
   }
 
-  const { stdout, stderr, success, exitCode } = Bun.spawnSync({
+  const {
+    stdout = null,
+    stderr = null,
+    success,
+    exitCode,
+  } = Bun.spawnSync({
     cmd: options.args,
     env: options.env || undefined,
     cwd: options.cwd || undefined,
-    stdin: bunStdio[0],
-    stdout: bunStdio[1],
-    stderr: bunStdio[2],
+    stdio: bunStdio,
   });
 
   const result = {
     signal: null,
     status: exitCode,
+    // TODO: Need to expose extra pipes from Bun.spawnSync to child_process
     output: [null, stdout, stderr],
   };
 
@@ -1101,7 +1105,7 @@ class ChildProcess extends EventEmitter {
   #stdioOptions;
 
   #createStdioObject() {
-    let result = {};
+    let result = new Array(this.#stdioOptions.length);
     for (let i = 0; i < this.#stdioOptions.length; i++) {
       const element = this.#stdioOptions[i];
       if (element !== "pipe") {
@@ -1123,7 +1127,7 @@ class ChildProcess extends EventEmitter {
           continue;
       }
     }
-    return ObjectCreate.$call(null, result);
+    return result;
   }
 
   get stdin() {
@@ -1327,16 +1331,22 @@ const nodeToBunLookup = {
   ipc: "ipc",
 };
 
-function nodeToBun(item) {
+function nodeToBun(item, index) {
+  // If not defined, use the default.
+  // For stdin/stdout/stderr, it's pipe. For others, it's ignore.
+  if (item == null) {
+    return index > 3 ? "ignore" : "pipe";
+  }
   // If inherit and we are referencing stdin/stdout/stderr index,
   // we can get the fd from the ReadStream for the corresponding stdio
   if (typeof item === "number") {
     return item;
-  } else {
-    const result = nodeToBunLookup[item];
-    if (result === undefined) throw new Error(`Invalid stdio option "${item}"`);
-    return result;
   }
+  const result = nodeToBunLookup[item];
+  if (result === undefined) {
+    throw new Error(`Invalid stdio option "${item}"`);
+  }
+  return result;
 }
 
 function fdToStdioName(fd) {
@@ -1376,7 +1386,7 @@ function getBunStdioFromOptions(stdio) {
   // ignore -> null
   // inherit -> inherit (stdin/stdout/stderr)
   // Stream -> throw err for now
-  const bunStdio = normalizedStdio.map(item => nodeToBun(item));
+  const bunStdio = normalizedStdio.map(nodeToBun);
   return bunStdio;
 }
 

diff --git a/test/regression/issue/08095.test.ts b/test/regression/issue/08095.test.ts
@@ -0,0 +1,19 @@
+import { test, expect } from "bun:test";
+import { bunExe } from "harness";
+import { Readable } from "node:stream";
+import { spawn, spawnSync } from "node:child_process";
+
+test.each([null, undefined])(`spawnSync can pass %p as option to stdio`, input => {
+  const { stdout, stderr, output } = spawnSync(bunExe(), { stdio: [input, input, input] });
+  expect(stdout).toBeInstanceOf(Buffer);
+  expect(stderr).toBeInstanceOf(Buffer);
+  expect(output).toStrictEqual([null, stdout, stderr]);
+});
+
+test.each([null, undefined])(`spawn can pass %p as option to stdio`, input => {
+  const { stdout, stderr, stdio } = spawn(bunExe(), { stdio: [input, input, input] });
+  expect(stdout).toBeInstanceOf(Readable);
+  expect(stderr).toBeInstanceOf(Readable);
+  expect(stdio).toBeArrayOfSize(3);
+  expect(stdio.slice(1)).toStrictEqual([stdout, stderr]);
+});