diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index ec92d4bac..14d761cf9 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -8,6 +8,7 @@ // spell-checker:ignore (ToDO) Chmoder cmode fmode fperm fref ugoa RFILE RFILE's use clap::{crate_version, Arg, ArgAction, Command}; +use std::ffi::OsString; use std::fs; use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::Path; @@ -35,14 +36,64 @@ mod options { pub const FILE: &str = "FILE"; } +/// Extract negative modes (starting with '-') from the rest of the arguments. +/// +/// This is mainly required for GNU compatibility, where "non-positional negative" modes are used +/// as the actual positional MODE. Some examples of these cases are: +/// * "chmod -w -r file", which is the same as "chmod -w,-r file" +/// * "chmod -w file -r", which is the same as "chmod -w,-r file" +/// +/// These can currently not be handled by clap. +/// Therefore it might be possible that a pseudo MODE is inserted to pass clap parsing. +/// The pseudo MODE is later replaced by the extracted (and joined) negative modes. +fn extract_negative_modes(mut args: impl uucore::Args) -> (Option, Vec) { + // we look up the args until "--" is found + // "-mode" will be extracted into parsed_cmode_vec + let (parsed_cmode_vec, pre_double_hyphen_args): (Vec, Vec) = + args.by_ref().take_while(|a| a != "--").partition(|arg| { + let arg = if let Some(arg) = arg.to_str() { + arg.to_string() + } else { + return false; + }; + arg.len() >= 2 + && arg.starts_with('-') + && matches!( + arg.chars().nth(1).unwrap(), + 'r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0'..='7' + ) + }); + + let mut clean_args = Vec::new(); + if !parsed_cmode_vec.is_empty() { + // we need a pseudo cmode for clap, which won't be used later. + // this is required because clap needs the default "chmod MODE FILE" scheme. + clean_args.push("w".into()); + } + clean_args.extend(pre_double_hyphen_args); + + if let Some(arg) = args.next() { + // as there is still something left in the iterator, we previously consumed the "--" + // -> add it to the args again + clean_args.push("--".into()); + clean_args.push(arg); + } + clean_args.extend(args); + + let parsed_cmode = Some( + parsed_cmode_vec + .iter() + .map(|s| s.to_str().unwrap()) + .collect::>() + .join(","), + ) + .filter(|s| !s.is_empty()); + (parsed_cmode, clean_args) +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let mut args = args.collect_lossy(); - - // 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 (parsed_cmode, args) = extract_negative_modes(args.skip(1)); // skip binary name let matches = uu_app().after_help(LONG_USAGE).try_get_matches_from(args)?; let changes = matches.get_flag(options::CHANGES); @@ -62,13 +113,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }, None => None, }; - let modes = matches.get_one::(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}") + + let modes = matches.get_one::(options::MODE); + let cmode = if let Some(parsed_cmode) = parsed_cmode { + parsed_cmode } else { - modes.to_string() + modes.unwrap().to_string() // modes is required }; + // FIXME: enable non-utf8 paths let mut files: Vec = matches .get_many::(options::FILE) .map(|v| v.map(ToString::to_string).collect()) @@ -107,6 +159,7 @@ pub fn uu_app() -> Command { .override_usage(format_usage(USAGE)) .args_override_self(true) .infer_long_args(true) + .no_binary_name(true) .arg( Arg::new(options::CHANGES) .long(options::CHANGES) @@ -376,3 +429,34 @@ impl Chmoder { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_extract_negative_modes() { + // "chmod -w -r file" becomes "chmod -w,-r file". clap does not accept "-w,-r" as MODE. + // Therefore, "w" is added as pseudo mode to pass clap. + let (c, a) = extract_negative_modes(vec!["-w", "-r", "file"].iter().map(OsString::from)); + assert_eq!(c, Some("-w,-r".to_string())); + assert_eq!(a, vec!["w", "file"]); + + // "chmod -w file -r" becomes "chmod -w,-r file". clap does not accept "-w,-r" as MODE. + // Therefore, "w" is added as pseudo mode to pass clap. + let (c, a) = extract_negative_modes(vec!["-w", "file", "-r"].iter().map(OsString::from)); + assert_eq!(c, Some("-w,-r".to_string())); + assert_eq!(a, vec!["w", "file"]); + + // "chmod -w -- -r file" becomes "chmod -w -r file", where "-r" is interpreted as file. + // Again, "w" is needed as pseudo mode. + let (c, a) = extract_negative_modes(vec!["-w", "--", "-r", "f"].iter().map(OsString::from)); + assert_eq!(c, Some("-w".to_string())); + assert_eq!(a, vec!["w", "--", "-r", "f"]); + + // "chmod -- -r file" becomes "chmod -r file". + let (c, a) = extract_negative_modes(vec!["--", "-r", "file"].iter().map(OsString::from)); + assert_eq!(c, None); + assert_eq!(a, vec!["--", "-r", "file"]); + } +} diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 956981254..a54824d18 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/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 { diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 83b633400..925643add 100644 --- a/tests/by-util/test_chmod.rs +++ b/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 = 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");