chmod: Correct chmod -R on dangling symlink and tests (uutils#7618)

* Correct chmod -R on dangling symlink and tests

* Add tests of arg-level symlink to chmod

* Add tests of all symlink flag combos on chmod dangling

* Fix no traverse on dangling symlink

* Add chmod recursive tests of default symlink method

* Add default chmod -H flag tests

* Set chmod default traversal method correctly to -H

* Fix arg symlink chmod case

* Remove extra chmod -H testing

---------

Co-authored-by: Clifford Ressel <EMAIL@gmail.com>

Author: 2 people

src/uu/chmod/src/chmod.rs

@@ -138,7 +138,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         return Err(UUsageError::new(1, "missing operand".to_string()));
     }
 
-    let (recursive, dereference, traverse_symlinks) = configure_symlink_and_recursion(&matches)?;
+    let (recursive, dereference, traverse_symlinks) =
+        configure_symlink_and_recursion(&matches, TraverseSymlinks::First)?;
 
     let chmoder = Chmoder {
         changes,
@@ -259,6 +260,10 @@ impl Chmoder {
                         // Don't try to change the mode of the symlink itself
                         continue;
                     }
+                    if self.recursive && self.traverse_symlinks == TraverseSymlinks::None {
+                        continue;
+                    }
+
                     if !self.quiet {
                         show!(USimpleError::new(
                             1,

src/uucore/src/lib/features/perms.rs

@@ -507,6 +507,7 @@ type GidUidFilterOwnerParser = fn(&ArgMatches) -> UResult<GidUidOwnerFilter>;
 /// Returns the updated `dereference` and `traverse_symlinks` values.
 pub fn configure_symlink_and_recursion(
     matches: &ArgMatches,
+    default_traverse_symlinks: TraverseSymlinks,
 ) -> Result<(bool, bool, TraverseSymlinks), Box<dyn crate::error::UError>> {
     let mut dereference = if matches.get_flag(options::dereference::DEREFERENCE) {
         Some(true) // Follow symlinks
@@ -516,12 +517,13 @@ pub fn configure_symlink_and_recursion(
         None // Default behavior
     };
 
-    let mut traverse_symlinks = if matches.get_flag("L") {
-        TraverseSymlinks::All
+    let mut traverse_symlinks = default_traverse_symlinks;
+    if matches.get_flag("L") {
+        traverse_symlinks = TraverseSymlinks::All
     } else if matches.get_flag("H") {
-        TraverseSymlinks::First
-    } else {
-        TraverseSymlinks::None
+        traverse_symlinks = TraverseSymlinks::First
+    } else if matches.get_flag("P") {
+        traverse_symlinks = TraverseSymlinks::None
     };
 
     let recursive = matches.get_flag(options::RECURSIVE);
@@ -597,7 +599,8 @@ pub fn chown_base(
         .unwrap_or_default();
 
     let preserve_root = matches.get_flag(options::preserve_root::PRESERVE);
-    let (recursive, dereference, traverse_symlinks) = configure_symlink_and_recursion(&matches)?;
+    let (recursive, dereference, traverse_symlinks) =
+        configure_symlink_and_recursion(&matches, TraverseSymlinks::None)?;
 
     let verbosity_level = if matches.get_flag(options::verbosity::CHANGES) {
         VerbosityLevel::Changes

tests/by-util/test_chmod.rs

@@ -878,7 +878,7 @@ fn test_chmod_symlink_target_no_dereference() {
 }
 
 #[test]
-fn test_chmod_symlink_to_dangling_recursive() {
+fn test_chmod_symlink_recursive_final_traversal_flag() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
 
@@ -891,9 +891,41 @@ fn test_chmod_symlink_to_dangling_recursive() {
         .ucmd()
         .arg("755")
         .arg("-R")
+        .arg("-H")
+        .arg("-L")
+        .arg("-H")
+        .arg("-L")
+        .arg("-P")
         .arg(symlink)
-        .fails()
-        .stderr_is("chmod: cannot operate on dangling symlink 'symlink'\n");
+        .succeeds()
+        .no_output();
+    assert_eq!(
+        at.symlink_metadata(symlink).permissions().mode(),
+        get_expected_symlink_permissions(),
+        "Expected symlink permissions: {:o}, but got: {:o}",
+        get_expected_symlink_permissions(),
+        at.symlink_metadata(symlink).permissions().mode()
+    );
+}
+
+#[test]
+fn test_chmod_symlink_to_dangling_recursive_no_traverse() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let dangling_target = "nonexistent_file";
+    let symlink = "symlink";
+
+    at.symlink_file(dangling_target, symlink);
+
+    scene
+        .ucmd()
+        .arg("755")
+        .arg("-R")
+        .arg("-P")
+        .arg(symlink)
+        .succeeds()
+        .no_output();
     assert_eq!(
         at.symlink_metadata(symlink).permissions().mode(),
         get_expected_symlink_permissions(),
@@ -903,9 +935,46 @@ fn test_chmod_symlink_to_dangling_recursive() {
     );
 }
 
+#[test]
+fn test_chmod_dangling_symlink_recursive_combos() {
+    let error_scenarios = [vec!["-R"], vec!["-R", "-H"], vec!["-R", "-L"]];
+
+    for flags in error_scenarios {
+        let scene = TestScenario::new(util_name!());
+        let at = &scene.fixtures;
+
+        let dangling_target = "nonexistent_file";
+        let symlink = "symlink";
+
+        at.symlink_file(dangling_target, symlink);
+
+        let mut ucmd = scene.ucmd();
+        for f in &flags {
+            ucmd.arg(f);
+        }
+        ucmd.arg("u+x")
+            .umask(0o022)
+            .arg(symlink)
+            .fails()
+            .stderr_is("chmod: cannot operate on dangling symlink 'symlink'\n");
+        assert_eq!(
+            at.symlink_metadata(symlink).permissions().mode(),
+            get_expected_symlink_permissions(),
+            "Expected symlink permissions: {:o}, but got: {:o}",
+            get_expected_symlink_permissions(),
+            at.symlink_metadata(symlink).permissions().mode()
+        );
+    }
+}
+
 #[test]
 fn test_chmod_traverse_symlink_combo() {
     let scenarios = [
+        (
+            vec!["-R"], // Should default to "-H"
+            0o100_664,
+            get_expected_symlink_permissions(),
+        ),
         (
             vec!["-R", "-H"],
             0o100_664,