Fix some edge cases in the `env` param of `spawn`. (#4364)

Close: #4362

2 files changed, 25 insertions, 14 deletions

diff --git a/src/bun.js/api/bun/subprocess.zig b/src/bun.js/api/bun/subprocess.zig
index 6cd6d9016..f2e33cf78 100644
--- a/src/bun.js/api/bun/subprocess.zig
+++ b/src/bun.js/api/bun/subprocess.zig
@@ -1041,6 +1041,7 @@ pub const Subprocess = struct {
 
         var env: [*:null]?[*:0]const u8 = undefined;
 
+        var override_env = false;
         var env_array = std.ArrayListUnmanaged(?[*:0]const u8){
             .items = &.{},
             .capacity = 0,
@@ -1168,6 +1169,7 @@ pub const Subprocess = struct {
                             return .zero;
                         }
 
+                        override_env = true;
                         var object_iter = JSC.JSPropertyIterator(.{
                             .skip_empty_name = false,
                             .include_value = true,
@@ -1257,7 +1259,7 @@ pub const Subprocess = struct {
         }
         defer actions.deinit();
 
-        if (env_array.items.len == 0) {
+        if (!override_env and env_array.items.len == 0) {
             env_array.items = jsc_vm.bundler.env.map.createNullDelimitedEnvMap(allocator) catch |err| return globalThis.handleError(err, "in posix_spawn");
             env_array.capacity = env_array.items.len;
         }
@@ -1302,13 +1304,11 @@ pub const Subprocess = struct {
             return .zero;
         };
 
-        if (env_array.items.len > 0) {
-            env_array.append(allocator, null) catch {
-                globalThis.throw("out of memory", .{});
-                return .zero;
-            };
-            env = @as(@TypeOf(env), @ptrCast(env_array.items.ptr));
-        }
+        env_array.append(allocator, null) catch {
+            globalThis.throw("out of memory", .{});
+            return .zero;
+        };
+        env = @as(@TypeOf(env), @ptrCast(env_array.items.ptr));
 
         const pid = brk: {
             defer {
diff --git a/test/js/node/child_process/child_process.test.ts b/test/js/node/child_process/child_process.test.ts
index 24b3b5e48..8c54a11c5 100644
--- a/test/js/node/child_process/child_process.test.ts
+++ b/test/js/node/child_process/child_process.test.ts
@@ -149,13 +149,24 @@ describe("spawn()", () => {
   });
 
   it("should allow us to set env", async () => {
-    const child = spawn("env", { env: { TEST: "test" } });
-    const result: string = await new Promise(resolve => {
-      child.stdout.on("data", data => {
-        resolve(data.toString());
+    async function getChildEnv(env: any): Promise<string> {
+      const child = spawn("env", { env: env });
+      const result: string = await new Promise(resolve => {
+        let output = "";
+        child.stdout.on("data", data => {
+          output += data;
+        });
+        child.stdout.on("end", () => {
+          resolve(output);
+        });
       });
-    });
-    expect(/TEST\=test/.test(result)).toBe(true);
+      return result;
+    }
+
+    expect(/TEST\=test/.test(await getChildEnv({ TEST: "test" }))).toBe(true);
+    expect(await getChildEnv({})).toStrictEqual("");
+    expect(await getChildEnv(undefined)).not.toStrictEqual("");
+    expect(await getChildEnv(null)).not.toStrictEqual("");
   });
 
   it("should allow explicit setting of argv0", async () => {