diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 79743385a..c69fe7dd0 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -38,7 +38,7 @@ use libc::mkfifo; use quick_error::ResultExt; use uucore::backup_control::{self, BackupMode}; use uucore::display::Quotable; -use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError}; +use uucore::error::{set_exit_code, UClapError, UError, UIoError, UResult, UUsageError}; use uucore::format_usage; use uucore::fs::{ canonicalize, paths_refer_to_same_file, FileInformation, MissingHandling, ResolveMode, @@ -1173,17 +1173,36 @@ fn copy_directory( }?; } } else { - copy_file( + // At this point, `path` is just a plain old file. + // Terminate this function immediately if there is any + // kind of error *except* a "permission denied" error. + // + // TODO What other kinds of errors, if any, should + // cause us to continue walking the directory? + match copy_file( path.as_path(), local_to_target.as_path(), options, symlinked_files, false, - )?; + ) { + Ok(_) => {} + Err(Error::IoErrContext(e, _)) + if e.kind() == std::io::ErrorKind::PermissionDenied => + { + show!(uio_error!( + e, + "cannot open {} for reading", + p.path().quote() + )); + } + Err(e) => return Err(e), + } } } } - + // Copy the attributes from the root directory to the target directory. + copy_attributes(root, target, &options.preserve_attributes)?; Ok(()) } @@ -1206,6 +1225,14 @@ impl OverwriteMode { } } +/// Copy the specified attributes from one path to another. +fn copy_attributes(source: &Path, dest: &Path, attributes: &[Attribute]) -> CopyResult<()> { + for attribute in attributes { + copy_attribute(source, dest, attribute)?; + } + Ok(()) +} + fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResult<()> { let context = &*format!("{} -> {}", source.quote(), dest.quote()); let source_metadata = fs::symlink_metadata(source).context(context)?; @@ -1554,9 +1581,7 @@ fn copy_file( // the user does not have permission to write to the file. fs::set_permissions(dest, dest_permissions).ok(); } - for attribute in &options.preserve_attributes { - copy_attribute(source, dest, attribute)?; - } + copy_attributes(source, dest, &options.preserve_attributes)?; Ok(()) } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 06e591c64..2c7157851 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -48,6 +48,27 @@ static TEST_MOUNT_OTHER_FILESYSTEM_FILE: &str = "mount/DO_NOT_copy_me.txt"; #[cfg(unix)] static TEST_NONEXISTENT_FILE: &str = "nonexistent_file.txt"; +/// Assert that mode, ownership, and permissions of two metadata objects match. +#[cfg(not(windows))] +macro_rules! assert_metadata_eq { + ($m1:expr, $m2:expr) => {{ + assert_eq!($m1.mode(), $m2.mode(), "mode is different"); + assert_eq!($m1.uid(), $m2.uid(), "uid is different"); + assert_eq!($m1.atime(), $m2.atime(), "atime is different"); + assert_eq!( + $m1.atime_nsec(), + $m2.atime_nsec(), + "atime_nsec is different" + ); + assert_eq!($m1.mtime(), $m2.mtime(), "mtime is different"); + assert_eq!( + $m1.mtime_nsec(), + $m2.mtime_nsec(), + "mtime_nsec is different" + ); + }}; +} + #[test] fn test_cp_cp() { let (at, mut ucmd) = at_and_ucmd!(); @@ -1787,20 +1808,7 @@ fn test_copy_through_dangling_symlink_no_dereference_permissions() { { let metadata1 = at.symlink_metadata("dangle"); let metadata2 = at.symlink_metadata("d2"); - assert_eq!(metadata1.mode(), metadata2.mode(), "mode is different"); - assert_eq!(metadata1.uid(), metadata2.uid(), "uid is different"); - assert_eq!(metadata1.atime(), metadata2.atime(), "atime is different"); - assert_eq!( - metadata1.atime_nsec(), - metadata2.atime_nsec(), - "atime_nsec is different" - ); - assert_eq!(metadata1.mtime(), metadata2.mtime(), "mtime is different"); - assert_eq!( - metadata1.mtime_nsec(), - metadata2.mtime_nsec(), - "mtime_nsec is different" - ); + assert_metadata_eq!(metadata1, metadata2); } } @@ -2136,3 +2144,67 @@ fn test_copy_nested_directory_to_itself_disallowed() { .fails() .stderr_only(expected); } + +/// Test for preserving permissions when copying a directory. +#[cfg(not(windows))] +#[test] +fn test_copy_dir_preserve_permissions() { + // Create a directory that has some non-default permissions. + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("d1"); + at.set_mode("d1", 0o0500); + + // Copy the directory, preserving those permissions. + // + // preserve permissions (mode, ownership, timestamps) + // | copy directories recursively + // | | from this source directory + // | | | to this destination + // | | | | + // V V V V + ucmd.args(&["-p", "-R", "d1", "d2"]) + .succeeds() + .no_stderr() + .no_stdout(); + assert!(at.dir_exists("d2")); + + // Assert that the permissions are preserved. + let metadata1 = at.metadata("d1"); + let metadata2 = at.metadata("d2"); + assert_metadata_eq!(metadata1, metadata2); +} + +/// Test for preserving permissions when copying a directory, even in +/// the face of an inaccessible file in that directory. +#[cfg(not(windows))] +#[test] +fn test_copy_dir_preserve_permissions_inaccessible_file() { + // Create a directory that has some non-default permissions and + // contains an inaccessible file. + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("d1"); + at.touch("d1/f"); + at.set_mode("d1/f", 0); + at.set_mode("d1", 0o0500); + + // Copy the directory, preserving those permissions. There should + // be an error message that the file `d1/f` is inaccessible. + // + // preserve permissions (mode, ownership, timestamps) + // | copy directories recursively + // | | from this source directory + // | | | to this destination + // | | | | + // V V V V + ucmd.args(&["-p", "-R", "d1", "d2"]) + .fails() + .status_code(1) + .stderr_only("cp: cannot open 'd1/f' for reading: Permission denied"); + assert!(at.dir_exists("d2")); + assert!(!at.file_exists("d2/f")); + + // Assert that the permissions are preserved. + let metadata1 = at.metadata("d1"); + let metadata2 = at.metadata("d2"); + assert_metadata_eq!(metadata1, metadata2); +} diff --git a/tests/common/util.rs b/tests/common/util.rs index 932ae1c31..b65e6cebf 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -19,7 +19,7 @@ use std::ffi::OsStr; use std::fs::{self, hard_link, File, OpenOptions}; use std::io::{BufWriter, Read, Result, Write}; #[cfg(unix)] -use std::os::unix::fs::{symlink as symlink_dir, symlink as symlink_file}; +use std::os::unix::fs::{symlink as symlink_dir, symlink as symlink_file, PermissionsExt}; #[cfg(windows)] use std::os::windows::fs::{symlink_dir, symlink_file}; #[cfg(windows)] @@ -821,6 +821,20 @@ impl AtPath { s } } + + /// Set the permissions of the specified file. + /// + /// # Panics + /// + /// This function panics if there is an error loading the metadata + /// or setting the permissions of the file. + #[cfg(not(windows))] + pub fn set_mode(&self, filename: &str, mode: u32) { + let path = self.plus(filename); + let mut perms = std::fs::metadata(&path).unwrap().permissions(); + perms.set_mode(mode); + std::fs::set_permissions(&path, perms).unwrap(); + } } /// An environment for running a single uutils test case, serves three functions: