From 837640bc0235533f498aace9fc5c9541638ad88f Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 23 Dec 2023 11:53:25 +0100 Subject: [PATCH] cp: manages the 'seen' file list before copying Should help with tests/mv/childproof.sh --- src/uu/cp/src/cp.rs | 46 ++++++++++++++++++++++++++++++---------- tests/by-util/test_cp.rs | 19 +++++++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index de01a5ef3..d7aeea1b9 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1171,6 +1171,9 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult // // key is the source file's information and the value is the destination filepath. let mut copied_files: HashMap = HashMap::with_capacity(sources.len()); + // remember the copied destinations for further usage. + // we can't use copied_files as it is because the key is the source file's information. + let mut copied_destinations: HashSet = HashSet::with_capacity(sources.len()); let progress_bar = if options.progress_bar { let pb = ProgressBar::new(disk_usage(sources, options.recursive)?) @@ -1191,17 +1194,38 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult if seen_sources.contains(source) { // FIXME: compare sources by the actual file they point to, not their path. (e.g. dir/file == dir/../dir/file in most cases) show_warning!("source {} specified more than once", source.quote()); - } else if let Err(error) = copy_source( - &progress_bar, - source, - target, - target_type, - options, - &mut symlinked_files, - &mut copied_files, - ) { - show_error_if_needed(&error); - non_fatal_errors = true; + } else { + // We need to compute the destination path + + let dest = construct_dest_path(source, target, target_type, options) + .unwrap_or_else(|_| target.to_path_buf()); + + if fs::metadata(&dest).is_ok() && !fs::symlink_metadata(&dest)?.file_type().is_symlink() + { + // There is already a file and it isn't a symlink (managed in a different place) + if copied_destinations.contains(&dest) { + // If the target file was already created in this cp call, do not overwrite + return Err(Error::Error(format!( + "will not overwrite just-created '{}' with '{}'", + dest.display(), + source.display() + ))); + } + } + + if let Err(error) = copy_source( + &progress_bar, + source, + target, + target_type, + options, + &mut symlinked_files, + &mut copied_files, + ) { + show_error_if_needed(&error); + non_fatal_errors = true; + } + copied_destinations.insert(dest.clone()); } seen_sources.insert(source); } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 37bec5222..5227f0194 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3559,3 +3559,22 @@ fn test_cp_attributes_only() { assert_eq!(mode_a, at.metadata(a).mode()); assert_eq!(mode_b, at.metadata(b).mode()); } + +#[test] +fn test_cp_seen_file() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.mkdir("a"); + at.mkdir("b"); + at.mkdir("c"); + at.write("a/f", "a"); + at.write("b/f", "b"); + + ucmd.arg("a/f") + .arg("b/f") + .arg("c") + .fails() + .stderr_contains("will not overwrite just-created 'c/f' with 'b/f'"); + + assert!(at.plus("c").join("f").exists()); +}