Bun.write: compare the destination with the source before the truncating open

src/runtime/webcore/blob/copy_file.rs

@@ -214,11 +214,13 @@ impl CopyFile {
         }
     }
 
-    pub(crate) fn do_open_file<const WHICH: IOWhich>(&mut self) -> Result<(), crate::Error> {
+    /// Opens nothing, and leaves `destination_fd` invalid, when the destination path names `source`'s file.
+    pub(crate) fn do_open_file<const WHICH: IOWhich>(
+        &mut self,
+        source: Option<&Stat>,
+    ) -> Result<(), crate::Error> {
         let mut path_buf1 = bun_paths::path_buffer_pool::get();
-        // open source file first
-        // if it fails, we don't want the extra destination file hanging out
-        if matches!(WHICH, IOWhich::Both | IOWhich::Source) {
+        if matches!(WHICH, IOWhich::Source) {
             self.source_fd = match bun_sys::open(
                 self.source_file_store
                     .pathlike
@@ -246,7 +248,7 @@ impl CopyFile {
             };
         }
 
-        if matches!(WHICH, IOWhich::Both | IOWhich::Destination) {
+        if matches!(WHICH, IOWhich::Destination) {
             loop {
                 // detach `dest` lifetime from `self` (borrowck) — slice_z
                 // copies into path_buf1, so build the ZStr directly from the buffer.
@@ -259,6 +261,12 @@ impl CopyFile {
                 };
                 // SAFETY: path_buf1[dest_len] == 0 written above.
                 let dest: &bun_core::ZStr = bun_core::ZStr::from_buf(&path_buf1[..], dest_len);
+                // Before every open: a parent directory from the last pass can make the path name the source.
+                if source
+                    .is_some_and(|source| Self::is_same_file(source, &bun_sys::stat_no_path(dest)))
+                {
+                    return Ok(());
+                }
                 let mode = self.destination_mode.unwrap_or(node_fs::DEFAULT_PERMISSION);
                 match bun_sys::open(dest, OPEN_DESTINATION_FLAGS, mode) {
                     bun_sys::Result::Ok(result) => {
@@ -277,20 +285,11 @@ impl CopyFile {
                         match blob::mkdir_if_not_exists(self, &errno, dest, dest.as_bytes()) {
                             Retry::Continue => continue,
                             Retry::Fail => {
-                                if matches!(WHICH, IOWhich::Both) {
-                                    self.source_fd.close();
-                                    self.source_fd = Fd::INVALID;
-                                }
                                 return Err(bun_errno::from_errno(errno.errno as i32).into());
                             }
                             Retry::No => {}
                         }
 
-                        if matches!(WHICH, IOWhich::Both) {
-                            self.source_fd.close();
-                            self.source_fd = Fd::INVALID;
-                        }
-
                         self.system_error = Some(
                             errno
                                 .with_path(self.destination_file_store.pathlike.path().slice())
@@ -595,12 +594,14 @@ impl CopyFile {
         Ok(())
     }
 
-    pub(crate) fn run_async(&mut self) {
-        #[cfg(target_os = "macos")]
-        let mut stat_: Option<Stat> = None;
-        #[cfg(not(target_os = "macos"))]
-        let stat_: Option<Stat> = None;
+    /// Whether `dest` is the file that `source` describes.
+    fn is_same_file(source: &Stat, dest: &bun_sys::Result<Stat>) -> bool {
+        // An inode number of 0 means the file system reports no identity.
+        matches!(dest, bun_sys::Result::Ok(dest)
+            if source.st_ino != 0 && dest.st_dev == source.st_dev && dest.st_ino == source.st_ino)
+    }
 
+    pub(crate) fn run_async(&mut self) {
         if let PathOrFileDescriptor::Fd(fd) = &self.destination_file_store.pathlike {
             self.destination_fd = *fd;
         }
@@ -609,8 +610,8 @@ impl CopyFile {
             self.source_fd = *fd;
         }
 
-        // Do we need to open both files?
-        if self.destination_fd == Fd::INVALID && self.source_fd == Fd::INVALID {
+        // The source comes first: the destination is compared with it before it is opened.
+        if self.source_fd == Fd::INVALID {
             // First, we attempt to clonefile() on macOS
             // This is the fastest way to copy a file.
             #[cfg(target_os = "macos")]
@@ -630,15 +631,13 @@ impl CopyFile {
 
                         // stat the output file, make sure it:
                         // 1. Exists
-                        match bun_sys::stat(
+                        let stat_size = match bun_sys::stat(
                             self.source_file_store
                                 .pathlike
                                 .path()
                                 .slice_z(&mut path_buf),
                         ) {
                             bun_sys::Result::Ok(result) => {
-                                stat_ = Some(result);
-
                                 if bun_sys::S::ISDIR(result.st_mode as u32) {
                                     self.system_error = Some(unsupported_directory_error());
                                     return;
@@ -647,17 +646,18 @@ impl CopyFile {
                                 if !bun_sys::S::ISREG(result.st_mode as u32) {
                                     break 'do_clonefile;
                                 }
+
+                                result.st_size
                             }
                             bun_sys::Result::Err(err) => {
                                 // If we can't stat it, we also can't copy it.
                                 self.system_error = Some(err.to_system_error());
                                 return;
                             }
-                        }
+                        };
 
                         match self.do_clonefile() {
                             Ok(()) => {
-                                let stat_size = stat_.unwrap().st_size;
                                 if self.max_length != MAX_SIZE
                                     && self.max_length
                                         < SizeType::try_from(stat_size).expect("int cast")
@@ -710,21 +710,7 @@ impl CopyFile {
                 }
             }
 
-            if self.do_open_file::<{ IOWhich::Both }>().is_err() {
-                return;
-            }
-            // Do we need to open only one file?
-        } else if self.destination_fd == Fd::INVALID {
-            self.source_fd = self.source_file_store.pathlike.fd();
-
-            if self.do_open_file::<{ IOWhich::Destination }>().is_err() {
-                return;
-            }
-            // Do we need to open only one file?
-        } else if self.source_fd == Fd::INVALID {
-            self.destination_fd = self.destination_file_store.pathlike.fd();
-
-            if self.do_open_file::<{ IOWhich::Source }>().is_err() {
+            if self.do_open_file::<{ IOWhich::Source }>(None).is_err() {
                 return;
             }
         }
@@ -733,7 +719,6 @@ impl CopyFile {
             return;
         }
 
-        debug_assert!(self.destination_fd.is_valid());
         debug_assert!(self.source_fd.is_valid());
 
         if matches!(
@@ -743,16 +728,14 @@ impl CopyFile {
             // nothing to do for the Fd case
         }
 
-        let stat: Stat = match stat_ {
-            Some(s) => s,
-            None => match bun_sys::fstat(self.source_fd) {
-                bun_sys::Result::Ok(result) => result,
-                bun_sys::Result::Err(err) => {
-                    self.do_close();
-                    self.system_error = Some(err.to_system_error());
-                    return;
-                }
-            },
+        // From the opened fd: the source path can name another file by now.
+        let stat: Stat = match bun_sys::fstat(self.source_fd) {
+            bun_sys::Result::Ok(result) => result,
+            bun_sys::Result::Err(err) => {
+                self.system_error = Some(err.to_system_error());
+                self.do_close();
+                return;
+            }
         };
 
         if bun_sys::S::ISDIR(stat.st_mode as _) {
@@ -761,6 +744,32 @@ impl CopyFile {
             return;
         }
 
+        if self.destination_fd == Fd::INVALID {
+            // Only a regular file has an identity to compare.
+            let source = bun_sys::S::ISREG(stat.st_mode as _).then_some(&stat);
+            if self
+                .do_open_file::<{ IOWhich::Destination }>(source)
+                .is_err()
+            {
+                self.do_close();
+                return;
+            }
+
+            if self.destination_fd == Fd::INVALID {
+                // A copy of a file onto itself: nothing to empty or write.
+                self.read_len = SizeType::try_from(stat.st_size).expect("int cast");
+                if let Some(mode) = self.destination_mode {
+                    if let bun_sys::Result::Err(err) = bun_sys::fchmod(self.source_fd, mode) {
+                        self.system_error = Some(err.to_system_error());
+                    }
+                }
+                self.do_close();
+                return;
+            }
+        }
+
+        debug_assert!(self.destination_fd.is_valid());
+
         // BSD fstat on a pipe reports bytes currently buffered in st_size;
         // only a regular-file st_size is a length.
         if stat.st_size != 0 && bun_sys::S::ISREG(stat.st_mode as _) {
@@ -781,6 +790,8 @@ impl CopyFile {
                 )
                 && self.max_length > PREALLOCATE_LENGTH
                 && self.max_length != MAX_SIZE
+                // A rename after the compare can make the source the destination: do not zero-fill it.
+                && !Self::is_same_file(&stat, &bun_sys::fstat(self.destination_fd))
             {
                 let _ = bun_sys::preallocate_file(
                     self.destination_fd.native(),

src/sys/lib.rs

@@ -2026,19 +2026,22 @@ mod posix_impl {
         }
     }
     pub fn stat(path: &ZStr) -> Maybe<Stat> {
+        stat_no_path(path).map_err(|e| e.with_path(path.as_bytes()))
+    }
+    /// [`stat`] with an error that has no `path`: a failure does not allocate.
+    pub fn stat_no_path(path: &ZStr) -> Maybe<Stat> {
         #[cfg(any(target_os = "linux", target_os = "android"))]
         {
             return super::linux_syscall::stat(path)
-                .map_err(|e| Error::from_code_int(e, Tag::stat).with_path(path.as_bytes()));
+                .map_err(|e| Error::from_code_int(e, Tag::stat));
         }
         #[cfg(not(any(target_os = "linux", target_os = "android")))]
         {
             let mut st = core::mem::MaybeUninit::<Stat>::uninit();
-            check_p!(
+            check!(
                 // SAFETY: `path` is NUL-terminated; `st` is a valid out-param.
                 unsafe { libc::stat(path.as_ptr(), st.as_mut_ptr()) },
-                Tag::stat,
-                path
+                Tag::stat
             );
             // SAFETY: rc == 0 ⇒ kernel populated `st`.
             Ok(unsafe { st.assume_init() })