chmod: fix GNU test 'chmod/usage'

src/uu/chmod/src/chmod.rs

@@ -35,14 +35,72 @@ mod options {
     pub const FILE: &str = "FILE";
 }
 
-#[uucore::main]
-pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    let mut args = args.collect_lossy();
+// "-w -w" is the same as "-w,-w"
+// "-w file -w" is the same as "-w,-w file"
+// "-- -w file" is the same as "-w file"
+fn extract_negative_modes(mut args: impl uucore::Args) -> (Option<String>, Vec<String>) {
+    // argv[0] is always here
+    // we look up the args until "--" is found
+    // "-mode" will be extracted into parsed_cmode_vec
+    let mut parsed_cmode_vec = Vec::new();
+    let mut clean_args: Vec<String> = vec![args.next().unwrap().to_string_lossy().to_string()];
+    let pre_double_hyphen_args: Vec<String> = args
+        .by_ref()
+        .take_while(|a| a != "--")
+        .filter_map(|arg| {
+            let arg = if let Some(arg) = arg.to_str() {
+                arg.to_string()
+            } else {
+                // FIXME: handle non-utf8 args
+                arg.to_string_lossy().to_string()
+            };
+            if arg.len() >= 2
+                && arg.starts_with('-')
+                && matches!(
+                    arg.chars().nth(1).unwrap(),
+                    'r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0'..='7'
+                )
+            {
+                parsed_cmode_vec.push(arg);
+                None
+            } else {
+                Some(arg)
+            }
+        })
+        .collect();
 
-    // 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 first_after_double_hyphen = args.next();
+    let parsed_cmode = if parsed_cmode_vec.is_empty() {
+        clean_args.extend(pre_double_hyphen_args);
+        if let Some(arg) = first_after_double_hyphen {
+            // nothing found but there is an argument after "--", take it
+            clean_args.push("--".to_string());
+            clean_args.push(arg.to_string_lossy().to_string());
+            // FIXME: handle non-utf8 args
+            Some(arg.to_string_lossy().to_string())
+        } else {
+            // nothing found, last chance is a match by clap
+            None
+        }
+    } else {
+        // we need a pseudo cmode for clap, which won't be used later
+        clean_args.push("w".to_string());
+        clean_args.extend(pre_double_hyphen_args);
+        if let Some(arg) = first_after_double_hyphen {
+            // found "--" -> put it and the argument after it back into the args
+            clean_args.push("--".to_string());
+            // FIXME: handle non-utf8 args
+            clean_args.push(arg.to_string_lossy().to_string());
+        }
+        Some(parsed_cmode_vec.join(","))
+    };
+    clean_args.extend(args.map(|a| a.to_string_lossy().to_string()));
+    (parsed_cmode, clean_args)
+}
 
+#[uucore::main]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
+    let (parsed_cmode, args) = extract_negative_modes(args);
     let matches = uu_app().after_help(LONG_USAGE).try_get_matches_from(args)?;
 
     let changes = matches.get_flag(options::CHANGES);
@@ -62,12 +120,14 @@ 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 modes = matches.get_one::<String>(options::MODE);
+    let cmode = if let Some(parsed_cmode) = parsed_cmode {
+        parsed_cmode
+    } else if let Some(modes) = modes {
         modes.to_string()
+    } else {
+        return Err(UUsageError::new(1, "missing mode".to_string()));
     };
     let mut files: Vec<String> = matches
         .get_many::<String>(options::FILE)

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

@@ -122,6 +122,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 {

tests/by-util/test_chmod.rs

@@ -4,9 +4,8 @@ use std::fs::{metadata, set_permissions, OpenOptions, Permissions};
 use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
 use std::sync::Mutex;
 
-extern crate libc;
-use uucore::mode::strip_minus_from_mode;
 extern crate chmod;
+extern crate libc;
 use self::libc::umask;
 
 static TEST_FILE: &str = "file";
@@ -503,35 +502,6 @@ fn test_chmod_symlink_non_existing_file_recursive() {
         .no_stderr();
 }
 
-#[test]
-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 -c -R -w,o+w FILE --preserve-root",
-            "chmod -c -R w,o+w FILE --preserve-root",
-        ),
-        ("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 -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"),
-    ];
-
-    for test in tests {
-        let mut args: Vec<String> = test.0.split(' ').map(|v| v.to_string()).collect();
-        let _mode_had_minus_prefix = strip_minus_from_mode(&mut args);
-        assert_eq!(test.1, args.join(" "));
-    }
-}
-
 #[test]
 fn test_chmod_keep_setgid() {
     for (from, arg, to) in [
@@ -691,7 +661,7 @@ fn test_gnu_options() {
 }
 
 #[test]
-fn test_gnu_reoccuring_options() {
+fn test_gnu_repeating_options() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
     at.touch("file");