chmod: improve GNU compatibility

src/uu/chmod/src/chmod.rs

@@ -46,9 +46,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     // Before we can parse 'args' with clap (and previously getopts),
     // a possible MODE prefix '-' needs to be removed (e.g. "chmod -x FILE").
-    let mode_had_minus_prefix = mode::strip_minus_from_mode(&mut args);
+    let unstripped_args = mode::strip_minus_from_mode(&mut args);
 
-    let matches = uu_app().after_help(LONG_USAGE).try_get_matches_from(args)?;
+    let matches = uu_app()
+        .after_help(LONG_USAGE)
+        .try_get_matches_from(args.clone())?;
 
     let changes = matches.get_flag(options::CHANGES);
     let quiet = matches.get_flag(options::QUIET);
@@ -67,12 +69,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         },
         None => None,
     };
+
     let modes = matches.get_one::<String>(options::MODE).unwrap(); // should always be Some because required
-    let cmode = if mode_had_minus_prefix {
-        // clap parsing is finished, now put prefix back
-        format!("-{modes}")
-    } else {
+    let cmode = if unstripped_args.is_empty() {
         modes.to_string()
+    } else {
+        unstripped_args.join(" ")
     };
     let mut files: Vec<String> = matches
         .get_many::<String>(options::FILE)

src/uu/mkdir/src/mkdir.rs

@@ -75,7 +75,7 @@ fn strip_minus_from_mode(_args: &mut [String]) -> bool {
 
 #[cfg(not(windows))]
 fn strip_minus_from_mode(args: &mut Vec<String>) -> bool {
-    mode::strip_minus_from_mode(args)
+    !mode::strip_minus_from_mode(args).is_empty()
 }
 
 #[uucore::main]

src/uucore/src/lib/features/mode.rs

@@ -10,6 +10,7 @@
 // spell-checker:ignore (vars) fperm srwx
 
 use libc::{mode_t, umask, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR};
+use std::collections::HashSet;
 
 pub fn parse_numeric(fperm: u32, mut mode: &str, considering_dir: bool) -> Result<u32, String> {
     let (op, pos) = parse_op(mode).map_or_else(|_| (None, 0), |(op, pos)| (Some(op), pos));
@@ -122,6 +123,15 @@ fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) {
             'o' => srwx = ((fperm << 6) & 0o700) | ((fperm << 3) & 0o070) | (fperm & 0o007),
             _ => break,
         };
+        if ch == 'u' || ch == 'g' || ch == 'o' {
+            // symbolic modes only allows perms to be a single letter of 'ugo'
+            // therefore this must either be the first char or it is unexpected
+            if pos != 0 {
+                break;
+            }
+            pos = 1;
+            break;
+        }
         pos += 1;
     }
     if pos == 0 {
@@ -172,24 +182,45 @@ pub fn get_umask() -> u32 {
 // Iterate 'args' and delete the first occurrence
 // of a prefix '-' if it's associated with MODE
 // e.g. "chmod -v -xw -R FILE" -> "chmod -v xw -R FILE"
-pub fn strip_minus_from_mode(args: &mut Vec<String>) -> bool {
-    for arg in args {
+// bubble possible files to the end:
+// $ chmod -gw -w -w -w -w f -w
+// chmod: invalid mode: ‘-gw,-w,-w,-w,-w,-w’
+pub fn strip_minus_from_mode(args: &mut Vec<String>) -> Vec<String> {
+    let mut unstripped = Vec::new();
+    let mut unique_args = HashSet::new();
+    let mut files = Vec::new();
+    for arg in &mut args[1..] {
         if arg == "--" {
             break;
         }
         if let Some(arg_stripped) = arg.strip_prefix('-') {
             if let Some(second) = arg.chars().nth(1) {
                 match second {
-                    'r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0'..='7' => {
+                    'r' | 'w' | 'x' | 'X' | 's' | 't' => {
+                        // these can come more than once
+                        if !unique_args.contains(&second) {
+                            unique_args.insert(second);
+                            unstripped.push(arg.to_string());
+                            *arg = arg_stripped.to_string();
+                        } else {
+                            *arg = String::new();
+                        }
+                    }
+                    'u' | 'g' | 'o' | '0'..='7' => {
+                        unstripped.push(arg.to_string());
                         *arg = arg_stripped.to_string();
-                        return true;
                     }
                     _ => {}
                 }
             }
+        } else {
+            files.push(arg.to_string());
+            *arg = String::new();
         }
     }
-    false
+    args.retain(|s| !s.is_empty());
+    args.append(&mut files);
+    unstripped
 }
 
 #[cfg(test)]

tests/by-util/test_chmod.rs

@@ -508,21 +508,21 @@ fn test_chmod_strip_minus_from_mode() {
     let tests = vec![
         // ( before, after )
         ("chmod -v -xw -R FILE", "chmod -v xw -R FILE"),
-        ("chmod g=rwx FILE -c", "chmod g=rwx FILE -c"),
+        ("chmod g=rwx FILE -c", "chmod -c g=rwx FILE"),
         (
             "chmod -c -R -w,o+w FILE --preserve-root",
-            "chmod -c -R w,o+w FILE --preserve-root",
+            "chmod -c -R w,o+w --preserve-root FILE",
         ),
         ("chmod -c -R +w FILE ", "chmod -c -R +w FILE "),
         ("chmod a=r,=xX FILE", "chmod a=r,=xX FILE"),
         (
             "chmod -v --reference REF_FILE -R FILE",
-            "chmod -v --reference REF_FILE -R FILE",
+            "chmod -v --reference -R REF_FILE FILE",
         ),
         ("chmod -Rvc -w-x FILE", "chmod -Rvc w-x FILE"),
-        ("chmod 755 -v FILE", "chmod 755 -v FILE"),
-        ("chmod -v +0004 FILE -R", "chmod -v +0004 FILE -R"),
-        ("chmod -v -0007 FILE -R", "chmod -v 0007 FILE -R"),
+        ("chmod 755 -v FILE", "chmod -v 755 FILE"),
+        ("chmod -v +0004 FILE -R", "chmod -v -R +0004 FILE"),
+        ("chmod -v -0007 FILE -R", "chmod -v 0007 -R FILE"),
     ];
 
     for test in tests {