diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index 7a9d797e81c..931217d92bc 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -213,7 +213,7 @@ where // `path.ends_with(".")` does not seem to work path.as_ref().display().to_string().ends_with("/.") } - +#[allow(clippy::too_many_arguments)] /// Copy a single entry during a directory traversal. fn copy_direntry( progress_bar: &Option, @@ -221,6 +221,7 @@ fn copy_direntry( options: &Options, symlinked_files: &mut HashSet, preserve_hard_links: bool, + copied_destinations: &HashSet, copied_files: &mut HashMap, ) -> CopyResult<()> { let Entry { @@ -267,6 +268,7 @@ fn copy_direntry( local_to_target.as_path(), options, symlinked_files, + copied_destinations, copied_files, false, ) { @@ -295,6 +297,7 @@ fn copy_direntry( local_to_target.as_path(), options, symlinked_files, + copied_destinations, copied_files, false, ) { @@ -323,12 +326,14 @@ fn copy_direntry( /// /// Any errors encountered copying files in the tree will be logged but /// will not cause a short-circuit. +#[allow(clippy::too_many_arguments)] pub(crate) fn copy_directory( progress_bar: &Option, root: &Path, target: &Path, options: &Options, symlinked_files: &mut HashSet, + copied_destinations: &HashSet, copied_files: &mut HashMap, source_in_command_line: bool, ) -> CopyResult<()> { @@ -344,6 +349,7 @@ pub(crate) fn copy_directory( target, options, symlinked_files, + copied_destinations, copied_files, source_in_command_line, ); @@ -417,6 +423,7 @@ pub(crate) fn copy_directory( options, symlinked_files, preserve_hard_links, + copied_destinations, copied_files, )?; } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 5d91591c9de..e44c525074b 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1220,12 +1220,14 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult target_type, options, &mut symlinked_files, + &copied_destinations, &mut copied_files, ) { show_error_if_needed(&error); non_fatal_errors = true; + } else { + copied_destinations.insert(dest.clone()); } - copied_destinations.insert(dest.clone()); } seen_sources.insert(source); } @@ -1271,7 +1273,7 @@ fn construct_dest_path( TargetType::File => target.to_path_buf(), }) } - +#[allow(clippy::too_many_arguments)] fn copy_source( progress_bar: &Option, source: &Path, @@ -1279,6 +1281,7 @@ fn copy_source( target_type: TargetType, options: &Options, symlinked_files: &mut HashSet, + copied_destinations: &HashSet, copied_files: &mut HashMap, ) -> CopyResult<()> { let source_path = Path::new(&source); @@ -1290,6 +1293,7 @@ fn copy_source( target, options, symlinked_files, + copied_destinations, copied_files, true, ) @@ -1302,6 +1306,7 @@ fn copy_source( dest.as_path(), options, symlinked_files, + copied_destinations, copied_files, true, ); @@ -1910,13 +1915,14 @@ fn calculate_dest_permissions( /// /// The original permissions of `source` will be copied to `dest` /// after a successful copy. -#[allow(clippy::cognitive_complexity)] +#[allow(clippy::cognitive_complexity, clippy::too_many_arguments)] fn copy_file( progress_bar: &Option, source: &Path, dest: &Path, options: &Options, symlinked_files: &mut HashSet, + copied_destinations: &HashSet, copied_files: &mut HashMap, source_in_command_line: bool, ) -> CopyResult<()> { @@ -1934,6 +1940,17 @@ fn copy_file( dest.display() ))); } + // Fail if cp tries to copy two sources of the same name into a single symlink + // Example: "cp file1 dir1/file1 tmp" where "tmp" is a directory containing a symlink "file1" pointing to a file named "foo". + // foo will contain the contents of "file1" and "dir1/file1" will not be copied over to "tmp/file1" + if copied_destinations.contains(dest) { + return Err(Error::Error(format!( + "will not copy '{}' through just-created symlink '{}'", + source.display(), + dest.display() + ))); + } + let copy_contents = options.dereference(source_in_command_line) || !source_is_symlink; if copy_contents && !dest.exists() diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 7db18806938..a8268f3c6ba 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2660,6 +2660,62 @@ fn test_copy_through_dangling_symlink_no_dereference() { .no_stdout(); } +#[test] +fn test_cp_symlink_overwrite_detection() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("good"); + at.mkdir("tmp"); + at.write("README", "file1"); + at.write("good/README", "file2"); + + at.symlink_file("tmp/foo", "tmp/README"); + at.touch("tmp/foo"); + + ts.ucmd() + .arg("README") + .arg("good/README") + .arg("tmp") + .fails() + .stderr_only(if cfg!(target_os = "windows") { + "cp: will not copy 'good/README' through just-created symlink 'tmp\\README'\n" + } else if cfg!(target_os = "macos") { + "cp: will not overwrite just-created 'tmp/README' with 'good/README'\n" + } else { + "cp: will not copy 'good/README' through just-created symlink 'tmp/README'\n" + }); + let contents = at.read("tmp/foo"); + // None of the files seem to be copied in macos + if cfg!(not(target_os = "macos")) { + assert_eq!(contents, "file1"); + } +} + +#[test] +fn test_cp_dangling_symlink_inside_directory() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("good"); + at.mkdir("tmp"); + at.write("README", "file1"); + at.write("good/README", "file2"); + + at.symlink_file("foo", "tmp/README"); + + ts.ucmd() + .arg("README") + .arg("good/README") + .arg("tmp") + .fails() + .stderr_only( if cfg!(target_os="windows") { + "cp: not writing through dangling symlink 'tmp\\README'\ncp: not writing through dangling symlink 'tmp\\README'\n" + } else { + "cp: not writing through dangling symlink 'tmp/README'\ncp: not writing through dangling symlink 'tmp/README'\n" + } ); +} + /// Test for copying a dangling symbolic link and its permissions. #[cfg(not(target_os = "freebsd"))] // FIXME: fix this test for FreeBSD #[test]