diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index c03eb25a5..cc45a8b8f 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 { + 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) + && options.backup != BackupMode::NumberedBackup + { + // 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/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 8b76aa73c..a056ee256 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -12,6 +12,7 @@ use uucore::fs::{make_path_relative_to, paths_refer_to_same_file}; use uucore::{format_usage, help_about, help_section, help_usage, prompt_yes, show_error}; use std::borrow::Cow; +use std::collections::HashSet; use std::error::Error; use std::ffi::OsString; use std::fmt::Display; @@ -295,6 +296,8 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) if !target_dir.is_dir() { return Err(LnError::TargetIsDirectory(target_dir.to_owned()).into()); } + // remember the linked destinations for further usage + let mut linked_destinations: HashSet = HashSet::with_capacity(files.len()); let mut all_successful = true; for srcpath in files { @@ -338,10 +341,20 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) } }; - if let Err(e) = link(srcpath, &targetpath, settings) { + if linked_destinations.contains(&targetpath) { + // If the target file was already created in this ln call, do not overwrite + show_error!( + "will not overwrite just-created '{}' with '{}'", + targetpath.display(), + srcpath.display() + ); + all_successful = false; + } else if let Err(e) = link(srcpath, &targetpath, settings) { show_error!("{}", e); all_successful = false; } + + linked_destinations.insert(targetpath.clone()); } if all_successful { Ok(()) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index dec6d3ad3..ff5aaf93d 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -10,6 +10,7 @@ mod error; use clap::builder::ValueParser; use clap::{crate_version, error::ErrorKind, Arg, ArgAction, ArgMatches, Command}; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use std::collections::HashSet; use std::env; use std::ffi::OsString; use std::fs; @@ -433,7 +434,10 @@ pub fn mv(files: &[OsString], opts: &Options) -> UResult<()> { } #[allow(clippy::cognitive_complexity)] -fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, opts: &Options) -> UResult<()> { +fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, options: &Options) -> UResult<()> { + // remember the moved destinations for further usage + let mut moved_destinations: HashSet = HashSet::with_capacity(files.len()); + if !target_dir.is_dir() { return Err(MvError::NotADirectory(target_dir.quote().to_string()).into()); } @@ -442,7 +446,7 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, opts: &Options) -> .canonicalize() .unwrap_or_else(|_| target_dir.to_path_buf()); - let multi_progress = opts.progress_bar.then(MultiProgress::new); + let multi_progress = options.progress_bar.then(MultiProgress::new); let count_progress = if let Some(ref multi_progress) = multi_progress { if files.len() > 1 { @@ -471,6 +475,20 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, opts: &Options) -> } }; + if moved_destinations.contains(&targetpath) && options.backup != BackupMode::NumberedBackup + { + // If the target file was already created in this mv call, do not overwrite + show!(USimpleError::new( + 1, + format!( + "will not overwrite just-created '{}' with '{}'", + targetpath.display(), + sourcepath.display() + ), + )); + continue; + } + // Check if we have mv dir1 dir2 dir2 // And generate an error if this is the case if let Ok(canonicalized_source) = sourcepath.canonicalize() { @@ -493,7 +511,7 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, opts: &Options) -> } } - match rename(sourcepath, &targetpath, opts, multi_progress.as_ref()) { + match rename(sourcepath, &targetpath, options, multi_progress.as_ref()) { Err(e) if e.to_string().is_empty() => set_exit_code(1), Err(e) => { let e = e.map_err_context(|| { @@ -513,6 +531,7 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, opts: &Options) -> if let Some(ref pb) = count_progress { pb.inc(1); } + moved_destinations.insert(targetpath.clone()); } Ok(()) } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index dfd20c9c2..48e7acf76 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -3592,3 +3592,36 @@ 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 ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("a"); + at.mkdir("b"); + at.mkdir("c"); + at.write("a/f", "a"); + at.write("b/f", "b"); + + let result = ts.ucmd().arg("a/f").arg("b/f").arg("c").fails(); + #[cfg(not(unix))] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c\\f' with 'b/f'")); + #[cfg(unix)] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c/f' with 'b/f'")); + + assert!(at.plus("c").join("f").exists()); + + ts.ucmd() + .arg("--backup=numbered") + .arg("a/f") + .arg("b/f") + .arg("c") + .succeeds(); + assert!(at.plus("c").join("f").exists()); + assert!(at.plus("c").join("f.~1~").exists()); +} diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index dc31f7261..2501e9d36 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -3,6 +3,8 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. use crate::common::util::TestScenario; +#[cfg(unix)] +use std::os::unix::fs::MetadataExt; use std::path::PathBuf; #[test] @@ -719,3 +721,49 @@ fn test_symlink_remove_existing_same_src_and_dest() { assert!(at.file_exists("a") && !at.symlink_exists("a")); assert_eq!(at.read("a"), "sample"); } + +#[test] +#[cfg(not(target_os = "android"))] +fn test_ln_seen_file() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("a"); + at.mkdir("b"); + at.mkdir("c"); + at.write("a/f", "a"); + at.write("b/f", "b"); + + let result = ts.ucmd().arg("a/f").arg("b/f").arg("c").fails(); + + #[cfg(not(unix))] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c\\f' with 'b/f'")); + #[cfg(unix)] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c/f' with 'b/f'")); + + assert!(at.plus("c").join("f").exists()); + // b/f still exists + assert!(at.plus("b").join("f").exists()); + // a/f still exists + assert!(at.plus("a").join("f").exists()); + #[cfg(unix)] + { + // Check inode numbers + let inode_a_f = at.plus("a").join("f").metadata().unwrap().ino(); + let inode_b_f = at.plus("b").join("f").metadata().unwrap().ino(); + let inode_c_f = at.plus("c").join("f").metadata().unwrap().ino(); + + assert_eq!( + inode_a_f, inode_c_f, + "Inode numbers of a/f and c/f should be equal" + ); + assert_ne!( + inode_b_f, inode_c_f, + "Inode numbers of b/f and c/f should not be equal" + ); + } +} diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 75500ac63..61a4aebf6 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1469,6 +1469,65 @@ fn test_mv_file_into_dir_where_both_are_files() { .stderr_contains("mv: failed to access 'b/': Not a directory"); } +#[test] +fn test_mv_seen_file() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("a"); + at.mkdir("b"); + at.mkdir("c"); + at.write("a/f", "a"); + at.write("b/f", "b"); + + let result = ts.ucmd().arg("a/f").arg("b/f").arg("c").fails(); + + #[cfg(not(unix))] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c\\f' with 'b/f'")); + #[cfg(unix)] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c/f' with 'b/f'")); + + // a/f has been moved into c/f + assert!(at.plus("c").join("f").exists()); + // b/f still exists + assert!(at.plus("b").join("f").exists()); + // a/f no longer exists + assert!(!at.plus("a").join("f").exists()); +} + +#[test] +fn test_mv_seen_multiple_files_to_directory() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("a"); + at.mkdir("b"); + at.mkdir("c"); + at.write("a/f", "a"); + at.write("b/f", "b"); + at.write("b/g", "g"); + + let result = ts.ucmd().arg("a/f").arg("b/f").arg("b/g").arg("c").fails(); + #[cfg(not(unix))] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c\\f' with 'b/f'")); + #[cfg(unix)] + assert!(result + .stderr_str() + .contains("will not overwrite just-created 'c/f' with 'b/f'")); + + assert!(!at.plus("a").join("f").exists()); + assert!(at.plus("b").join("f").exists()); + assert!(!at.plus("b").join("g").exists()); + assert!(at.plus("c").join("f").exists()); + assert!(at.plus("c").join("g").exists()); +} + #[test] fn test_mv_dir_into_file_where_both_are_files() { let scene = TestScenario::new(util_name!());