diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index c1df9ed139a..7ba3d2fecdc 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1664,6 +1664,13 @@ pub(crate) fn copy_attributes( let source_metadata = fs::symlink_metadata(source).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + // if --no-preserve wasn't explicitly passed and we're copying a directory by default we should preserve mode + let mode = if dest.is_dir() && attributes.mode == (Preserve::No { explicit: false }) { + Preserve::Yes { required: false } + } else { + attributes.mode + }; + // Ownership must be changed first to avoid interfering with mode change. #[cfg(unix)] handle_preserve(&attributes.ownership, || -> CopyResult<()> { @@ -1701,7 +1708,7 @@ pub(crate) fn copy_attributes( Ok(()) })?; - handle_preserve(&attributes.mode, || -> CopyResult<()> { + handle_preserve(&mode, || -> CopyResult<()> { // The `chmod()` system call that underlies the // `fs::set_permissions()` call is unable to change the // permissions of a symbolic link. In that case, we just diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 2563e533ae4..1cac109d281 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -7400,3 +7400,71 @@ fn test_cp_recurse_verbose_output_with_symlink_already_exists() { .no_stderr() .stdout_is(output); } + +#[test] +#[cfg(not(target_os = "windows"))] +fn test_cp_preserve_directory_permissions_by_default() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let dir = "a/b/c/d"; + let file = "foo.txt"; + + at.mkdir_all(dir); + + let file_path = format!("{dir}/{file}"); + + at.touch(file_path); + + scene.cmd("chmod").arg("-R").arg("555").arg("a").succeeds(); + scene.cmd("cp").arg("-r").arg("a").arg("b").succeeds(); + + scene.ucmd().arg("-r").arg("a").arg("c").succeeds(); + + assert_eq!(at.get_mode("b"), 0o40555); + assert_eq!(at.get_mode("b/b"), 0o40555); + assert_eq!(at.get_mode("b/b/c"), 0o40555); + assert_eq!(at.get_mode("b/b/c/d"), 0o40555); + + assert_eq!(at.get_mode("c"), 0o40555); + assert_eq!(at.get_mode("c/b"), 0o40555); + assert_eq!(at.get_mode("c/b/c"), 0o40555); + assert_eq!(at.get_mode("c/b/c/d"), 0o40555); +} + +#[test] +#[cfg(not(target_os = "windows"))] +fn test_cp_no_preserve_directory_permissions() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let dir = "a/b/c/d"; + let file = "foo.txt"; + + at.mkdir_all(dir); + + let file_path = format!("{dir}/{file}"); + + at.touch(file_path); + + scene.cmd("chmod").arg("-R").arg("555").arg("a").succeeds(); + scene.cmd("cp").arg("-r").arg("a").arg("b").succeeds(); + + scene + .ucmd() + .arg("-r") + .arg("--no-preserve=mode") + .arg("a") + .arg("c") + .succeeds(); + + assert_eq!(at.get_mode("b"), 0o40555); + assert_eq!(at.get_mode("b/b"), 0o40555); + assert_eq!(at.get_mode("b/b/c"), 0o40555); + assert_eq!(at.get_mode("b/b/c/d"), 0o40555); + + assert_eq!(at.get_mode("c"), 0o40755); + assert_eq!(at.get_mode("c/b"), 0o40755); + assert_eq!(at.get_mode("c/b/c"), 0o40755); + assert_eq!(at.get_mode("c/b/c/d"), 0o40755); +} diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 5c5ed3ef401..7f1b0b5a144 100644 --- a/tests/uutests/src/lib/util.rs +++ b/tests/uutests/src/lib/util.rs @@ -1324,6 +1324,18 @@ impl AtPath { perms.set_mode(mode); std::fs::set_permissions(&path, perms).unwrap(); } + + /// Get the permissions of the specified file. + /// + /// # Panics + /// + /// This function panics if there is an error loading the metadata + #[cfg(not(windows))] + pub fn get_mode(&self, filename: &str) -> u32 { + let path = self.plus(filename); + let perms = std::fs::metadata(&path).unwrap().permissions(); + perms.mode() + } } /// An environment for running a single uutils test case, serves three functions: