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();