diff --git a/Cargo.lock b/Cargo.lock index 72e78bcc0..05b86a986 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3407,6 +3407,7 @@ dependencies = [ "sha2", "sha3", "sm3", + "tempfile", "thiserror", "time", "uucore_procs", diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index fc21fa26d..54616e75e 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -38,7 +38,8 @@ use uucore::backup_control::{self, BackupMode}; use uucore::display::Quotable; use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError}; use uucore::fs::{ - canonicalize, paths_refer_to_same_file, FileInformation, MissingHandling, ResolveMode, + canonicalize, is_symlink_loop, paths_refer_to_same_file, FileInformation, MissingHandling, + ResolveMode, }; use uucore::update_control::{self, UpdateMode}; use uucore::{ @@ -1389,11 +1390,10 @@ fn handle_existing_dest( backup_dest(dest, &backup_path)?; } } - match options.overwrite { // FIXME: print that the file was removed if --verbose is enabled OverwriteMode::Clobber(ClobberMode::Force) => { - if fs::metadata(dest)?.permissions().readonly() { + if is_symlink_loop(dest) || fs::metadata(dest)?.permissions().readonly() { fs::remove_file(dest)?; } } @@ -1501,6 +1501,7 @@ fn copy_file( options.overwrite, OverwriteMode::Clobber(ClobberMode::RemoveDestination) ) + && !is_symlink_loop(dest) && std::env::var_os("POSIXLY_CORRECT").is_none() { return Err(Error::Error(format!( diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index 669971a69..d5fe9e54c 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -54,6 +54,7 @@ nix = { workspace=true, features = ["fs", "uio", "zerocopy", "signal"] } [dev-dependencies] clap = { workspace=true } once_cell = { workspace=true } +tempfile = { workspace=true } [target.'cfg(target_os = "windows")'.dependencies] winapi-util = { version= "0.1.5", optional=true } diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index b82eba94a..6dc371a10 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -584,10 +584,45 @@ pub fn make_path_relative_to, P2: AsRef>(path: P1, to: P2) components.iter().collect() } +/// Checks if there is a symlink loop in the given path. +/// +/// A symlink loop is a chain of symlinks where the last symlink points back to one of the previous symlinks in the chain. +/// +/// # Arguments +/// +/// * `path` - A reference to a `Path` representing the starting path to check for symlink loops. +/// +/// # Returns +/// +/// * `bool` - Returns `true` if a symlink loop is detected, `false` otherwise. +pub fn is_symlink_loop(path: &Path) -> bool { + let mut visited_symlinks = HashSet::new(); + let mut current_path = path.to_path_buf(); + + while let (Ok(metadata), Ok(link)) = ( + current_path.symlink_metadata(), + fs::read_link(¤t_path), + ) { + if !metadata.file_type().is_symlink() { + return false; + } + if !visited_symlinks.insert(current_path.clone()) { + return true; + } + current_path = link; + } + + false +} + #[cfg(test)] mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope. use super::*; + #[cfg(unix)] + use std::os::unix; + #[cfg(unix)] + use tempfile::tempdir; struct NormalizePathTestCase<'a> { path: &'a str, @@ -695,4 +730,41 @@ mod tests { display_permissions_unix(S_IFCHR | S_ISVTX as mode_t | 0o054, true) ); } + + #[cfg(unix)] + #[test] + fn test_is_symlink_loop_no_loop() { + let temp_dir = tempdir().unwrap(); + let file_path = temp_dir.path().join("file.txt"); + let symlink_path = temp_dir.path().join("symlink"); + + fs::write(&file_path, "test content").unwrap(); + unix::fs::symlink(&file_path, &symlink_path).unwrap(); + + assert!(!is_symlink_loop(&symlink_path)); + } + + #[cfg(unix)] + #[test] + fn test_is_symlink_loop_direct_loop() { + let temp_dir = tempdir().unwrap(); + let symlink_path = temp_dir.path().join("loop"); + + unix::fs::symlink(&symlink_path, &symlink_path).unwrap(); + + assert!(is_symlink_loop(&symlink_path)); + } + + #[cfg(unix)] + #[test] + fn test_is_symlink_loop_indirect_loop() { + let temp_dir = tempdir().unwrap(); + let symlink1_path = temp_dir.path().join("symlink1"); + let symlink2_path = temp_dir.path().join("symlink2"); + + unix::fs::symlink(&symlink1_path, &symlink2_path).unwrap(); + unix::fs::symlink(&symlink2_path, &symlink1_path).unwrap(); + + assert!(is_symlink_loop(&symlink1_path)); + } } diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index a54824d18..9435e3201 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -209,7 +209,7 @@ mod test { assert_eq!(super::parse_mode("u+x").unwrap(), 0o766); assert_eq!( super::parse_mode("+x").unwrap(), - if !crate::os::is_wsl_1() { 0o777 } else { 0o776 } + if crate::os::is_wsl_1() { 0o776 } else { 0o777 } ); assert_eq!(super::parse_mode("a-w").unwrap(), 0o444); assert_eq!(super::parse_mode("g-r").unwrap(), 0o626); diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 005efa14d..8d55a17ca 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2495,6 +2495,20 @@ fn test_remove_destination_symbolic_link_loop() { assert!(at.file_exists("loop")); } +#[test] +#[cfg(not(windows))] +fn test_cp_symbolic_link_loop() { + let (at, mut ucmd) = at_and_ucmd!(); + at.symlink_file("loop", "loop"); + at.plus("loop"); + at.touch("f"); + ucmd.args(&["-f", "f", "loop"]) + .succeeds() + .no_stdout() + .no_stderr(); + assert!(at.file_exists("loop")); +} + /// Test that copying a directory to itself is disallowed. #[test] fn test_copy_directory_to_itself_disallowed() { diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index ce9e26a81..abe9a4e36 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -72,7 +72,7 @@ fn test_mv_move_file_into_dir_with_target_arg() { .succeeds() .no_stderr(); - assert!(at.file_exists(format!("{dir}/{file}"))) + assert!(at.file_exists(format!("{dir}/{file}"))); } #[test] @@ -90,7 +90,7 @@ fn test_mv_move_file_into_file_with_target_arg() { .fails() .stderr_is(format!("mv: target directory '{file1}': Not a directory\n")); - assert!(at.file_exists(file1)) + assert!(at.file_exists(file1)); } #[test]