diff --git a/src/lib.rs b/src/lib.rs index fbf25c3..830a5e4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -837,6 +837,8 @@ pub fn uu_app() -> Command { } /// Expand GNU grep's `-NUM` shorthand to `-C NUM`. +/// +/// Note that `-a12b` means `-a -C 12 -b`. fn expand_num_shorthand(args: impl Iterator) -> Vec { const SHORT_OPTS_WITH_VALUE: &[u8] = b"efmABCDd"; const LONG_OPTS_WITH_VALUE: &[&[u8]] = &[ @@ -857,13 +859,14 @@ fn expand_num_shorthand(args: impl Iterator) -> Vec { b"group-separator", ]; - fn consumes_next_arg(arg: &[u8]) -> bool { - if let Some(name) = arg.strip_prefix(b"-") { - LONG_OPTS_WITH_VALUE.contains(&name) - } else { - arg.last() - .is_some_and(|b| SHORT_OPTS_WITH_VALUE.contains(b)) - } + // Rebuild a single option argument out of `prefix` and `bytes`. + fn option(prefix: &str, bytes: &[u8]) -> OsString { + let mut buf = Vec::with_capacity(prefix.len() + bytes.len()); + buf.extend_from_slice(prefix.as_bytes()); + buf.extend_from_slice(bytes); + // SAFETY: `bytes` is a subslice of an OsStr's encoded bytes + // that starts and ends on ASCII code point boundary. + unsafe { OsString::from_encoded_bytes_unchecked(buf) } } let mut out: Vec = args.collect(); @@ -884,23 +887,56 @@ fn expand_num_shorthand(args: impl Iterator) -> Vec { break; } - // This flag consumes two args. Skip both. - if consumes_next_arg(arg) { - i += 2; + // Skip --long options. + if let Some(name) = arg.strip_prefix(b"-") { + i += 1 + usize::from(LONG_OPTS_WITH_VALUE.contains(&name)); continue; } - // Translate -NUM to -C NUM. - if !arg.is_empty() && arg.iter().all(u8::is_ascii_digit) { - // SAFETY: We know that all `arg` bytes are valid ASCII (digits). - let digits = unsafe { OsStr::from_encoded_bytes_unchecked(arg) }.to_owned(); - out[i] = OsString::from("-C"); - out.insert(i + 1, digits); - i += 2; + // Find the first byte that isn't a plain short option: a digit to + // translate, or a byte that claims the rest of the argument (an option + // taking a value like -ePATTERN, or something that's no option at all). + let Some(at) = arg + .iter() + .position(|b| b.is_ascii_digit() || !b.is_ascii() || SHORT_OPTS_WITH_VALUE.contains(b)) + else { + // Plain short options only. + i += 1; continue; + }; + + if !arg[at].is_ascii_digit() { + // Nothing to rewrite. A value option with nothing + // attached takes the next argument instead. + let consumes_next = at + 1 == arg.len() && SHORT_OPTS_WITH_VALUE.contains(&arg[at]); + i += 1 + usize::from(consumes_next); + continue; + } + + // Translate -aNUMb to -a -C NUM -b. option() only deals with ASCII and + // we don't need to deal with anything else either, so the translation + // stops at the byte that claims the rest of the argument. + let end = arg[at..] + .iter() + .position(|b| !b.is_ascii() || SHORT_OPTS_WITH_VALUE.contains(b)) + .map_or(arg.len(), |n| at + n); + let (options, rest) = arg.split_at(end); + let consumes_next = rest.len() == 1 && SHORT_OPTS_WITH_VALUE.contains(&rest[0]); + let mut expanded = Vec::with_capacity(options.len() + 1); + + for chunk in options.chunk_by(|a, b| a.is_ascii_digit() == b.is_ascii_digit()) { + let prefix = if chunk[0].is_ascii_digit() { "-C" } else { "-" }; + expanded.push(option(prefix, chunk)); + } + + // The tail holds the value option with its value, if any. + if !rest.is_empty() { + expanded.push(option("-", rest)); } - i += 1; + let len = expanded.len(); + out.splice(i..=i, expanded); + i += len + usize::from(consumes_next); } out diff --git a/tests/test_grep.rs b/tests/test_grep.rs index e250165..717f44c 100644 --- a/tests/test_grep.rs +++ b/tests/test_grep.rs @@ -931,6 +931,102 @@ fn num_shorthand_is_context() { .stdout_only("a\nb\nMATCH\nc\nd\n"); } +#[test] +fn num_shorthand_in_short_option_clusters() { + // Five lines of leading and trailing context around the match. + const CTX: &str = "zero\none\ntwo\nthree\nfour\nMATCH\nsix\nseven\neight\nnine\nten\n"; + let ok = |args: &[&str], expected: &str| { + let (_s, mut c) = ucmd(); + c.args(args).pipe_in(CTX).succeeds().stdout_only(expected); + }; + + // A shorthand bundled behind another short option (the reported case). + ok(&["-w2", "MATCH"], "three\nfour\nMATCH\nsix\nseven\n"); + ok(&["-i2", "match"], "three\nfour\nMATCH\nsix\nseven\n"); + + // A run of digits is a single number, so `-w44` is 44 lines of context + // rather than two separate `-4` options. + ok(&["-w44", "MATCH"], CTX); + + // Runs split by another option stay separate values, and the last wins. + ok(&["-w1x2", "MATCH"], "three\nfour\nMATCH\nsix\nseven\n"); + + // Order is preserved, so a later option only overrides what it names. + ok(&["-w2C1", "MATCH"], "four\nMATCH\nsix\n"); + ok(&["-2A1", "MATCH"], "three\nfour\nMATCH\nsix\n"); + + // Regression for the original fuzzer case: trailing flags in the same + // cluster are still parsed after the numeric options. + let (_s, mut c) = ucmd(); + c.args(&["-e", "a", "-w44ov"]) + .pipe_in("a\nb\n") + .succeeds() + .stdout_only(""); +} + +#[test] +fn num_shorthand_does_not_capture_option_values() { + let ok = |args: &[&str], input: &str, expected: &str| { + let (_s, mut c) = ucmd(); + c.args(args).pipe_in(input).succeeds().stdout_only(expected); + }; + + // Digits attached to a value-taking short option are its value. + ok(&["-e123"], "123\n456\n", "123\n"); + ok(&["-m2", "x"], "x\nx\nx\n", "x\nx\n"); + ok(&["-wm2", "x"], "x\nx\nx\n", "x\nx\n"); + + // Only the first value option in a cluster takes a value, so the trailing + // `e` is `-e`'s value here and the following `-2` is still a shorthand. + ok(&["-ee", "-2"], "a\nbee\nc\nd\n", "a\nbee\nc\nd\n"); + + // A detached value is left alone, even after an expanded cluster. + ok(&["-e", "-2"], "-2\nx\n", "-2\n"); + ok(&["--regexp", "-2"], "-2\nx\n", "-2\n"); + ok(&["-i2e", "-3"], "a\n-3\nb\n", "a\n-3\nb\n"); + + // Non-option arguments are never shorthand. + ok(&["--regexp=-2"], "-2\nx\n", "-2\n"); + ok(&["--", "-2"], "-2\nx\n", "-2\n"); + ok(&["w2"], "w2\nw3\n", "w2\n"); +} + +#[test] +fn num_shorthand_allows_non_ascii_attached_values() { + // A non-ASCII attached value is legitimate: `-1e🙂` is `-C 1 -e 🙂`. + let ok = |args: &[&str]| { + let (_s, mut c) = ucmd(); + c.args(args) + .pipe_in("a\n🙂\nb\nc\n") + .succeeds() + .stdout_only("a\n🙂\nb\n"); + }; + ok(&["-1e🙂"]); + ok(&["-w1e🙂"]); + + // A digit only reachable past a non-ASCII byte is not a shorthand, leaving + // the cluster the invalid option it would be without the digit. + let fails = |args: &[&str]| { + let (_s, mut c) = ucmd(); + c.args(args).pipe_in("x\n").fails().code_is(2); + }; + fails(&["-🙂2", "x"]); + fails(&["-2🙂3", "x"]); +} + +#[cfg(unix)] +#[test] +fn num_shorthand_preserves_invalid_utf8_arguments() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + + // Arguments that are not valid UTF-8 reach clap without being sliced apart. + let (_s, mut c) = ucmd(); + c.arg(OsString::from_vec(vec![b'-', b'w', b'2', 0xff])) + .arg("x") + .pipe_in("x\n") + .fails(); +} #[test] fn context_line_prefixes_use_dash() { // Match line uses `:`, context lines use `-`.