diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index aa513be0a..65519901b 100644 --- a/src/uu/cp/src/cp.rs +++ b/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 = 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 source_is_stream = is_stream(&source_metadata); 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, )?; diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 1095d4674..5c1acf958 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/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 -/// determine if we need to modify the file's attributes before and after copying. -fn copy_stream

(source: P, dest: P, is_fifo: bool) -> std::io::Result +/// Copy the contents of a stream from `source` to `dest`. +fn copy_stream

(source: P, dest: P) -> std::io::Result where P: AsRef, { @@ -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 { 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); diff --git a/src/uu/cp/src/platform/macos.rs b/src/uu/cp/src/platform/macos.rs index 8c915d2df..d833c476a 100644 --- a/src/uu/cp/src/platform/macos.rs +++ b/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 { 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) - .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())?; + 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)?; } - 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()))? diff --git a/src/uu/cp/src/platform/other_unix.rs b/src/uu/cp/src/platform/other_unix.rs index 00f3cbf6e..174eaca3d 100644 --- a/src/uu/cp/src/platform/other_unix.rs +++ b/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 { 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); } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index fc3576861..fda8bee87 100644 --- a/tests/by-util/test_cp.rs +++ b/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;