From 2dc3d867d8d12acae08874c654c1d3bebc149a0b Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Tue, 8 Feb 2022 19:37:55 -0600 Subject: [PATCH] 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 --- src/uu/cp/src/cp.rs | 68 +++++++++++++++++++++++++++++++--------- tests/by-util/test_cp.rs | 46 +++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 15 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 63dca874e..a4a512d6b 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1245,7 +1245,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 @@ -1295,13 +1296,32 @@ 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_file_type = fs::symlink_metadata(&source).context(context)?.file_type(); + let source_is_symlink = source_file_type.is_symlink(); + + #[cfg(unix)] + let source_is_fifo = source_file_type.is_fifo(); + #[cfg(not(unix))] + let source_is_fifo = false; + + 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 { @@ -1339,7 +1359,15 @@ 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, + source_is_fifo, + symlinked_files, + )?; } CopyMode::SymLink => { symlink_file(&source, &dest, context, symlinked_files)?; @@ -1355,10 +1383,26 @@ 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, + source_is_fifo, + symlinked_files, + )?; } } else { - copy_helper(&source, &dest, options, context, symlinked_files)?; + copy_helper( + &source, + &dest, + options, + context, + source_is_symlink, + source_is_fifo, + symlinked_files, + )?; } } CopyMode::AttrOnly => { @@ -1397,6 +1441,8 @@ fn copy_helper( dest: &Path, options: &Options, context: &str, + source_is_symlink: bool, + source_is_fifo: bool, symlinked_files: &mut HashSet, ) -> CopyResult<()> { if options.parents { @@ -1404,23 +1450,15 @@ fn copy_helper( fs::create_dir_all(parent)?; } - let file_type = fs::symlink_metadata(&source)?.file_type(); - let is_symlink = file_type.is_symlink(); - - #[cfg(unix)] - let is_fifo = file_type.is_fifo(); - #[cfg(not(unix))] - let is_fifo = false; - 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_fifo && options.recursive { + } else if source_is_fifo && options.recursive { #[cfg(unix)] copy_fifo(dest, options.overwrite)?; - } 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")))] diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 7ba2fa762..9ee76bb5e 100644 --- a/tests/by-util/test_cp.rs +++ b/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,51 @@ 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) + .file_type() + .is_symlink()); + assert!(at + .symlink_metadata(TEST_HELLO_WORLD_DEST_SYMLINK) + .file_type() + .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!();