From 0530b1c8a60e5ababe460b57c09988d5f4a6a2d8 Mon Sep 17 00:00:00 2001 From: mhead Date: Mon, 3 Jun 2024 13:08:14 +0530 Subject: [PATCH 1/5] cp: symlink permission --- src/uu/cp/src/copydir.rs | 3 ++- src/uu/cp/src/cp.rs | 14 ++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index 43ca9f998a5..e89eefa4b32 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -431,7 +431,8 @@ pub(crate) fn copy_directory( let dest = target.join(root.file_name().unwrap()); copy_attributes(root, dest.as_path(), &options.attributes)?; for (x, y) in aligned_ancestors(root, dest.as_path()) { - copy_attributes(x, y, &options.attributes)?; + let src = x.canonicalize()?; + copy_attributes(&src, y, &options.attributes)?; } } else { copy_attributes(root, target, &options.attributes)?; diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 2b2a2a2bf09..fd170d0ae34 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1332,7 +1332,7 @@ fn construct_dest_path( Ok(match target_type { TargetType::Directory => { let root = if options.parents { - Path::new("") + Path::new("") } else { source_path.parent().unwrap_or(source_path) }; @@ -1380,7 +1380,8 @@ fn copy_source( ); if options.parents { for (x, y) in aligned_ancestors(source, dest.as_path()) { - copy_attributes(x, y, &options.attributes)?; + let src = x.canonicalize()?; + copy_attributes(&src, y, &options.attributes)?; } } res @@ -2162,9 +2163,14 @@ fn copy_file( // the user does not have permission to write to the file. fs::set_permissions(dest, dest_permissions).ok(); } + + if options.dereference(source_in_command_line){ + copy_attributes(&source.canonicalize()?, dest, &options.attributes)?; + }else { + copy_attributes(source, dest, &options.attributes)?; - copy_attributes(source, dest, &options.attributes)?; - + } + copied_files.insert( FileInformation::from_path(source, options.dereference(source_in_command_line))?, dest.to_path_buf(), From 62491037827990d94c4d9c7e8645d09d148c5cf5 Mon Sep 17 00:00:00 2001 From: mhead Date: Mon, 3 Jun 2024 13:45:21 +0530 Subject: [PATCH 2/5] cp: symlink permission test --- tests/by-util/test_cp.rs | 65 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 1e6586dc830..92a4f30145f 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -5573,3 +5573,68 @@ fn test_preserve_attrs_overriding_2() { assert_ne!(dest_file1_metadata.ino(), dest_file2_metadata.ino()); } } + + /// Test the behavior of preserving permissions when copying through a symlink +#[test] +fn test_cp_symlink_permissions() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("a"); + at.set_mode("a", 0o700); + at.symlink_file("a", "symlink"); + at.mkdir("dest"); + scene + .ucmd() + .args(&["--preserve", "symlink", "dest"]) + .succeeds(); + let dest_dir_metadata = at.metadata("dest/symlink"); + let src_dir_metadata = at.metadata("a"); + assert_eq!( + src_dir_metadata.permissions().mode(), + dest_dir_metadata.permissions().mode() + ); +} + + /// Test the behavior of preserving permissions of parents when copying through a symlink +#[test] +fn test_cp_parents_symlink_permissions_file() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("a"); + at.touch("a/file"); + at.set_mode("a", 0o700); + at.symlink_dir("a", "symlink"); + at.mkdir("dest"); + scene + .ucmd() + .args(&["--parents", "-a", "symlink/file", "dest"]) + .succeeds(); + let dest_dir_metadata = at.metadata("dest/symlink"); + let src_dir_metadata = at.metadata("a"); + assert_eq!( + src_dir_metadata.permissions().mode(), + dest_dir_metadata.permissions().mode() + ); +} + + /// Test the behavior of preserving permissions of parents when copying through + /// a symlink when source is a dir. +#[test] +fn test_cp_parents_symlink_permissions_dir() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("a/b"); + at.set_mode("a", 0o755); // Set mode for the actual directory + at.symlink_dir("a", "symlink"); + at.mkdir("dest"); + scene + .ucmd() + .args(&["--parents", "-a", "symlink/b", "dest"]) + .succeeds(); + let dest_dir_metadata = at.metadata("dest/symlink"); + let src_dir_metadata = at.metadata("a"); + assert_eq!( + src_dir_metadata.permissions().mode(), + dest_dir_metadata.permissions().mode() + ); +} From 9a9859d7c7ec35bd8033f4e8740e0d44d5cb70d8 Mon Sep 17 00:00:00 2001 From: mhead Date: Mon, 3 Jun 2024 15:25:29 +0530 Subject: [PATCH 3/5] cp: `--parents` absolute path fix --- src/uu/cp/src/cp.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index fd170d0ae34..041676cdd25 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1332,7 +1332,11 @@ fn construct_dest_path( Ok(match target_type { TargetType::Directory => { let root = if options.parents { + if source_path.has_root() { + Path::new("/") + } else { Path::new("") + } } else { source_path.parent().unwrap_or(source_path) }; @@ -1444,8 +1448,8 @@ pub(crate) fn copy_attributes( let dest_uid = source_metadata.uid(); let dest_gid = source_metadata.gid(); - - wrap_chown( + // gnu compatibility: cp doesn't report an error if it fails to set the ownership. + let _ = wrap_chown( dest, &dest.symlink_metadata().context(context)?, Some(dest_uid), @@ -1453,11 +1457,9 @@ pub(crate) fn copy_attributes( false, Verbosity { groups_only: false, - level: VerbosityLevel::Normal, + level: VerbosityLevel::Silent, }, - ) - .map_err(Error::Error)?; - + ); Ok(()) })?; From 651fc7c762628ac8cd969da9485c2af2a5075ca4 Mon Sep 17 00:00:00 2001 From: mhead Date: Mon, 3 Jun 2024 15:30:41 +0530 Subject: [PATCH 4/5] cp: `--parents` absolute path fix tests --- src/uu/cp/src/cp.rs | 11 +++++------ tests/by-util/test_cp.rs | 25 +++++++++++++++++++++---- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 041676cdd25..2a47fe724f2 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1459,7 +1459,7 @@ pub(crate) fn copy_attributes( groups_only: false, level: VerbosityLevel::Silent, }, - ); + ); Ok(()) })?; @@ -2165,14 +2165,13 @@ fn copy_file( // the user does not have permission to write to the file. fs::set_permissions(dest, dest_permissions).ok(); } - - if options.dereference(source_in_command_line){ + + if options.dereference(source_in_command_line) { copy_attributes(&source.canonicalize()?, dest, &options.attributes)?; - }else { + } else { copy_attributes(source, dest, &options.attributes)?; - } - + copied_files.insert( FileInformation::from_path(source, options.dereference(source_in_command_line))?, dest.to_path_buf(), diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 92a4f30145f..e4073135756 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -5574,7 +5574,7 @@ fn test_preserve_attrs_overriding_2() { } } - /// Test the behavior of preserving permissions when copying through a symlink +/// Test the behavior of preserving permissions when copying through a symlink #[test] fn test_cp_symlink_permissions() { let scene = TestScenario::new(util_name!()); @@ -5595,7 +5595,7 @@ fn test_cp_symlink_permissions() { ); } - /// Test the behavior of preserving permissions of parents when copying through a symlink +/// Test the behavior of preserving permissions of parents when copying through a symlink #[test] fn test_cp_parents_symlink_permissions_file() { let scene = TestScenario::new(util_name!()); @@ -5617,8 +5617,8 @@ fn test_cp_parents_symlink_permissions_file() { ); } - /// Test the behavior of preserving permissions of parents when copying through - /// a symlink when source is a dir. +/// Test the behavior of preserving permissions of parents when copying through +/// a symlink when source is a dir. #[test] fn test_cp_parents_symlink_permissions_dir() { let scene = TestScenario::new(util_name!()); @@ -5638,3 +5638,20 @@ fn test_cp_parents_symlink_permissions_dir() { dest_dir_metadata.permissions().mode() ); } + +/// Test the behavior of copying a file to a destination with parents using absolute paths. +#[test] +fn test_cp_parents_absolute_path() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("a/b"); + at.touch("a/b/f"); + at.mkdir("dest"); + let src = format!("{}/a/b/f", at.root_dir_resolved()); + scene + .ucmd() + .args(&["--parents", src.as_str(), "dest"]) + .succeeds(); + let res = format!("dest{}/a/b/f", at.root_dir_resolved()); + at.file_exists(res); +} From 6514a4c9a04a171779944782a35a916b88e9885c Mon Sep 17 00:00:00 2001 From: mhead Date: Tue, 4 Jun 2024 18:32:46 +0530 Subject: [PATCH 5/5] minor changes --- src/uu/cp/src/copydir.rs | 5 +++-- src/uu/cp/src/cp.rs | 11 +++++++---- tests/by-util/test_cp.rs | 4 ++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index e89eefa4b32..627f5a4de32 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -431,8 +431,9 @@ pub(crate) fn copy_directory( let dest = target.join(root.file_name().unwrap()); copy_attributes(root, dest.as_path(), &options.attributes)?; for (x, y) in aligned_ancestors(root, dest.as_path()) { - let src = x.canonicalize()?; - copy_attributes(&src, y, &options.attributes)?; + if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { + copy_attributes(&src, y, &options.attributes)?; + } } } else { copy_attributes(root, target, &options.attributes)?; diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 2a47fe724f2..7207673a5ff 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1332,7 +1332,7 @@ fn construct_dest_path( Ok(match target_type { TargetType::Directory => { let root = if options.parents { - if source_path.has_root() { + if source_path.has_root() && cfg!(unix) { Path::new("/") } else { Path::new("") @@ -1384,8 +1384,9 @@ fn copy_source( ); if options.parents { for (x, y) in aligned_ancestors(source, dest.as_path()) { - let src = x.canonicalize()?; - copy_attributes(&src, y, &options.attributes)?; + if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { + copy_attributes(&src, y, &options.attributes)?; + } } } res @@ -2167,7 +2168,9 @@ fn copy_file( } if options.dereference(source_in_command_line) { - copy_attributes(&source.canonicalize()?, dest, &options.attributes)?; + if let Ok(src) = canonicalize(source, MissingHandling::Normal, ResolveMode::Physical) { + copy_attributes(&src, dest, &options.attributes)?; + } } else { copy_attributes(source, dest, &options.attributes)?; } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index e4073135756..3b4ec74f775 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -5576,6 +5576,7 @@ fn test_preserve_attrs_overriding_2() { /// Test the behavior of preserving permissions when copying through a symlink #[test] +#[cfg(unix)] fn test_cp_symlink_permissions() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; @@ -5597,6 +5598,7 @@ fn test_cp_symlink_permissions() { /// Test the behavior of preserving permissions of parents when copying through a symlink #[test] +#[cfg(unix)] fn test_cp_parents_symlink_permissions_file() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; @@ -5620,6 +5622,7 @@ fn test_cp_parents_symlink_permissions_file() { /// Test the behavior of preserving permissions of parents when copying through /// a symlink when source is a dir. #[test] +#[cfg(unix)] fn test_cp_parents_symlink_permissions_dir() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; @@ -5640,6 +5643,7 @@ fn test_cp_parents_symlink_permissions_dir() { } /// Test the behavior of copying a file to a destination with parents using absolute paths. +#[cfg(unix)] #[test] fn test_cp_parents_absolute_path() { let scene = TestScenario::new(util_name!());