From b358ace5058681dde57754dda8e365b3d4f2dfc1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 5 Oct 2026 00:48:25 -0700 Subject: [PATCH] fix(fs): copy macOS descriptor paths positionally Route devfs copyFile sources through bounded positional reads after cloning fails. Preserve source offsets, destination exclusivity and the sequential fallback used by Bun.write and fs.cp. Cover threshold sizes, aliases, clone controls and non-seekable fallback. --- src/runtime/node/node_fs.rs | 66 +++++++++++++++++++++++++--- test/js/bun/io/bun-write.test.js | 61 ++++++++++++++++++++++++++ test/js/node/fs/fs.test.ts | 75 ++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 7 deletions(-) diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 98b1bb51fd0e..466e35cbea02 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -5176,10 +5176,39 @@ impl NodeFS { Ok(()) } + #[cfg(not(windows))] + pub(crate) fn copy_file_using_read_write_loop( + src: &ZStr, + dest: &ZStr, + src_fd: FD, + dest_fd: FD, + stat_size: usize, + wrote: &mut u64, + ) -> Maybe { + Self::copy_file_using_read_write_loop_impl::( + src, dest, src_fd, dest_fd, stat_size, wrote, + ) + } + + #[cfg(target_os = "macos")] + fn copy_file_using_pread_write_loop( + src: &ZStr, + dest: &ZStr, + src_fd: FD, + dest_fd: FD, + stat_size: usize, + wrote: &mut u64, + ) -> Maybe { + // Opening /dev/fd/N shares the caller's offset; path copies must not consume it. + Self::copy_file_using_read_write_loop_impl::( + src, dest, src_fd, dest_fd, stat_size, wrote, + ) + } + // since we use a 64 KB stack buffer, we should not let this function get inlined #[inline(never)] #[cfg(not(windows))] - pub(crate) fn copy_file_using_read_write_loop( + fn copy_file_using_read_write_loop_impl( src: &ZStr, dest: &ZStr, src_fd: FD, @@ -5199,7 +5228,7 @@ impl NodeFS { const STACK_BUF_LEN: usize = 64 * 1024; let mut stack_buf = bun_core::vec::UninitBuf::::uninit(); let mut buf_to_free: Vec = Vec::new(); - // SAFETY: `Syscall::read` is the only writer of `buf`; each iteration reads back only `buf[..amt]`. + // SAFETY: read/pread initialize `buf`; each iteration reads back only `buf[..amt]`. let mut buf: &mut [u8] = unsafe { stack_buf.as_bytes_mut() }; 'maybe_allocate_large_temp_buf: { @@ -5230,7 +5259,12 @@ impl NodeFS { let mut broke = false; 'toplevel: while remain > 0 { let read_len = (buf.len() as u64).min(remain) as usize; - let amt = match Syscall::read(src_fd, &mut buf[..read_len]) { + let read_result = if POSITIONAL { + Syscall::pread(src_fd, &mut buf[..read_len], *wrote as i64) + } else { + Syscall::read(src_fd, &mut buf[..read_len]) + }; + let amt = match read_result { Ok(result) => result, Err(err) => { return Err(if !src.is_empty() { @@ -5269,7 +5303,12 @@ impl NodeFS { } if !broke { 'outer: loop { - let amt = match Syscall::read(src_fd, buf) { + let read_result = if POSITIONAL { + Syscall::pread(src_fd, buf, *wrote as i64) + } else { + Syscall::read(src_fd, buf) + }; + let amt = match read_result { Ok(result) => result, Err(err) => { return Err(if !src.is_empty() { @@ -5391,6 +5430,16 @@ impl NodeFS { }) } + #[cfg(target_os = "macos")] + fn is_devfs_path(path: &ZStr) -> bool { + // Darwin copyfile rejects /dev/fd paths because stat and fstat report different device IDs. + sys::statfs(path).is_ok_and(|fs| { + fs.f_fstypename + .map(|byte| byte as u8) + .starts_with(b"devfs\0") + }) + } + /// https://github.com/libuv/libuv/pull/2233 /// https://github.com/pnpm/pnpm/issues/2761 /// https://github.com/libuv/libuv/pull/2578 @@ -5428,7 +5477,8 @@ impl NodeFS { // 64 KB is about the break-even point for clonefile() to be worth it // at least, on an M1 with an NVME SSD. - if stat_.st_size > 128 * 1024 { + let mut use_read_write = stat_.st_size <= 128 * 1024; + if !use_read_write { if !args.mode.shouldnt_overwrite() { // clonefile() will fail if it already exists let _ = Syscall::unlink(dest); @@ -5443,7 +5493,9 @@ impl NodeFS { let _ = Syscall::chmod(dest, stat_.st_mode as u32); return Ok(()); } - } else { + use_read_write = Self::is_devfs_path(src); + } + if use_read_write { let src_fd = match Syscall::open(src, sys::O::RDONLY, 0o644) { Ok(result) => result, Err(err) => return Err(err.with_path(args.src.slice())), @@ -5467,7 +5519,7 @@ impl NodeFS { Err(err) => return Err(err.with_path(args.dest.slice())), }; - let result = Self::copy_file_using_read_write_loop( + let result = Self::copy_file_using_pread_write_loop( src, dest, src_fd, diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index bcbd9deef142..8e06e9d3d6dc 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -25,6 +25,67 @@ const IS_UV_FS_COPYFILE_DISABLED = (isWindows ? describe : describe.concurrent)("Bun.write", () => { process.platform === "win32" && process.env.BUN_FEATURE_FLAG_DISABLE_UV_FS_COPYFILE === "1"; + // The macOS fcopyfile fallback must still read non-seekable sources sequentially. + it.skipIf(!isMacOS || !Bun.which("cc"))("copies piped stdin to a file when fcopyfile returns EBADF", async () => { + using dir = tempDir("bun-write-fcopyfile-fallback", { + "fault.c": ` + #include + #include + #include + static int fail_fcopyfile(int from, int to, copyfile_state_t state, copyfile_flags_t flags) { + errno = EBADF; + return -1; + } + __attribute__((used)) static struct { + const void *replacement; + const void *original; + } hook __attribute__((section("__DATA,__interpose"))) = { + (const void *)(uintptr_t)&fail_fcopyfile, + (const void *)(uintptr_t)&fcopyfile, + }; + `, + }); + const library = join(String(dir), "fault.dylib"); + await using compiler = Bun.spawn({ + cmd: [Bun.which("cc"), "-dynamiclib", join(String(dir), "fault.c"), "-o", library], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [compileOut, compileErr, compileExit] = await Promise.all([ + compiler.stdout.text(), + compiler.stderr.text(), + compiler.exited, + ]); + expect({ stdout: compileOut, stderr: compileErr, exitCode: compileExit }).toEqual({ + stdout: "", + stderr: "", + exitCode: 0, + }); + + const contents = Buffer.alloc(8193, 0x5a); + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", 'console.log(await Bun.write("output", Bun.stdin))'], + cwd: String(dir), + env: { + ...bunEnv, + DYLD_INSERT_LIBRARIES: [library, bunEnv.DYLD_INSERT_LIBRARIES].filter(Boolean).join(":"), + }, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + proc.stdin.write(contents); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + proc.stdin.end(), + ]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "8193\n", stderr: "", exitCode: 0 }); + expect(await Bun.file(join(String(dir), "output")).bytes()).toEqual(contents); + }); + it("Bun.write blob", async () => { using tmpbase = tempDir("bun-write-blob", {}); await Bun.write( diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index b42366f8a1a3..0b9e643ea53e 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -9,6 +9,7 @@ import { isGlibc, isIntelMacOS, isLinux, + isMacOS, isPosix, isWindows, tempDir, @@ -919,6 +920,80 @@ describe("writeFile with a preallocate-sized buffer", () => { }); describe("copyFileSync", () => { + describe.skipIf(!isMacOS)("/dev/fd sources", () => { + for (const [api, copy] of [ + ["sync", fs.copyFileSync], + ["callback", promisify(fs.copyFile)], + ["promise", fs.promises.copyFile], + ] as const) { + for (const sourceKind of ["descriptor", "symlink"] as const) { + for (const mode of [0, constants.COPYFILE_FICLONE]) { + it.each([131072, 131073, 172832])(`${api} copies a ${sourceKind} at %i bytes (mode ${mode})`, async size => { + const contents = Buffer.alloc(size, 0x5a); + contents[0] = 0x41; + contents[1] = 0x42; + using dir = tempDir("copy-devfd", { + source: contents, + destination: Buffer.alloc(size + 16, 0x7f), + }); + const fd = openSync(join(String(dir), "source"), "r"); + try { + expect(readSync(fd, Buffer.alloc(1), 0, 1, null)).toBe(1); + let source = `/dev/fd/${fd}`; + if (sourceKind === "symlink") { + const link = join(String(dir), "source-link"); + symlinkSync(source, link); + source = link; + } + const destination = join(String(dir), "destination"); + await copy(source, destination, mode); + expect(readFileSync(destination)).toEqual(contents); + expect(readFileSync(join(String(dir), "source"))).toEqual(contents); + expect(fstatSync(fd).size).toBe(size); + const next = Buffer.alloc(1); + expect(readSync(fd, next, 0, 1, null)).toBe(1); + expect(next[0]).toBe(contents[1]); + } finally { + closeSync(fd); + } + }); + } + } + + it(`${api} does not fall back from forced cloning of /dev/fd`, async () => { + using dir = tempDir("copy-devfd-force", { source: Buffer.alloc(172832, 0x5a) }); + const fd = openSync(join(String(dir), "source"), "r"); + try { + const destination = join(String(dir), "destination"); + await expect( + Promise.resolve().then(() => copy(`/dev/fd/${fd}`, destination, constants.COPYFILE_FICLONE_FORCE)), + ).rejects.toThrow(expect.objectContaining({ code: "EINVAL" })); + expect(existsSync(destination)).toBe(false); + expect(fstatSync(fd).size).toBe(172832); + } finally { + closeSync(fd); + } + }); + + it(`${api} preserves an existing destination with COPYFILE_EXCL`, async () => { + using dir = tempDir("copy-devfd-exclusive", { + source: Buffer.alloc(172832, 0x5a), + destination: "keep me", + }); + const fd = openSync(join(String(dir), "source"), "r"); + try { + const destination = join(String(dir), "destination"); + await expect( + Promise.resolve().then(() => copy(`/dev/fd/${fd}`, destination, constants.COPYFILE_EXCL)), + ).rejects.toThrow(expect.objectContaining({ code: "EEXIST" })); + expect(readFileSync(destination, "utf8")).toBe("keep me"); + } finally { + closeSync(fd); + } + }); + } + }); + it("should work for files < 128 KB", () => { const tempdir = tmpdirTestMkdir();