grep: expand bundled numeric context options

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<Item = OsString>) -> Vec<OsString> {
     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<Item = OsString>) -> Vec<OsString> {
         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<OsString> = args.collect();
@@ -884,23 +887,56 @@ fn expand_num_shorthand(args: impl Iterator<Item = OsString>) -> Vec<OsString> {
             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

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 `-`.