diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index e41579856cdd..44dfb21bd994 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -23,8 +23,6 @@ use bun_sys::windows::libuv; use bun_sys::{self, Fd, FdExt, Mode}; #[cfg(windows)] use bun_sys_jsc::ErrorJsc as _; -#[cfg(any(target_os = "linux", target_os = "android"))] -use core::ffi::c_int; #[cfg(windows)] use core::ffi::c_void; #[cfg(not(windows))] @@ -340,9 +338,7 @@ impl CopyFile { } #[cfg(any(target_os = "linux", target_os = "android"))] - pub(crate) fn do_copy_file_range( - &mut self, - ) -> Result<(), crate::Error> { + pub(crate) fn do_copy_file_range(&mut self) -> Result<(), crate::Error> { use bun_sys::linux; let mut remain: usize = self.max_length as usize; @@ -367,8 +363,6 @@ impl CopyFile { unsafe { *read_len_slot = *total_written_slot as SizeType }; } - let mut has_unset_append = false; - // If they can't use copy_file_range, they probably also can't // use sendfile() or splice() if !bun_sys::copy_file::can_use_copy_file_range_syscall() { @@ -428,32 +422,18 @@ impl CopyFile { return self.fallback_read_write(remain, unknown_size, &mut total_written); } + // EBADF: copy_file_range rejects an O_APPEND destination (`>> log`, an "a" fd). + bun_sys::E::EBADF + if matches!(USE, TryWith::CopyFileRange) + && total_written == 0 + && is_writable_append(dest_fd) => + { + return self.fallback_read_write(remain, unknown_size, &mut total_written); + } + // EINVAL: eCryptfs and other filesystems may not support copy_file_range. // Also returned when the file descriptor is incompatible with the syscall. bun_sys::E::EINVAL => { - if CLEAR_APPEND_IF_INVALID { - if !has_unset_append { - // https://kylelaker.com/2018/08/31/stdout-oappend.html - // make() can set STDOUT / STDERR to O_APPEND - // this messes up sendfile() - has_unset_append = true; - // SAFETY: dest_fd is a valid open fd; raw fcntl(2). - let flags = - unsafe { libc::fcntl(dest_fd.native(), libc::F_GETFL, 0 as c_int) }; - if (flags & bun_sys::O::APPEND) != 0 { - // SAFETY: dest_fd is a valid open fd; raw fcntl(2). - let _ = unsafe { - libc::fcntl( - dest_fd.native(), - libc::F_SETFL, - flags ^ bun_sys::O::APPEND, - ) - }; - continue; - } - } - } - // If the Linux machine doesn't support // copy_file_range or the file descriptor is // incompatible with the chosen syscall, fall back @@ -815,12 +795,7 @@ impl CopyFile { && (bun_sys::S::ISREG(self.destination_file_store.mode as _) || self.destination_file_store.mode == 0) { - if self.destination_file_store.is_atty.unwrap_or(false) { - let _ = self.do_copy_file_range::<{ TryWith::CopyFileRange }, true>(); - } else { - let _ = self.do_copy_file_range::<{ TryWith::CopyFileRange }, false>(); - } - + let _ = self.do_copy_file_range::<{ TryWith::CopyFileRange }>(); self.do_close(); return; } @@ -829,12 +804,7 @@ impl CopyFile { if bun_sys::S::ISFIFO(stat.st_mode as _) && bun_sys::S::ISFIFO(self.destination_file_store.mode as _) { - if self.destination_file_store.is_atty.unwrap_or(false) { - let _ = self.do_copy_file_range::<{ TryWith::Splice }, true>(); - } else { - let _ = self.do_copy_file_range::<{ TryWith::Splice }, false>(); - } - + let _ = self.do_copy_file_range::<{ TryWith::Splice }>(); self.do_close(); return; } @@ -843,12 +813,7 @@ impl CopyFile { || bun_sys::S::ISCHR(stat.st_mode as _) || bun_sys::S::ISSOCK(stat.st_mode as _) { - if self.destination_file_store.is_atty.unwrap_or(false) { - let _ = self.do_copy_file_range::<{ TryWith::Sendfile }, true>(); - } else { - let _ = self.do_copy_file_range::<{ TryWith::Sendfile }, false>(); - } - + let _ = self.do_copy_file_range::<{ TryWith::Sendfile }>(); self.do_close(); return; } @@ -948,6 +913,15 @@ impl CopyFile { } } +#[cfg(any(target_os = "linux", target_os = "android"))] +fn is_writable_append(fd: Fd) -> bool { + use bun_sys::O; + bun_sys::get_fcntl_flags(fd).is_ok_and(|flags| { + let flags = flags as i32; + matches!(flags & O::ACCMODE, O::WRONLY | O::RDWR) && flags & O::APPEND != 0 + }) +} + #[cfg(any(target_os = "linux", target_os = "android"))] fn read_write_fallback( src_fd: Fd, diff --git a/src/sys/lib.rs b/src/sys/lib.rs index 14ee170f6bb6..33f37210ae1c 100644 --- a/src/sys/lib.rs +++ b/src/sys/lib.rs @@ -1189,6 +1189,8 @@ pub mod O { pub const NOCTTY: i32 = libc::O_NOCTTY; #[cfg(windows)] pub const NOCTTY: i32 = 0; + #[cfg(unix)] + pub const ACCMODE: i32 = libc::O_ACCMODE; #[cfg(windows)] pub const ACCMODE: i32 = 3; #[cfg(target_os = "macos")] diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index bcbd9deef142..ac58e060f12f 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -7,6 +7,7 @@ import { exampleSite, gcTick, isASAN, + isLinux, isMacOS, isWindows, tempDir, @@ -451,6 +452,158 @@ const IS_UV_FS_COPYFILE_DISABLED = await Bun.write(Bun.stderr, Bun.file(path.join(import.meta.dir, "hello-world.txt"))); }); + // copy_file_range(2) and sendfile(2) reject a destination whose open file + // description has O_APPEND (EBADF / EINVAL). `>> log`, a GNU make recipe + // redirected to a file and fs.openSync(p, "a") all hand Bun such a + // destination. Windows copies a disk file by path (uv_fs_copyfile), which + // does not append to an fd destination. + describe.skipIf(isWindows)("Bun.write(dest, Bun.file(src)) with an O_APPEND destination", () => { + it("appends a file to stdout redirected with >>", async () => { + using dir = tempDir("bun-write-stdout-append-cfr", { + "src.txt": "file-content\n", + "log.txt": "header\n", + }); + const src = join(String(dir), "src.txt"); + const log = join(String(dir), "log.txt"); + const script = `process.stderr.write(String(await Bun.write(Bun.stdout, Bun.file(${JSON.stringify(src)}))))`; + + await using proc = Bun.spawn({ + cmd: ["sh", "-c", `"$BUN" -e ${JSON.stringify(script)} >> ${JSON.stringify(log)}`], + env: { ...bunEnv, BUN: bunExe() }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout, resolved: stderr, content: fs.readFileSync(log, "utf8") }).toEqual({ + stdout: "", + resolved: "13", + content: "header\nfile-content\n", + }); + expect(exitCode).toBe(0); + }); + + it("appends to an fd opened with 'a' and 'a+'", async () => { + // Above 64 KiB, so the read/write loop takes more than one step. + const body = Buffer.alloc(100_000, "S").toString(); + using dir = tempDir("bun-write-fd-append-cfr", { "src.txt": body, "log.txt": "header\n" }); + const src = join(String(dir), "src.txt"); + const log = join(String(dir), "log.txt"); + const written = []; + for (const flag of ["a", "a+"]) { + const fd = fs.openSync(log, flag); + try { + written.push(await Bun.write(Bun.file(fd), Bun.file(src))); + } finally { + fs.closeSync(fd); + } + } + + const content = fs.readFileSync(log, "utf8"); + expect({ written, size: content.length, intact: content === "header\n" + body + body }).toEqual({ + written: [100_000, 100_000], + size: 7 + 200_000, + intact: true, + }); + }); + + // copy_file_range also reports a destination that is not writable as + // EBADF. That stays an error, and nothing is read from the source fd. + it.skipIf(!isLinux)("rejects a destination fd that is not writable and leaves the source fd unread", async () => { + using dir = tempDir("bun-write-fd-not-writable", { + "src.txt": "payload\n", + "log.txt": "header\n", + "write.mjs": ` + import fs from "node:fs"; + const results = []; + for (const flags of [fs.constants.O_RDONLY, fs.constants.O_RDONLY | fs.constants.O_APPEND]) { + const src = fs.openSync("src.txt", "r"); + const dest = fs.openSync("log.txt", flags); + const code = await Bun.write(Bun.file(dest), Bun.file(src)).then(() => "resolved", e => e.code); + results.push({ code, unread: fs.readSync(src, Buffer.alloc(64)) }); + fs.closeSync(dest); + fs.closeSync(src); + } + console.log(JSON.stringify(results)); + `, + }); + // In a child: another test in this process can switch the kernel copy off. + await using proc = Bun.spawn({ + cmd: [bunExe(), "write.mjs"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ + results: stdout.trim(), + stderr, + content: fs.readFileSync(join(String(dir), "log.txt"), "utf8"), + }).toEqual({ + results: JSON.stringify([ + { code: "EBADF", unread: 8 }, + { code: "EBADF", unread: 8 }, + ]), + stderr: "", + content: "header\n", + }); + expect(exitCode).toBe(0); + }); + + // GNU make 4.0 to 4.3 sets O_APPEND on a recipe's stdout when it is a + // terminal. The description is shared with the parent shell, so Bun must + // not clear the flag. Then fd 1 becomes a log opened for append, as in a + // process that detaches from its terminal: the copy must land at the end. + it.skipIf(!isLinux)("keeps O_APPEND on a tty description and on a log that takes its place", async () => { + using dir = tempDir("bun-write-tty-append-flag", { + "src.txt": "payload\n", + "log.txt": "header-header\n", + "write.mjs": ` + import fs from "node:fs"; + const flags = () => /^flags:\\s*(\\d+)/m.exec(fs.readFileSync("/proc/self/fdinfo/1", "utf8"))[1]; + const stdout = Bun.stdout; + const before = flags(); + await Bun.write(stdout, Bun.file("src.txt")); + process.stderr.write("before=" + before + " after=" + flags() + "\\n"); + + fs.closeSync(1); + const fd = fs.openSync("log.txt", "a"); + const written = await Bun.write(stdout, Bun.file("src.txt")); + process.stderr.write(JSON.stringify({ fd, written }) + "\\n"); + `, + }); + let output = ""; + const { promise: ptyClosed, resolve } = Promise.withResolvers(); + await using proc = Bun.spawn({ + cmd: ["sh", "-c", `exec 1>>/dev/tty; exec "$BUN" write.mjs`], + cwd: String(dir), + env: { ...bunEnv, BUN: bunExe() }, + terminal: { + data(_, chunk) { + output += chunk.toString(); + }, + exit: () => resolve(), + }, + }); + const exitCode = await proc.exited; + await ptyClosed; + + const [, before] = /before=(\d+) after=\d+/.exec(output) ?? []; + expect({ + output: output.replace(/\r/g, ""), + appendBefore: Number(`0o${before}`) & 0o2000, + log: fs.readFileSync(join(String(dir), "log.txt"), "utf8"), + }).toEqual({ + output: `payload\nbefore=${before} after=${before}\n{"fd":1,"written":8}\n`, + appendBefore: 0o2000, + log: "header-header\npayload\n", + }); + expect(exitCode).toBe(0); + }); + }); + // macOS fcopyfile(COPYFILE_DATA) rewrites dst from offset 0, and the // slice trim on macOS/FreeBSD (and the Linux read/write fallback) was // ftruncate(dst, N); both destroy bytes in a file the caller already had