diff --git a/src/sys/lib.rs b/src/sys/lib.rs index cf908fe8fec6..922205c11b21 100644 --- a/src/sys/lib.rs +++ b/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 { - // 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, diff --git a/test/js/bun/http/fetch-file-upload.test.ts b/test/js/bun/http/fetch-file-upload.test.ts index 38ea1ae6c323..c2b40025c85f 100644 --- a/test/js/bun/http/fetch-file-upload.test.ts +++ b/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 diff --git a/test/js/bun/util/bun-file-fd-read.test.ts b/test/js/bun/util/bun-file-fd-read.test.ts index fe1e53191e31..ea04b749be4d 100644 --- a/test/js/bun/util/bun-file-fd-read.test.ts +++ b/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); + } + }); +});