From 281ec190e0688b30619cb6d05c17426440ca10f4 Mon Sep 17 00:00:00 2001 From: xxyzz Date: Sat, 12 Feb 2022 20:10:17 +0800 Subject: [PATCH 1/4] chmod: replace walkdir with std::fs walkdir filters some files that need to be modified, for example, files inside an unreadable folder. --- Cargo.lock | 1 - src/uu/chmod/Cargo.toml | 1 - src/uu/chmod/src/chmod.rs | 16 +++++++++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 805c1c83a..fd036ac64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2385,7 +2385,6 @@ dependencies = [ "clap 3.0.10", "libc", "uucore", - "walkdir", ] [[package]] diff --git a/src/uu/chmod/Cargo.toml b/src/uu/chmod/Cargo.toml index 237368cf4..07cf65d15 100644 --- a/src/uu/chmod/Cargo.toml +++ b/src/uu/chmod/Cargo.toml @@ -18,7 +18,6 @@ path = "src/chmod.rs" clap = { version = "3.0", features = ["wrap_help", "cargo"] } libc = "0.2.42" uucore = { version=">=0.0.11", package="uucore", path="../../uucore", features=["fs", "mode"] } -walkdir = "2.2" [[bin]] name = "chmod" diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 84c5850d6..19c4eced2 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -18,7 +18,6 @@ use uucore::libc::mode_t; #[cfg(not(windows))] use uucore::mode; use uucore::{format_usage, show_error, InvalidEncodingHandling}; -use walkdir::WalkDir; static ABOUT: &str = "Change the mode of each FILE to MODE. With --reference, change the mode of each FILE to that of RFILE."; @@ -227,10 +226,17 @@ impl Chmoder { if !self.recursive { r = self.chmod_file(file).and(r); } else { - for entry in WalkDir::new(&filename).into_iter().filter_map(|e| e.ok()) { - let file = entry.path(); - r = self.chmod_file(file).and(r); - } + r = self.walk_dir(file); + } + } + r + } + + fn walk_dir(&self, file_path: &Path) -> UResult<()> { + let mut r = self.chmod_file(file_path); + if file_path.is_dir() { + for dir_entry in fs::read_dir(file_path)? { + r = self.walk_dir(dir_entry?.path().as_path()); } } r From 9083f236da43175a78908da3c9735338c06287c6 Mon Sep 17 00:00:00 2001 From: xxyzz Date: Sun, 13 Feb 2022 08:55:58 +0800 Subject: [PATCH 2/4] test_chmod: add read permission recursively test --- tests/by-util/test_chmod.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index f3c01722e..bc5fd97e9 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -356,6 +356,26 @@ fn test_chmod_recursive() { } } +#[test] +#[allow(clippy::unreadable_literal)] +fn test_chmod_recursive_read_permission() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + at.mkdir("a/b"); + let mut perms = at.metadata("a/b").permissions(); + perms.set_mode(0o311); + set_permissions(at.plus_as_string("a/b"), perms.clone()).unwrap(); + set_permissions(at.plus_as_string("a"), perms).unwrap(); + + ucmd.arg("-R") + .arg("u+r") + .arg("a") + .succeeds(); + + assert_eq!(at.metadata("a").permissions().mode(), 0o40711); + assert_eq!(at.metadata("a/b").permissions().mode(), 0o40711); +} + #[test] fn test_chmod_non_existing_file() { new_ucmd!() From 3272a590dbfa30eece811c12d801218c6f6f1a79 Mon Sep 17 00:00:00 2001 From: xxyzz Date: Mon, 14 Feb 2022 08:26:14 +0800 Subject: [PATCH 3/4] test_chmod: update expected results of test_chmod_recursive The test succeeds before because the files are read before the permission change and that's inconsistent with GNU chmod. --- tests/by-util/test_chmod.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index bc5fd97e9..6cbe7e887 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -334,19 +334,20 @@ fn test_chmod_recursive() { make_file(&at.plus_as_string("a/b/c/c"), 0o100444); make_file(&at.plus_as_string("z/y"), 0o100444); + // only the permissions of folder `a` and `z` are changed + // folder can't be read after read permission is removed ucmd.arg("-R") .arg("--verbose") .arg("-r,a+w") .arg("a") .arg("z") - .succeeds() - .stdout_contains(&"to 0333 (-wx-wx-wx)") - .stdout_contains(&"to 0222 (-w--w--w-)"); + .fails() + .stderr_is("chmod: Permission denied"); - assert_eq!(at.metadata("z/y").permissions().mode(), 0o100222); - assert_eq!(at.metadata("a/a").permissions().mode(), 0o100222); - assert_eq!(at.metadata("a/b/b").permissions().mode(), 0o100222); - assert_eq!(at.metadata("a/b/c/c").permissions().mode(), 0o100222); + assert_eq!(at.metadata("z/y").permissions().mode(), 0o100444); + assert_eq!(at.metadata("a/a").permissions().mode(), 0o100444); + assert_eq!(at.metadata("a/b/b").permissions().mode(), 0o100444); + assert_eq!(at.metadata("a/b/c/c").permissions().mode(), 0o100444); println!("mode {:o}", at.metadata("a").permissions().mode()); assert_eq!(at.metadata("a").permissions().mode(), 0o40333); assert_eq!(at.metadata("z").permissions().mode(), 0o40333); @@ -367,10 +368,7 @@ fn test_chmod_recursive_read_permission() { set_permissions(at.plus_as_string("a/b"), perms.clone()).unwrap(); set_permissions(at.plus_as_string("a"), perms).unwrap(); - ucmd.arg("-R") - .arg("u+r") - .arg("a") - .succeeds(); + ucmd.arg("-R").arg("u+r").arg("a").succeeds(); assert_eq!(at.metadata("a").permissions().mode(), 0o40711); assert_eq!(at.metadata("a/b").permissions().mode(), 0o40711); From ce385be575fed805dad7f5fc96efe92db1467284 Mon Sep 17 00:00:00 2001 From: xxyzz Date: Sun, 6 Mar 2022 09:38:39 +0800 Subject: [PATCH 4/4] chmod: ignore symbolic links during recursive directory traversal --- src/uu/chmod/src/chmod.rs | 9 ++++++--- tests/by-util/test_chmod.rs | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 19c4eced2..c2b51ae5e 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -234,9 +234,12 @@ impl Chmoder { fn walk_dir(&self, file_path: &Path) -> UResult<()> { let mut r = self.chmod_file(file_path); - if file_path.is_dir() { - for dir_entry in fs::read_dir(file_path)? { - r = self.walk_dir(dir_entry?.path().as_path()); + if !is_symlink(file_path) && file_path.is_dir() { + for dir_entry in file_path.read_dir()? { + let path = dir_entry?.path(); + if !is_symlink(&path) { + r = self.walk_dir(path.as_path()); + } } } r diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 6cbe7e887..373ad97ce 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -473,8 +473,8 @@ fn test_chmod_symlink_non_existing_file_recursive() { let expected_stdout = &format!( // spell-checker:disable-next-line - "mode of '{}' retained as 0755 (rwxr-xr-x)\nneither symbolic link '{}/{}' nor referent has been changed", - test_directory, test_directory, test_symlink + "mode of '{}' retained as 0755 (rwxr-xr-x)", + test_directory ); // '-v': this should succeed without stderr