sys(windows): dup() of a descriptor that is not open fails with EBADF instead of returning a process handle

src/sys/lib.rs

@@ -3956,13 +3956,22 @@ mod windows_impl {
         super::openat_windows_a(dir, path.as_bytes(), flags, mode)
     }
     pub fn dup(fd: Fd) -> Maybe<Fd> {
-        // DuplicateHandle on the underlying HANDLE.
+        // DuplicateHandle on the underlying HANDLE. A uv fd that is not open
+        // (and `Fd::INVALID`) decodes to INVALID_HANDLE_VALUE, which is the
+        // same value as the GetCurrentProcess() pseudo handle: DuplicateHandle
+        // accepts it and hands back a handle to this process instead of
+        // failing, so it has to be rejected before the call.
+        let source = fd.native();
+        if source == w::INVALID_HANDLE_VALUE {
+            return Err(Error::new(E::EBADF, Tag::dup).with_fd(fd));
+        }
         let process = w::kernel32::GetCurrentProcess();
         let mut target: w::HANDLE = core::ptr::null_mut();
+        // SAFETY: FFI; `target` is valid for the write.
         let out = unsafe {
             w::kernel32::DuplicateHandle(
                 process,
-                fd.native() as w::HANDLE,
+                source,
                 process,
                 &mut target,
                 0,

test/js/bun/http/fetch-file-upload.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import { isBroken, isWindows, tempDir, withoutAggressiveGC } from "harness";
+import { bunEnv, bunExe, isBroken, isWindows, tempDir, withoutAggressiveGC } from "harness";
 import { tmpdir } from "os";
 import { join } from "path";
 
@@ -222,6 +222,48 @@ describe("Bun.file().slice() upload sends the slice's Content-Length", () => {
   });
 });
 
+// An fd-backed body is dup()ed before it is read. On Windows a descriptor that
+// is not open used to be reported as EMFILE: it maps to INVALID_HANDLE_VALUE,
+// which DuplicateHandle accepts as the current-process pseudo handle.
+describe.concurrent("Bun.file(fd) body whose descriptor is not open", () => {
+  const dupSyscall = isWindows ? "dup" : "fcntl";
+
+  test("descriptor that was never opened rejects with EBADF", async () => {
+    const fd = 1 << 20;
+    const promise = fetch("http://127.0.0.1:1/", { method: "POST", body: Bun.file(fd) });
+    // The body fails before anything is connected.
+    expect(Bun.peek.status(promise)).toBe("rejected");
+    await expect(promise).rejects.toMatchObject({ code: "EBADF", syscall: dupSyscall, fd });
+  });
+
+  test("descriptor that was closed rejects with EBADF", async () => {
+    // A fresh process, so nothing can reuse the number between close and fetch.
+    using dir = tempDir("fetch-closed-fd-body", { "upload.txt": "hello" });
+    await using proc = Bun.spawn({
+      cmd: [
+        bunExe(),
+        "-e",
+        `
+        const { openSync, closeSync } = require("fs");
+        const fd = openSync("upload.txt", "r");
+        closeSync(fd);
+        fetch("http://127.0.0.1:1/", { method: "POST", body: Bun.file(fd) }).then(
+          () => console.log(JSON.stringify({ resolved: true })),
+          err => console.log(JSON.stringify({ code: err.code, syscall: err.syscall, fdMatches: err.fd === fd })),
+        );
+        `,
+      ],
+      cwd: String(dir),
+      env: bunEnv,
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(stderr).toBe("");
+    expect(JSON.parse(stdout)).toEqual({ code: "EBADF", syscall: dupSyscall, fdMatches: true });
+    expect(exitCode).toBe(0);
+  });
+});
+
 test("missing file throws the expected error", async () => {
   Bun.gc(true);
   // Run this 1000 times to check for GC bugs

test/js/bun/util/bun-file-fd-read.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, test } from "bun:test";
-import { closeSync, openSync } from "fs";
-import { isWindows, tempDir } from "harness";
+import { closeSync, openSync, readFileSync } from "fs";
+import { bunEnv, bunExe, isWindows, tempDir } from "harness";
 import { join } from "path";
 
 // Reading a Bun.file() backed by a file descriptor goes through
@@ -50,3 +50,62 @@ describe.skipIf(isWindows)("Bun.file(fd) read", () => {
     expect((await withFd(path, fd => Bun.file(fd).arrayBuffer())).byteLength).toBe(0);
   });
 });
+
+// stream() dup()s the descriptor when the stream starts. On Windows a
+// descriptor that is not open used to be reported as EMFILE: it maps to
+// INVALID_HANDLE_VALUE, which DuplicateHandle accepts as the current-process
+// pseudo handle.
+describe.concurrent("Bun.file(fd).stream() on a descriptor that is not open", () => {
+  const dupSyscall = isWindows ? "dup" : "fcntl";
+
+  test("descriptor that was never opened fails with EBADF", async () => {
+    const fd = 1 << 20;
+    let error: unknown;
+    try {
+      await Bun.file(fd).stream().text();
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toMatchObject({ code: "EBADF", syscall: dupSyscall, fd });
+  });
+
+  test("descriptor that was closed fails with EBADF", async () => {
+    // A fresh process, so nothing can reuse the number between close and stream().
+    using dir = tempDir("bun-file-closed-fd-stream", { "fd-closed.txt": "hello" });
+    await using proc = Bun.spawn({
+      cmd: [
+        bunExe(),
+        "-e",
+        `
+        const { openSync, closeSync } = require("fs");
+        const fd = openSync("fd-closed.txt", "r");
+        closeSync(fd);
+        try {
+          await Bun.file(fd).stream().text();
+          console.log(JSON.stringify({ resolved: true }));
+        } catch (err) {
+          console.log(JSON.stringify({ code: err.code, syscall: err.syscall, fdMatches: err.fd === fd }));
+        }
+        `,
+      ],
+      cwd: String(dir),
+      env: bunEnv,
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(stderr).toBe("");
+    expect(JSON.parse(stdout)).toEqual({ code: "EBADF", syscall: dupSyscall, fdMatches: true });
+    expect(exitCode).toBe(0);
+  });
+
+  test("descriptor that is open still streams", async () => {
+    // This file rather than a tempDir: the stream closes its copy of the
+    // descriptor asynchronously, which would race the directory removal.
+    const fd = openSync(import.meta.path, "r");
+    try {
+      expect(await Bun.file(fd).stream().text()).toBe(readFileSync(import.meta.path, "utf8"));
+    } finally {
+      closeSync(fd);
+    }
+  });
+});