From a7f6b4420a4a28148657c437aed13dda798176bd Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 2 Sep 2021 22:31:49 +0200 Subject: [PATCH] uucore/perms: take traverse_symlinks into account --- src/uucore/src/lib/features/perms.rs | 15 ++++++- tests/by-util/test_chgrp.rs | 65 ++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index b605c0cdf..a4a01c499 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -265,10 +265,21 @@ impl ChownExecutor { } fn dive_into>(&self, root: P) -> i32 { - let mut ret = 0; let root = root.as_ref(); + + // walkdir always dereferences the root directory, so we have to check it ourselves + // TODO: replace with `root.is_symlink()` once it is stable + if self.traverse_symlinks == TraverseSymlinks::None + && std::fs::symlink_metadata(root) + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false) + { + return 0; + } + + let mut ret = 0; let mut iterator = WalkDir::new(root) - .follow_links(self.dereference) + .follow_links(self.traverse_symlinks == TraverseSymlinks::All) .min_depth(1) .into_iter(); // We can't use a for loop because we need to manipulate the iterator inside the loop. diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 0fc73520e..1d047cfe2 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -288,3 +288,68 @@ fn test_subdir_permission_denied() { .stderr_only("chgrp: cannot access 'dir/subdir': Permission denied"); } } + +#[test] +#[cfg(not(target_vendor = "apple"))] +fn test_traverse_symlinks() { + use std::os::unix::prelude::MetadataExt; + let groups = nix::unistd::getgroups().unwrap(); + if groups.len() < 2 { + return; + } + let (first_group, second_group) = (groups[0], groups[1]); + + for &(args, traverse_first, traverse_second) in &[ + (&[][..] as &[&str], false, false), + (&["-H"][..], true, false), + (&["-P"][..], false, false), + (&["-L"][..], true, true), + ] { + let scenario = TestScenario::new("chgrp"); + + let (at, mut ucmd) = (scenario.fixtures.clone(), scenario.ucmd()); + + at.mkdir("dir"); + at.mkdir("dir2"); + at.touch("dir2/file"); + at.mkdir("dir3"); + at.touch("dir3/file"); + at.symlink_dir("dir2", "dir/dir2_ln"); + at.symlink_dir("dir3", "dir3_ln"); + + scenario + .ccmd("chgrp") + .arg(first_group.to_string()) + .arg("dir2/file") + .arg("dir3/file") + .succeeds(); + + assert!(at.plus("dir2/file").metadata().unwrap().gid() == first_group.as_raw()); + assert!(at.plus("dir3/file").metadata().unwrap().gid() == first_group.as_raw()); + + ucmd.arg("-R") + .args(args) + .arg(second_group.to_string()) + .arg("dir") + .arg("dir3_ln") + .succeeds() + .no_stderr(); + + assert_eq!( + at.plus("dir2/file").metadata().unwrap().gid(), + if traverse_second { + second_group.as_raw() + } else { + first_group.as_raw() + } + ); + assert_eq!( + at.plus("dir3/file").metadata().unwrap().gid(), + if traverse_first { + second_group.as_raw() + } else { + first_group.as_raw() + } + ); + } +}