Added overwrite detection for existing symlinks

src/uu/cp/src/copydir.rs

@@ -221,6 +221,7 @@ fn copy_direntry(
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
     preserve_hard_links: bool,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
 ) -> 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,
             ) {
@@ -329,6 +332,7 @@ pub(crate) fn copy_directory(
     target: &Path,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
@@ -344,6 +348,7 @@ pub(crate) fn copy_directory(
             target,
             options,
             symlinked_files,
+            copied_destinations,
             copied_files,
             source_in_command_line,
         );
@@ -417,6 +422,7 @@ pub(crate) fn copy_directory(
                     options,
                     symlinked_files,
                     preserve_hard_links,
+                    copied_destinations,
                     copied_files,
                 )?;
             }

src/uu/cp/src/cp.rs

@@ -1220,6 +1220,7 @@ 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);
@@ -1279,6 +1280,7 @@ fn copy_source(
     target_type: TargetType,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
 ) -> CopyResult<()> {
     let source_path = Path::new(&source);
@@ -1290,6 +1292,7 @@ fn copy_source(
             target,
             options,
             symlinked_files,
+            copied_destinations,
             copied_files,
             true,
         )
@@ -1302,6 +1305,7 @@ fn copy_source(
             dest.as_path(),
             options,
             symlinked_files,
+            copied_destinations,
             copied_files,
             true,
         );
@@ -1917,6 +1921,7 @@ fn copy_file(
     dest: &Path,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
+    copied_destinations: &HashSet<PathBuf>,
     copied_files: &mut HashMap<FileInformation, PathBuf>,
     source_in_command_line: bool,
 ) -> CopyResult<()> {
@@ -1934,6 +1939,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()

tests/by-util/test_cp.rs

@@ -2660,6 +2660,42 @@ 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.touch("README");
+    at.touch("good/README");
+
+    at.write("README", "file1");
+    at.write("good/README", "file2");
+
+    ts.ccmd("ln")
+        .arg("-s")
+        .arg("foo")
+        .arg("tmp/README")
+        .succeeds();
+
+    at.touch("tmp/foo");
+
+    let result = ts
+        .ucmd()
+        .arg("README")
+        .arg("good/README")
+        .arg("tmp")
+        .fails();
+    let stderr = result.stderr_str();
+
+    assert_eq!(
+        "cp: will not copy 'good/README' through just-created symlink 'tmp/README'\n",
+        stderr
+    );
+    let contents = at.read("tmp/foo");
+    assert_eq!(contents, "file1");
+}
 /// Test for copying a dangling symbolic link and its permissions.
 #[cfg(not(target_os = "freebsd"))] // FIXME: fix this test for FreeBSD
 #[test]