cp: Avoid following a destination symlink with -P

Previously, given 'cp -P a b', where 'a' and 'b' were both symlinks, cp
would end up replacing the target of 'b'.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>

src/uu/cp/src/cp.rs

@@ -1230,7 +1230,8 @@ fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyRe
 }
 
 /// Copy the a file from `source` to `dest`. `source` will be dereferenced if
-/// `options.dereference` is set to true. `dest` will always be dereferenced.
+/// `options.dereference` is set to true. `dest` will be dereferenced only if
+/// the source was not a symlink.
 ///
 /// Behavior when copying to existing files is contingent on the
 /// `options.overwrite` mode. If a file is skipped, the return type
@@ -1280,13 +1281,25 @@ fn copy_file(
     let context = context.as_str();
 
     // canonicalize dest and source so that later steps can work with the paths directly
-    let dest = canonicalize(dest, MissingHandling::Missing, ResolveMode::Physical).unwrap();
     let source = if options.dereference {
         canonicalize(source, MissingHandling::Missing, ResolveMode::Physical).unwrap()
     } else {
         source.to_owned()
     };
 
+    let source_is_symlink = fs::symlink_metadata(&source).context(context)?.is_symlink();
+    let dest_already_exists_as_symlink = fs::symlink_metadata(&dest)
+        .map(|meta| meta.file_type().is_symlink())
+        .unwrap_or(false);
+
+    let dest = if !(source_is_symlink && dest_already_exists_as_symlink) {
+        canonicalize(dest, MissingHandling::Missing, ResolveMode::Physical).unwrap()
+    } else {
+        // Don't canonicalize a symlink copied over another symlink, because
+        // then we'll end up overwriting the destination's target.
+        dest.to_path_buf()
+    };
+
     let dest_permissions = if dest.exists() {
         dest.symlink_metadata().context(context)?.permissions()
     } else {
@@ -1315,7 +1328,14 @@ fn copy_file(
             fs::hard_link(&source, &dest).context(context)?;
         }
         CopyMode::Copy => {
-            copy_helper(&source, &dest, options, context, symlinked_files)?;
+            copy_helper(
+                &source,
+                &dest,
+                options,
+                context,
+                source_is_symlink,
+                symlinked_files,
+            )?;
         }
         CopyMode::SymLink => {
             symlink_file(&source, &dest, context, symlinked_files)?;
@@ -1331,10 +1351,24 @@ fn copy_file(
                 if src_time <= dest_time {
                     return Ok(());
                 } else {
-                    copy_helper(&source, &dest, options, context, symlinked_files)?;
+                    copy_helper(
+                        &source,
+                        &dest,
+                        options,
+                        context,
+                        source_is_symlink,
+                        symlinked_files,
+                    )?;
                 }
             } else {
-                copy_helper(&source, &dest, options, context, symlinked_files)?;
+                copy_helper(
+                    &source,
+                    &dest,
+                    options,
+                    context,
+                    source_is_symlink,
+                    symlinked_files,
+                )?;
             }
         }
         CopyMode::AttrOnly => {
@@ -1373,19 +1407,20 @@ fn copy_helper(
     dest: &Path,
     options: &Options,
     context: &str,
+    source_is_symlink: bool,
     symlinked_files: &mut HashSet<FileInformation>,
 ) -> CopyResult<()> {
     if options.parents {
         let parent = dest.parent().unwrap_or(dest);
         fs::create_dir_all(parent)?;
     }
-    let is_symlink = fs::symlink_metadata(&source)?.file_type().is_symlink();
+
     if source.as_os_str() == "/dev/null" {
         /* workaround a limitation of fs::copy
          * https://github.com/rust-lang/rust/issues/79390
          */
         File::create(dest).context(dest.display().to_string())?;
-    } else if is_symlink {
+    } else if source_is_symlink {
         copy_link(source, dest, symlinked_files)?;
     } else if options.reflink_mode != ReflinkMode::Never {
         #[cfg(not(any(target_os = "linux", target_os = "macos")))]

tests/by-util/test_cp.rs

@@ -29,6 +29,7 @@ static TEST_EXISTING_FILE: &str = "existing_file.txt";
 static TEST_HELLO_WORLD_SOURCE: &str = "hello_world.txt";
 static TEST_HELLO_WORLD_SOURCE_SYMLINK: &str = "hello_world.txt.link";
 static TEST_HELLO_WORLD_DEST: &str = "copy_of_hello_world.txt";
+static TEST_HELLO_WORLD_DEST_SYMLINK: &str = "copy_of_hello_world.txt.link";
 static TEST_HOW_ARE_YOU_SOURCE: &str = "how_are_you.txt";
 static TEST_HOW_ARE_YOU_DEST: &str = "hello_dir/how_are_you.txt";
 static TEST_COPY_TO_FOLDER: &str = "hello_dir/";
@@ -688,6 +689,47 @@ fn test_cp_no_deref() {
     assert_eq!(at.read(path_to_check), "Hello, World!\n");
 }
 
+#[test]
+fn test_cp_no_deref_link_onto_link() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.copy(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_DEST);
+
+    #[cfg(not(windows))]
+    let _r = fs::symlink(
+        TEST_HELLO_WORLD_SOURCE,
+        at.subdir.join(TEST_HELLO_WORLD_SOURCE_SYMLINK),
+    );
+    #[cfg(windows)]
+    let _r = symlink_file(
+        TEST_HELLO_WORLD_SOURCE,
+        at.subdir.join(TEST_HELLO_WORLD_SOURCE_SYMLINK),
+    );
+
+    #[cfg(not(windows))]
+    let _r = fs::symlink(
+        TEST_HELLO_WORLD_DEST,
+        at.subdir.join(TEST_HELLO_WORLD_DEST_SYMLINK),
+    );
+    #[cfg(windows)]
+    let _r = symlink_file(
+        TEST_HELLO_WORLD_DEST,
+        at.subdir.join(TEST_HELLO_WORLD_DEST_SYMLINK),
+    );
+
+    ucmd.arg("-P")
+        .arg(TEST_HELLO_WORLD_SOURCE_SYMLINK)
+        .arg(TEST_HELLO_WORLD_DEST_SYMLINK)
+        .succeeds();
+
+    // Ensure that the target of the destination was not modified.
+    assert!(!at.symlink_metadata(TEST_HELLO_WORLD_DEST).is_symlink());
+    assert!(at
+        .symlink_metadata(TEST_HELLO_WORLD_DEST_SYMLINK)
+        .is_symlink());
+    assert_eq!(at.read(TEST_HELLO_WORLD_DEST_SYMLINK), "Hello, World!\n");
+}
+
 #[test]
 fn test_cp_strip_trailing_slashes() {
     let (at, mut ucmd) = at_and_ucmd!();