Bun.write(file, file): check the source before opening the destination

Draft
RO
1mo ago
+333-239

Status: draft, waits for #43388. #43388 changes the same block of CopyFile::run_async and covers the directory and closed-fd rows below. After #43388 merges, this PR shrinks to the FIFO source on Linux (the copy syscall choice before the destination open) and the Windows fallback order. Details: https://github.com/oven-sh/bun/pull/38186#issuecomment-5959244842

Problem

  • Bun.write(dest, Bun.file(dir)) rejects with That doesn't work on folders, but dest is 0 bytes afterwards. Same with a directory fd as the source, with a Bun.file(fd) source whose fd is closed (EBADF from fstat), with a FIFO source on Linux (Non-regular files aren't supported yet), and with a Bun.file(dest) destination; a destination that did not exist is left behind as an empty file.
  • Cause: CopyFile::do_open_file in src/runtime/webcore/blob/copy_file.rs opened the source and right after it the destination with O_CREAT | O_WRONLY | O_TRUNC. The source fstat, the directory check and the Linux source-kind dispatch (where the FIFO rejection comes from) all ran later, in run_async.
  • fs.copyFile and cp refuse the same inputs without touching the destination, and Bun.write with a missing source already did, because the source open happened first.

Fix

  • run_async now opens the source (if it is a path), fstats it, rejects a directory, picks the Linux copy syscall, and only then calls open_destination(). The position of that call is the fix; the checks themselves are the existing ones.
  • do_open_file<IOWhich> is split into open_source / open_destination. Its Both mode encoded the old order; the dest-open failure path now closes a Bun-opened source through the existing do_close().
  • pick_copy_syscall is the dispatch block's three conditions, unchanged, returning None for the unsupported case so it can be evaluated before the destination exists; the dispatch block now matches on its result.
  • The macOS clonefile attempt, size clamp, preallocate and copy code are unchanged apart from indentation. The dead if destination is Fd {} block and the redundant system_error.is_some() check in the rewritten region are removed.
  • Windows default path (uv_fs_copyfile) never touches the destination for an unusable source (probed with a directory, a directory fd and a missing source), so it is unchanged.
  • Windows read/write fallback (prepare_read_write_loop) had the same order: under BUN_FEATURE_FLAG_DISABLE_UV_FS_COPYFILE=1 a missing source truncated the destination. It now opens the source first, and closes it before a mkdirp retry because copyfile() re-runs from the top. It still cannot recognise a directory source at open time (GetFileType reports directories as files); with a path source that fallback is only reachable under the test flag, so that is left alone.
  • Verification:
    • test/js/bun/io/bun-write.test.js, Bun.write(dest, Bun.file(unusable source)) leaves the destination alone: directory source for {path, Bun.file} destination x {path, fd} source, directory source with an absent destination, FIFO source (Linux only; macOS copies FIFOs through its read/write fallback), closed fd source (Bun.file(987) in a fresh process, POSIX), missing source. The seven directory/FIFO/closed-fd cases fail on the released build and pass with this change. The missing-source case passes on POSIX either way; it is there for the Windows fallback, where it failed before (the file re-runs itself with the flag on Windows).
    • Windows x64 debug build: the file passes in both modes.
    • cargo check -p bun_runtime for aarch64-apple-darwin and x86_64-unknown-freebsd, cargo clippy -p bun_runtime, cargo fmt --check: clean.
  • Related open PRs, not duplicates: #32859 (copying a file onto itself) moves the truncation to an explicit ftruncate after the source fstat, which would still create an absent destination and still truncate before the Linux FIFO rejection. #36692 would make Linux copy FIFO sources; if it lands, the FIFO case here becomes a successful copy and that test goes away. #31515 (source slice windows) contains the same Windows prepare_read_write_loop reorder as part of its Windows plumbing but leaves the POSIX order alone; whichever lands second has a trivial conflict in that one Windows function.

Background

  • A file-to-file Bun.write is handled by CopyFile (POSIX) and CopyFileWindows, not by the generic blob write path. CopyFile::run_async runs on the work pool and reports through self.system_error: set it and return, and then() rejects the promise with it.
  • Each side is either a path, which Bun opens and closes itself, or a caller-supplied fd (Bun.stdout, Bun.file(fd)), which Bun never opens, truncates or closes. That is why both opens are conditional on *_fd == Fd::INVALID and why do_close() only closes what Bun opened.
  • On Linux the copy uses copy_file_range (regular to regular), splice (pipe to pipe) or sendfile (regular file, char device or socket source), each with a read/write fallback; any other source kind is rejected. The choice depends only on the source's st_mode and the destination store's cached mode, so it can be made before the destination is opened.
  • destination_file_store.mode is 0 unless that BunFile was stat'ed earlier (.size, .lastModified, or an fd store like Bun.stdout); pick_copy_syscall keeps the existing rule that 0 counts as a regular file.
Local note on an unrelated test in the same file

should work when copyFileRange is not available > on large files exceeds the 5 s default per-test timeout in my environment with both the released and the patched build (it moves 256 MB through the disk; CI runs the file with a much larger per-test timeout). #37792 is already about that test.

no test proof · iteration 3 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/io/bun-write.test.js