cp: clean existing file when copy from stream (#8149)

* cp: clean existing file when copy from stream

* cp: remove outdated comments

---------

Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>

src/uu/cp/src/cp.rs

@@ -2545,12 +2545,7 @@ fn copy_file(
     #[cfg(not(unix))]
     let source_is_fifo = false;
 
-    #[cfg(unix)]
+    let source_is_stream = is_stream(&source_metadata);
-    let source_is_stream = source_is_fifo
-        || source_metadata.file_type().is_char_device()
-        || source_metadata.file_type().is_block_device();
-    #[cfg(not(unix))]
-    let source_is_stream = false;
 
     let performed_action = handle_copy_mode(
         source,
@@ -2620,6 +2615,19 @@ fn copy_file(
     Ok(())
 }
 
+fn is_stream(metadata: &Metadata) -> bool {
+    #[cfg(unix)]
+    {
+        let file_type = metadata.file_type();
+        file_type.is_fifo() || file_type.is_char_device() || file_type.is_block_device()
+    }
+    #[cfg(not(unix))]
+    {
+        let _ = metadata;
+        false
+    }
+}
+
 #[cfg(unix)]
 fn handle_no_preserve_mode(options: &Options, org_mode: u32) -> u32 {
     let (is_preserve_mode, is_explicit_no_preserve_mode) = options.preserve_mode();
@@ -2700,8 +2708,6 @@ fn copy_helper(
             options.sparse_mode,
             context,
             #[cfg(unix)]
-            source_is_fifo,
-            #[cfg(unix)]
             source_is_stream,
         )?;
 

src/uu/cp/src/platform/linux.rs

@@ -18,6 +18,7 @@ use uucore::mode::get_umask;
 
 use crate::{
     CopyDebug, CopyResult, CpError, OffloadReflinkDebug, ReflinkMode, SparseDebug, SparseMode,
+    is_stream,
 };
 
 /// The fallback behavior for [`clone`] on failed system call.
@@ -220,9 +221,8 @@ fn check_dest_is_fifo(dest: &Path) -> bool {
     }
 }
 
-/// Copy the contents of a stream from `source` to `dest`. The `if_fifo` argument is used to
+/// Copy the contents of a stream from `source` to `dest`.
-/// determine if we need to modify the file's attributes before and after copying.
+fn copy_stream<P>(source: P, dest: P) -> std::io::Result<u64>
-fn copy_stream<P>(source: P, dest: P, is_fifo: bool) -> std::io::Result<u64>
 where
     P: AsRef<Path>,
 {
@@ -252,29 +252,25 @@ where
         .mode(mode)
         .open(&dest)?;
 
+    let dest_is_stream = is_stream(&dst_file.metadata()?);
+    if !dest_is_stream {
+        // `copy_stream` doesn't clear the dest file, if dest is not a stream, we should clear it manually.
+        dst_file.set_len(0)?;
+    }
+
     let num_bytes_copied = buf_copy::copy_stream(&mut src_file, &mut dst_file)
         .map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))?;
 
-    if is_fifo {
-        dst_file.set_permissions(src_file.metadata()?.permissions())?;
-    }
-
     Ok(num_bytes_copied)
 }
 
 /// Copies `source` to `dest` using copy-on-write if possible.
-///
-/// The `source_is_fifo` flag must be set to `true` if and only if
-/// `source` is a FIFO (also known as a named pipe). In this case,
-/// copy-on-write is not possible, so we copy the contents using
-/// [`std::io::copy`].
 pub(crate) fn copy_on_write(
     source: &Path,
     dest: &Path,
     reflink_mode: ReflinkMode,
     sparse_mode: SparseMode,
     context: &str,
-    source_is_fifo: bool,
     source_is_stream: bool,
 ) -> CopyResult<CopyDebug> {
     let mut copy_debug = CopyDebug {
@@ -289,7 +285,7 @@ pub(crate) fn copy_on_write(
             copy_debug.reflink = OffloadReflinkDebug::No;
             if source_is_stream {
                 copy_debug.offload = OffloadReflinkDebug::Avoided;
-                copy_stream(source, dest, source_is_fifo).map(|_| ())
+                copy_stream(source, dest).map(|_| ())
             } else {
                 let mut copy_method = CopyMethod::Default;
                 let result = handle_reflink_never_sparse_always(source, dest);
@@ -309,7 +305,7 @@ pub(crate) fn copy_on_write(
 
             if source_is_stream {
                 copy_debug.offload = OffloadReflinkDebug::Avoided;
-                copy_stream(source, dest, source_is_fifo).map(|_| ())
+                copy_stream(source, dest).map(|_| ())
             } else {
                 let result = handle_reflink_never_sparse_never(source);
                 if let Ok(debug) = result {
@@ -323,7 +319,7 @@ pub(crate) fn copy_on_write(
 
             if source_is_stream {
                 copy_debug.offload = OffloadReflinkDebug::Avoided;
-                copy_stream(source, dest, source_is_fifo).map(|_| ())
+                copy_stream(source, dest).map(|_| ())
             } else {
                 let mut copy_method = CopyMethod::Default;
                 let result = handle_reflink_never_sparse_auto(source, dest);
@@ -343,7 +339,7 @@ pub(crate) fn copy_on_write(
             // SparseMode::Always
             if source_is_stream {
                 copy_debug.offload = OffloadReflinkDebug::Avoided;
-                copy_stream(source, dest, source_is_fifo).map(|_| ())
+                copy_stream(source, dest).map(|_| ())
             } else {
                 let mut copy_method = CopyMethod::Default;
                 let result = handle_reflink_auto_sparse_always(source, dest);
@@ -363,7 +359,7 @@ pub(crate) fn copy_on_write(
             copy_debug.reflink = OffloadReflinkDebug::No;
             if source_is_stream {
                 copy_debug.offload = OffloadReflinkDebug::Avoided;
-                copy_stream(source, dest, source_is_fifo).map(|_| ())
+                copy_stream(source, dest).map(|_| ())
             } else {
                 let result = handle_reflink_auto_sparse_never(source);
                 if let Ok(debug) = result {
@@ -376,7 +372,7 @@ pub(crate) fn copy_on_write(
         (ReflinkMode::Auto, SparseMode::Auto) => {
             if source_is_stream {
                 copy_debug.offload = OffloadReflinkDebug::Unsupported;
-                copy_stream(source, dest, source_is_fifo).map(|_| ())
+                copy_stream(source, dest).map(|_| ())
             } else {
                 let mut copy_method = CopyMethod::Default;
                 let result = handle_reflink_auto_sparse_auto(source, dest);

src/uu/cp/src/platform/macos.rs

@@ -16,19 +16,16 @@ use uucore::mode::get_umask;
 
 use crate::{
     CopyDebug, CopyResult, CpError, OffloadReflinkDebug, ReflinkMode, SparseDebug, SparseMode,
+    is_stream,
 };
 
 /// Copies `source` to `dest` using copy-on-write if possible.
-///
-/// The `source_is_fifo` flag must be set to `true` if and only if
-/// `source` is a FIFO (also known as a named pipe).
 pub(crate) fn copy_on_write(
     source: &Path,
     dest: &Path,
     reflink_mode: ReflinkMode,
     sparse_mode: SparseMode,
     context: &str,
-    source_is_fifo: bool,
     source_is_stream: bool,
 ) -> CopyResult<CopyDebug> {
     if sparse_mode != SparseMode::Auto {
@@ -110,14 +107,15 @@ pub(crate) fn copy_on_write(
                         .mode(mode)
                         .open(dest)?;
 
-                    let context = buf_copy::copy_stream(&mut src_file, &mut dst_file)
+                    let dest_is_stream = is_stream(&dst_file.metadata()?);
-                        .map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))
+                    if !dest_is_stream {
-                        .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
+                        // `copy_stream` doesn't clear the dest file, if dest is not a stream, we should clear it manually.
-
+                        dst_file.set_len(0)?;
-                    if source_is_fifo {
-                        dst_file.set_permissions(src_file.metadata()?.permissions())?;
                     }
-                    context
+
+                    buf_copy::copy_stream(&mut src_file, &mut dst_file)
+                        .map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))
+                        .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?
                 } else {
                     fs::copy(source, dest)
                         .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?

src/uu/cp/src/platform/other_unix.rs

@@ -13,6 +13,7 @@ use uucore::mode::get_umask;
 
 use crate::{
     CopyDebug, CopyResult, CpError, OffloadReflinkDebug, ReflinkMode, SparseDebug, SparseMode,
+    is_stream,
 };
 
 /// Copies `source` to `dest` for systems without copy-on-write
@@ -22,7 +23,6 @@ pub(crate) fn copy_on_write(
     reflink_mode: ReflinkMode,
     sparse_mode: SparseMode,
     context: &str,
-    source_is_fifo: bool,
     source_is_stream: bool,
 ) -> CopyResult<CopyDebug> {
     if reflink_mode != ReflinkMode::Never {
@@ -50,13 +50,16 @@ pub(crate) fn copy_on_write(
             .mode(mode)
             .open(dest)?;
 
+        let dest_is_stream = is_stream(&dst_file.metadata()?);
+        if !dest_is_stream {
+            // `copy_stream` doesn't clear the dest file, if dest is not a stream, we should clear it manually.
+            dst_file.set_len(0)?;
+        }
+
         buf_copy::copy_stream(&mut src_file, &mut dst_file)
             .map_err(|_| std::io::Error::from(std::io::ErrorKind::Other))
             .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
 
-        if source_is_fifo {
-            dst_file.set_permissions(src_file.metadata()?.permissions())?;
-        }
         return Ok(copy_debug);
     }
 

tests/by-util/test_cp.rs

@@ -6247,6 +6247,55 @@ fn test_cp_update_none_interactive_prompt_no() {
     assert_eq!(at.read(new_file), "new content");
 }
 
+/// only unix has `/dev/fd/0`
+#[cfg(unix)]
+#[test]
+fn test_cp_from_stream() {
+    let target = "target";
+    let test_string1 = "longer: Hello, World!\n";
+    let test_string2 = "shorter";
+    let scenario = TestScenario::new(util_name!());
+    let at = &scenario.fixtures;
+    at.touch(target);
+
+    let mut ucmd = scenario.ucmd();
+    ucmd.arg("/dev/fd/0")
+        .arg(target)
+        .pipe_in(test_string1)
+        .succeeds();
+    assert_eq!(at.read(target), test_string1);
+
+    let mut ucmd = scenario.ucmd();
+    ucmd.arg("/dev/fd/0")
+        .arg(target)
+        .pipe_in(test_string2)
+        .succeeds();
+    assert_eq!(at.read(target), test_string2);
+}
+
+/// only unix has `/dev/fd/0`
+#[cfg(unix)]
+#[test]
+fn test_cp_from_stream_permission() {
+    let target = "target";
+    let link = "link";
+    let test_string = "Hello, World!\n";
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.touch(target);
+    at.symlink_file(target, link);
+    let mode = 0o777;
+    at.set_mode("target", mode);
+
+    ucmd.arg("/dev/fd/0")
+        .arg(link)
+        .pipe_in(test_string)
+        .succeeds();
+
+    assert_eq!(at.read(target), test_string);
+    assert_eq!(at.metadata(target).permissions().mode(), 0o100_777);
+}
+
 #[cfg(feature = "feat_selinux")]
 fn get_getfattr_output(f: &str) -> String {
     use std::process::Command;