From 8a8664d1271fbfae8fb3f957dc5e00f910727851 Mon Sep 17 00:00:00 2001 From: Alessio Attilio Date: Fri, 18 Sep 2026 00:18:47 +0200 Subject: [PATCH] uucore: stop option parsing at first operand when POSIXLY_CORRECT is set --- src/uu/mktemp/src/mktemp.rs | 16 +- src/uu/rm/src/rm.rs | 2 +- src/uu/tail/src/args.rs | 3 +- src/uu/uniq/src/uniq.rs | 1 + src/uucore/src/lib/mods/clap_localization.rs | 237 ++++++++++++++++++- tests/by-util/test_cat.rs | 14 ++ tests/by-util/test_du.rs | 13 + tests/by-util/test_ls.rs | 14 ++ tests/by-util/test_rm.rs | 15 ++ 9 files changed, 299 insertions(+), 16 deletions(-) diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 887798c44a1..d4500d5aeaf 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -8,7 +8,7 @@ use clap::builder::{TypedValueParser, ValueParserFactory}; use clap::{Arg, ArgAction, ArgMatches, Command}; use uucore::display::{Quotable, println_verbatim}; -use uucore::error::{FromIo, UError, UResult, UUsageError}; +use uucore::error::{FromIo, UError, UResult}; use uucore::format_usage; use uucore::translate; @@ -381,7 +381,7 @@ impl ValueParserFactory for OptionalPathBufParser { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let args: Vec<_> = args.collect(); + let args = uucore::clap_localization::prepare_args(&uu_app(), args); let matches = uu_app().try_get_matches_from(&args).map_err(|e| { use clap::error::{ContextKind, ContextValue, ErrorKind}; use uucore::clap_localization::handle_clap_error_with_exit_code; @@ -393,7 +393,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { k == ContextKind::InvalidArg && v == &ContextValue::String("[template]".into()) }) => { - UUsageError::new(1, translate!("mktemp-error-too-many-templates")) + Box::new(MkTempError::TooManyTemplates) as Box } _ => e.into(), } @@ -403,16 +403,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // application logic. let options = Options::from(&matches); - if env::var_os("POSIXLY_CORRECT").is_some() { - // If POSIXLY_CORRECT was set, template MUST be the last argument. - if matches.contains_id(ARG_TEMPLATE) { - // Template argument was provided, check if was the last one. - if args.last().unwrap() != &options.template { - return Err(Box::new(MkTempError::TooManyTemplates)); - } - } - } - let dry_run = options.dry_run; let suppress_file_err = options.quiet; let make_dir = options.directory; diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index eada6c3e34c..b500bd6825d 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -235,7 +235,7 @@ static ARG_FILES: &str = "files"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let args: Vec = args.collect(); + let args: Vec = uucore::clap_localization::prepare_args(&uu_app(), args); let matches = uu_app() .try_get_matches_from(args.iter()) .map_err(|e| handle_parse_error(e, &args))?; diff --git a/src/uu/tail/src/args.rs b/src/uu/tail/src/args.rs index 70f5d09d0f0..f060683d32b 100644 --- a/src/uu/tail/src/args.rs +++ b/src/uu/tail/src/args.rs @@ -427,7 +427,8 @@ fn parse_num(src: &str) -> Result { pub fn parse_args(args: impl uucore::Args) -> UResult { let args_vec: Vec = args.collect(); - let clap_args = uu_app().try_get_matches_from(args_vec.clone()); + let prepared_args = uucore::clap_localization::prepare_args(&uu_app(), args_vec.clone()); + let clap_args = uu_app().try_get_matches_from(prepared_args); let clap_result = match clap_args { // Kept for the caret in size diagnostics, which needs the value as // typed. diff --git a/src/uu/uniq/src/uniq.rs b/src/uu/uniq/src/uniq.rs index c6e1856c567..bdda151e7dc 100644 --- a/src/uu/uniq/src/uniq.rs +++ b/src/uu/uniq/src/uniq.rs @@ -655,6 +655,7 @@ fn map_clap_errors(clap_error: Error) -> Box { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let (args, skip_fields_old, skip_chars_old) = handle_obsolete(args); + let args = uucore::clap_localization::prepare_args(&uu_app(), args); let matches = match uu_app().try_get_matches_from(args) { Ok(matches) => matches, diff --git a/src/uucore/src/lib/mods/clap_localization.rs b/src/uucore/src/lib/mods/clap_localization.rs index 19f3f71d47f..03967933416 100644 --- a/src/uucore/src/lib/mods/clap_localization.rs +++ b/src/uucore/src/lib/mods/clap_localization.rs @@ -473,7 +473,8 @@ where I: IntoIterator, T: Into + Clone, { - cmd.try_get_matches_from(itr).map_err(|e| { + let args = prepare_args(&cmd, itr); + cmd.try_get_matches_from(args).map_err(|e| { if e.exit_code() == 0 { e.into() // Preserve help/version } else { @@ -484,6 +485,135 @@ where }) } +fn opt_takes_value(arg: &clap::Arg) -> bool { + if !arg.get_action().takes_values() { + return false; + } + if let Some(num_args) = arg.get_num_args() + && num_args.min_values() == 0 + { + return false; + } + true +} + +fn find_long_opt<'a>(cmd: &'a Command, name: &str) -> Option<&'a clap::Arg> { + for arg in cmd.get_arguments() { + if arg.get_long() == Some(name) { + return Some(arg); + } + if let Some(aliases) = arg.get_all_aliases() + && aliases.contains(&name) + { + return Some(arg); + } + } + let matches: Vec<_> = cmd + .get_arguments() + .filter(|a| { + a.get_long().is_some_and(|l| l.starts_with(name)) + || a.get_all_aliases() + .is_some_and(|aliases| aliases.iter().any(|l| l.starts_with(name))) + }) + .collect(); + if matches.len() == 1 { + return Some(matches[0]); + } + None +} + +fn find_short_opt(cmd: &Command, c: char) -> Option<&clap::Arg> { + for arg in cmd.get_arguments() { + if arg.get_short() == Some(c) { + return Some(arg); + } + if let Some(aliases) = arg.get_short_and_visible_aliases() + && aliases.contains(&c) + { + return Some(arg); + } + if let Some(aliases) = arg.get_all_short_aliases() + && aliases.contains(&c) + { + return Some(arg); + } + } + None +} + +pub fn prepare_args(cmd: &Command, itr: I) -> Vec +where + I: IntoIterator, + T: Into, +{ + let mut args: Vec = itr.into_iter().map(Into::into).collect(); + if std::env::var_os("POSIXLY_CORRECT").is_none() { + return args; + } + let cmd_name = cmd.get_name(); + if cmd_name == "join" || cmd_name == "pr" { + return args; + } + if args.len() <= 1 { + return args; + } + + let mut i = 1; + while i < args.len() { + let Ok(arg_bytes) = crate::os_str_as_bytes(args[i].as_os_str()) else { + args.insert(i, OsString::from("--")); + return args; + }; + + if arg_bytes == b"--" { + return args; + } + + if arg_bytes == b"-" || !arg_bytes.starts_with(b"-") { + args.insert(i, OsString::from("--")); + return args; + } + + if arg_bytes.starts_with(b"--") { + let opt_bytes = &arg_bytes[2..]; + if opt_bytes.contains(&b'=') { + i += 1; + } else if let Ok(opt_str) = std::str::from_utf8(opt_bytes) { + let takes_val = find_long_opt(cmd, opt_str).is_some_and(opt_takes_value); + if takes_val && i + 1 < args.len() { + i += 2; + } else { + i += 1; + } + } else { + i += 1; + } + } else { + let short_bytes = &arg_bytes[1..]; + let mut consumed_next = false; + if let Ok(short_str) = std::str::from_utf8(short_bytes) { + let chars: Vec = short_str.chars().collect(); + for (idx, &c) in chars.iter().enumerate() { + if let Some(arg) = find_short_opt(cmd, c) + && opt_takes_value(arg) + { + if idx + 1 == chars.len() && i + 1 < args.len() { + consumed_next = true; + } + break; + } + } + } + if consumed_next { + i += 2; + } else { + i += 1; + } + } + } + args +} + /// Handles a clap error directly with a custom exit code. /// /// This function processes a clap error and exits the program with the specified @@ -735,5 +865,110 @@ mod tests { } } } + + #[test] + fn test_prepare_args_posixly_correct() { + use std::env; + let cmd = Command::new("test") + .arg( + Arg::new("verbose") + .short('v') + .long("verbose") + .action(clap::ArgAction::SetTrue), + ) + .arg(Arg::new("width").short('w').long("width").value_name("NUM")) + .arg( + Arg::new("files") + .action(clap::ArgAction::Append) + .num_args(1..), + ); + + unsafe { + env::remove_var("POSIXLY_CORRECT"); + } + let args = vec!["test", "file", "-v"]; + let prepared = prepare_args(&cmd, args.clone()); + assert_eq!( + prepared, + args.iter().map(OsString::from).collect::>() + ); + + unsafe { + env::set_var("POSIXLY_CORRECT", "1"); + } + let prepared = prepare_args(&cmd, vec!["test", "file", "-v"]); + assert_eq!( + prepared, + vec![ + OsString::from("test"), + OsString::from("--"), + OsString::from("file"), + OsString::from("-v") + ] + ); + + let prepared = prepare_args(&cmd, vec!["test", "-w", "80", "file", "-v"]); + assert_eq!( + prepared, + vec![ + OsString::from("test"), + OsString::from("-w"), + OsString::from("80"), + OsString::from("--"), + OsString::from("file"), + OsString::from("-v") + ] + ); + + let prepared = prepare_args(&cmd, vec!["test", "-w80", "file", "-v"]); + assert_eq!( + prepared, + vec![ + OsString::from("test"), + OsString::from("-w80"), + OsString::from("--"), + OsString::from("file"), + OsString::from("-v") + ] + ); + + let prepared = prepare_args(&cmd, vec!["test", "--width=80", "file", "-v"]); + assert_eq!( + prepared, + vec![ + OsString::from("test"), + OsString::from("--width=80"), + OsString::from("--"), + OsString::from("file"), + OsString::from("-v") + ] + ); + + let prepared = prepare_args(&cmd, vec!["test", "--", "file", "-v"]); + assert_eq!( + prepared, + vec![ + OsString::from("test"), + OsString::from("--"), + OsString::from("file"), + OsString::from("-v") + ] + ); + + let join_cmd = Command::new("join"); + let prepared_join = prepare_args(&join_cmd, vec!["join", "file", "-v"]); + assert_eq!( + prepared_join, + vec![ + OsString::from("join"), + OsString::from("file"), + OsString::from("-v") + ] + ); + + unsafe { + env::remove_var("POSIXLY_CORRECT"); + } + } } /* spell-checker: enable */ diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index 5612b7b5ca3..2ca89797f3c 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -957,3 +957,17 @@ fn test_cat_eintr_handling() { // Verify that the interruption was encountered and handled assert_eq!(*interrupt_count.lock().unwrap(), 1); } + +#[test] +fn test_posixly_correct_options_after_operands() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("data.txt", "hello\n"); + + ucmd.env("POSIXLY_CORRECT", "1") + .arg("data.txt") + .arg("-n") + .fails() + .code_is(1) + .stdout_is("hello\n") + .stderr_contains("-n"); +} diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 470382848fe..c8c900ce51d 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -2913,4 +2913,17 @@ du: invalid suffix in --block-size argument '1fb' .fails_with_code(1) .stderr_is("du: invalid suffix in --block-size argument '1fb'\n"); } + + #[test] + fn test_posixly_correct_options_after_operands() { + let (at, mut ucmd) = uutests::at_and_ucmd!(); + at.mkdir("dir"); + + ucmd.env("POSIXLY_CORRECT", "1") + .arg("dir") + .arg("-s") + .fails() + .code_is(1) + .stderr_contains("-s"); + } } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 817d1b4052e..b4ef2b31e48 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -8064,4 +8064,18 @@ ls: invalid --block-size argument '1fb' .fails_with_code(2) .stderr_is("ls: invalid --block-size argument '1fb'\n"); } + + #[test] + fn test_posixly_correct_options_after_operands() { + let (at, mut ucmd) = uutests::at_and_ucmd!(); + at.touch("file"); + + ucmd.env("POSIXLY_CORRECT", "1") + .arg("file") + .arg("-l") + .fails() + .code_is(2) + .stdout_is("file\n") + .stderr_contains("-l"); + } } diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 6f52610341b..f42ae1691a0 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -1910,3 +1910,18 @@ fn test_dash_hint_is_shell_escaped() { .fails_with_code(1) .stderr_contains("./'-a'$'\\t''b'\\''c'' to remove the file '-a'$'\\t''b'\\''c'."); } + +#[test] +fn test_posixly_correct_options_after_operands() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("test_dir"); + at.touch("test_dir/file"); + + ucmd.env("POSIXLY_CORRECT", "1") + .arg("test_dir") + .arg("-rf") + .fails() + .code_is(1); + + assert!(at.dir_exists("test_dir")); +}