diff --git a/src/uucore/locales/en-US.ftl b/src/uucore/locales/en-US.ftl index 6afc1ab7647..33487d7a47f 100644 --- a/src/uucore/locales/en-US.ftl +++ b/src/uucore/locales/en-US.ftl @@ -99,6 +99,7 @@ mode-diag-help-syntax = a mode is either octal, as in 644, or clauses such as u+ # Shared recursive chown/chgrp/chmod diagnostics (uucore::perms) perms-cannot-access = cannot access { $file }: { $error } +perms-cannot-read-directory = cannot read directory { $file }: { $error } perms-cannot-access-replaced = cannot access { $file }: replaced while it was being processed # Encoding/decoding error messages (base32, base64, basenc) diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 3eb3dca9fae..b42d371c716 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -541,9 +541,8 @@ impl ChownExecutor { *ret = 1; if self.verbosity.level != VerbosityLevel::Silent { show_error!( - "cannot read directory {}: {}", - dir_path.quote(), - strip_errno(&e) + "{}", + translate!("perms-cannot-read-directory", "file" => dir_path.quote(), "error" => strip_errno(&e)) ); } return; @@ -632,7 +631,7 @@ impl ChownExecutor { if self.verbosity.level != VerbosityLevel::Silent { show_error!( "{}", - translate!("perms-cannot-access", "file" => entry_path.quote(), "error" => strip_errno(&e)) + translate!("perms-cannot-read-directory", "file" => entry_path.quote(), "error" => strip_errno(&e)) ); } } @@ -662,16 +661,28 @@ impl ChownExecutor { .follow_links(self.traverse_symlinks == TraverseSymlinks::All) .min_depth(1) .into_iter(); + // The directory whose entry came last: walkdir reports a directory it + // cannot read right after that entry, under the same path. The root is + // below min_depth, so it never comes out as an entry: start from it. + let mut last_dir = Some(root.to_path_buf()); // We can't use a for loop because we need to manipulate the iterator inside the loop. while let Some(entry) = iterator.next() { let entry = match entry { Err(e) => { ret = 1; + if self.verbosity.level == VerbosityLevel::Silent { + continue; + } if let Some(path) = e.path() { + let message = if last_dir.as_deref() == Some(path) { + "perms-cannot-read-directory" + } else { + "perms-cannot-access" + }; show_error!( "{}", translate!( - "perms-cannot-access", + message, "file" => path.quote(), "error" => if let Some(error) = e.io_error() { strip_errno(error) @@ -688,6 +699,7 @@ impl ChownExecutor { Ok(entry) => entry, }; let path = entry.path(); + last_dir = entry.file_type().is_dir().then(|| path.to_path_buf()); let Some(meta) = self.obtain_meta(path, self.dereference) else { ret = 1; @@ -807,7 +819,7 @@ impl ChownExecutor { if self.verbosity.level != VerbosityLevel::Silent { show_error!( "{}", - translate!("perms-cannot-access", "file" => path.quote(), "error" => strip_errno(&e)) + translate!("perms-cannot-read-directory", "file" => path.quote(), "error" => strip_errno(&e)) ); } }) diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 3f29e130a69..f74c0573707 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -89,6 +89,43 @@ fn test_fail_silently() { } } +#[test] +#[cfg(unix)] +fn test_chgrp_recursive_unreadable_directory() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::PermissionsExt; + + if getegid().is_root() { + // root can read any directory + return; + } + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir_all("d/a/y"); + set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o311)).unwrap(); + + ucmd.args(&["-R", "--reference=d", "d"]) + .fails_with_code(1) + .stderr_is("chgrp: cannot read directory 'd/a': Permission denied\n"); + + // -f hides the message; the failure still sets the exit status. + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir_all("d/a/y"); + set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o311)).unwrap(); + ucmd.args(&["-f", "-R", "--reference=d", "d"]) + .fails_with_code(1) + .no_output(); + set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o755)).unwrap(); + + // The operand itself. + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("d"); + set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o311)).unwrap(); + ucmd.args(&["-R", "--reference=d", "d"]) + .fails_with_code(1) + .stderr_is("chgrp: cannot read directory 'd': Permission denied\n"); + set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o755)).unwrap(); +} + #[test] fn test_preserve_root() { // It's weird that on OS X, `realpath /etc/..` returns '/private' @@ -336,7 +373,7 @@ fn test_permission_denied() { .arg(group.as_raw().to_string()) .arg("dir") .fails() - .stderr_only("chgrp: cannot access 'dir': Permission denied\n"); + .stderr_only("chgrp: cannot read directory 'dir': Permission denied\n"); } } @@ -355,7 +392,7 @@ fn test_subdir_permission_denied() { .arg(group.as_raw().to_string()) .arg("dir") .fails() - .stderr_only("chgrp: cannot access 'dir/subdir': Permission denied\n"); + .stderr_only("chgrp: cannot read directory 'dir/subdir': Permission denied\n"); } } diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 4d77788792a..353899c6781 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -766,6 +766,46 @@ fn test_chown_owner_group_mix() { .stderr_contains("failed to change"); } +#[test] +#[cfg(unix)] +fn test_chown_recursive_unreadable_directory() { + use std::fs::{Permissions, set_permissions}; + use std::os::unix::fs::PermissionsExt; + + if rustix::process::geteuid().is_root() { + // root can read any directory + return; + } + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("d/a/y"); + set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o311)).unwrap(); + + scene + .ucmd() + .args(&["-R", "--reference=d", "d"]) + .fails_with_code(1) + .stderr_is("chown: cannot read directory 'd/a': Permission denied\n"); + + // -f hides the message; the failure still sets the exit status. + scene + .ucmd() + .args(&["-f", "-R", "--reference=d", "d"]) + .fails_with_code(1) + .no_output(); + + set_permissions(at.plus_as_string("d/a"), Permissions::from_mode(0o755)).unwrap(); + + // The operand itself. + set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o311)).unwrap(); + scene + .ucmd() + .args(&["-R", "--reference=d", "d"]) + .fails_with_code(1) + .stderr_is("chown: cannot read directory 'd': Permission denied\n"); + set_permissions(at.plus_as_string("d"), Permissions::from_mode(0o755)).unwrap(); +} + #[test] fn test_chown_recursive() { let scene = TestScenario::new(util_name!());