uutils/coreutils
0
0
Fork 0
mirror of https://github.com/uutils/coreutils.git synced 2026-06-10 15:48:22 -07:00
Code Issues Packages Projects Releases Wiki Activity

cp: fix recursive copy of readonly directories (#10529)

Browse Source 
Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
This commit is contained in:
Nikola Lukovic
2026-02-07 14:28:24 +01:00
committed by GitHub
parent 2d5c28f344
commit 31c5102bee
3 changed files with 132 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/uu/cp/src/copydir.rs
+16 -10
View File
@@ -237,6 +237,7 @@ impl Entry {
#[allow(clippy::too_many_arguments)]
/// Copy a single entry during a directory traversal.
/// Returns a bool value indicating whether this function created a directory or not
fn copy_direntry(
progress_bar: Option<&ProgressBar>,
entry: &Entry,
@@ -248,7 +249,7 @@ fn copy_direntry(
copied_destinations: &HashSet<PathBuf>,
copied_files: &mut HashMap<FileInformation, PathBuf>,
created_parent_dirs: &mut HashSet<PathBuf>,
) -> CopyResult<()> {
) -> CopyResult<bool> {
let source_is_symlink = entry_is_symlink;
let source_is_dir = if source_is_symlink && !options.dereference {
false
@@ -276,7 +277,7 @@ fn copy_direntry(
context_for(&entry.source_relative, &entry.local_to_target)
);
}
Ok(())
Ok(true)
};
}
@@ -326,7 +327,7 @@ fn copy_direntry(
// In any other case, there is nothing to do, so we just return to
// continue the traversal.
Ok(())
Ok(false)
}
/// Read the contents of the directory `root` and recursively copy the
@@ -421,7 +422,7 @@ pub(crate) fn copy_directory(
let mut last_iter: Option<DirEntry> = None;
// Keep track of all directories we've created that need permission fixes
let mut dirs_needing_permissions: Vec<(PathBuf, PathBuf)> = Vec::new();
let mut dirs_needing_permissions: Vec<(PathBuf, PathBuf, bool)> = Vec::new();
// Traverse the contents of the directory, copying each one.
for direntry_result in WalkDir::new(root)
@@ -442,7 +443,7 @@ pub(crate) fn copy_directory(
};
let entry = Entry::new(&context, direntry_path, options.no_target_dir)?;
copy_direntry(
let created = copy_direntry(
progress_bar,
&entry,
entry_is_symlink,
@@ -475,12 +476,16 @@ pub(crate) fn copy_directory(
&entry.source_absolute,
&entry.local_to_target,
&options.attributes,
false,
)?;
continue;
}
// Add this directory to our list for permission fixing later
dirs_needing_permissions
.push((entry.source_absolute.clone(), entry.local_to_target.clone()));
dirs_needing_permissions.push((
entry.source_absolute.clone(),
entry.local_to_target.clone(),
created,
));
// If true, last_iter is not a parent of this iter.
// The means we just exited a directory.
@@ -513,6 +518,7 @@ pub(crate) fn copy_directory(
&entry.source_absolute,
&entry.local_to_target,
&options.attributes,
false,
)?;
}
}
@@ -528,8 +534,8 @@ pub(crate) fn copy_directory(
// Fix permissions for all directories we created
// This ensures that even sibling directories get their permissions fixed
for (source_path, dest_path) in dirs_needing_permissions {
copy_attributes(&source_path, &dest_path, &options.attributes)?;
for (source_path, dest_path, created) in dirs_needing_permissions {
copy_attributes(&source_path, &dest_path, &options.attributes, created)?;
}
// Also fix permissions for parent directories,
@@ -538,7 +544,7 @@ pub(crate) fn copy_directory(
let dest = target.join(root.file_name().unwrap());
for (x, y) in aligned_ancestors(root, dest.as_path()) {
if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) {
copy_attributes(&src, y, &options.attributes)?;
copy_attributes(&src, y, &options.attributes, false)?;
}
}
}
src/uu/cp/src/cp.rs
+15 -5
View File
@@ -1550,7 +1550,7 @@ fn copy_source(
if options.parents {
for (x, y) in aligned_ancestors(source, dest.as_path()) {
if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) {
copy_attributes(&src, y, &options.attributes)?;
copy_attributes(&src, y, &options.attributes, false)?;
}
}
}
@@ -1717,11 +1717,21 @@ pub(crate) fn copy_attributes(
source: &Path,
dest: &Path,
attributes: &Attributes,
created: bool,
) -> CopyResult<()> {
let context = &*format!("{} -> {}", source.quote(), dest.quote());
let source_metadata =
fs::symlink_metadata(source).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
let is_explicit = matches!(attributes.mode, Preserve::No { explicit: true });
// preserve is true by default if the destination is created by us and it's a directory
let mode = if !is_explicit && dest.is_dir() && created {
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<()> {
@@ -1759,7 +1769,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
@@ -2573,14 +2583,14 @@ fn copy_file(
.ok()
.filter(|p| p.exists())
.unwrap_or_else(|| source.to_path_buf());
copy_attributes(&src_for_attrs, dest, &options.attributes)?;
copy_attributes(&src_for_attrs, dest, &options.attributes, false)?;
} else if source_is_stream && !source.exists() {
// Some stream files may not exist after we have copied it,
// like anonymous pipes. Thus, we can't really copy its
// attributes. However, this is already handled in the stream
// copy function (see `copy_stream` under platform/linux.rs).
} else {
copy_attributes(source, dest, &options.attributes)?;
copy_attributes(source, dest, &options.attributes, false)?;
}
#[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))]
@@ -2762,7 +2772,7 @@ fn copy_link(
delete_path(dest, options)?;
}
symlink_file(&link, dest, symlinked_files)?;
copy_attributes(source, dest, &options.attributes)
copy_attributes(source, dest, &options.attributes, false)
}
/// Generate an error message if `target` is not the correct `target_type`
tests/by-util/test_cp.rs
+101
View File
@@ -7570,3 +7570,104 @@ fn test_cp_xattr_enotsup_handling() {
std::fs::remove_file(f).ok();
}
}
#[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();
// only verify owner bits on Android
if cfg!(target_os = "android") {
assert_eq!(at.metadata("b").mode() & 0o700, 0o500);
assert_eq!(at.metadata("b/b").mode() & 0o700, 0o500);
assert_eq!(at.metadata("b/b/c").mode() & 0o700, 0o500);
assert_eq!(at.metadata("b/b/c/d").mode() & 0o700, 0o500);
assert_eq!(at.metadata("c").mode() & 0o700, 0o500);
assert_eq!(at.metadata("c/b").mode() & 0o700, 0o500);
assert_eq!(at.metadata("c/b/c").mode() & 0o700, 0o500);
assert_eq!(at.metadata("c/b/c/d").mode() & 0o700, 0o500);
} else {
assert_eq!(at.metadata("b").mode(), 0o40555);
assert_eq!(at.metadata("b/b").mode(), 0o40555);
assert_eq!(at.metadata("b/b/c").mode(), 0o40555);
assert_eq!(at.metadata("b/b/c/d").mode(), 0o40555);
assert_eq!(at.metadata("c").mode(), 0o40555);
assert_eq!(at.metadata("c/b").mode(), 0o40555);
assert_eq!(at.metadata("c/b/c").mode(), 0o40555);
assert_eq!(at.metadata("c/b/c/d").mode(), 0o40555);
}
}
#[test]
#[cfg(not(target_os = "windows"))]
fn test_cp_existing_perm_dir() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
scene
.cmd("mkdir")
.arg("-p")
.arg("-m")
.arg("ug-s,u=rwx,g=rwx,o=rx")
.arg("src/dir")
.umask(0o022)
.succeeds();
scene
.cmd("mkdir")
.arg("-p")
.arg("-m")
.arg("ug-s,u=rwx,g=,o=")
.arg("dst/dir")
.umask(0o022)
.succeeds();
scene.ucmd().arg("-r").arg("src/.").arg("dst/").succeeds();
let mode = at.metadata("dst/dir").mode();
assert_eq!(mode, 0o40700);
}
#[test]
#[cfg(not(target_os = "windows"))]
fn test_cp_gnu_preserve_mode() {
use std::io;
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
scene.cmd("mkdir").arg("d1").succeeds();
scene.cmd("mkdir").arg("d2").succeeds();
scene.cmd("chmod").arg("705").arg("d2").succeeds();
scene
.ucmd()
.arg("--no-preserve=mode")
.arg("-r")
.arg("d2")
.arg("d3")
.set_stdout(io::stdout())
.succeeds();
let d1_mode = at.metadata("d1").mode();
let d3_mode = at.metadata("d3").mode();
assert_eq!(d1_mode, d3_mode);
}