Bun.write: copy a file to an O_APPEND destination fd with the read/write loop

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<const USE: TryWith, const CLEAR_APPEND_IF_INVALID: bool>(
-        &mut self,
-    ) -> Result<(), crate::Error> {
+    pub(crate) fn do_copy_file_range<const USE: TryWith>(&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,

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")]

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