tail: skip clap for obsolete args

src/uu/tail/src/args.rs

@@ -13,7 +13,6 @@ use fundu::DurationParser;
 use is_terminal::IsTerminal;
 use same_file::Handle;
 use std::collections::VecDeque;
-use std::ffi::OsString;
 use std::time::Duration;
 use uucore::error::{UResult, USimpleError, UUsageError};
 use uucore::parse_size::{parse_size, ParseSizeError};
@@ -59,6 +58,19 @@ pub enum FilterMode {
 }
 
 impl FilterMode {
+    fn from_obsolete_args(args: &parse::ObsoleteArgs) -> Self {
+        let signum = if args.plus {
+            Signum::Positive(args.num)
+        } else {
+            Signum::Negative(args.num)
+        };
+        if args.lines {
+            Self::Lines(signum, b'\n')
+        } else {
+            Self::Bytes(signum)
+        }
+    }
+
     fn from(matches: &ArgMatches) -> UResult<Self> {
         let zero_term = matches.get_flag(options::ZERO_TERM);
         let mode = if let Some(arg) = matches.get_one::<String>(options::BYTES) {
@@ -132,6 +144,29 @@ pub struct Settings {
 }
 
 impl Settings {
+    pub fn from_obsolete_args(args: &parse::ObsoleteArgs, name: Option<&str>) -> Self {
+        let mut settings: Self = Self {
+            sleep_sec: Duration::from_secs_f32(1.0),
+            max_unchanged_stats: 5,
+            ..Default::default()
+        };
+        if args.follow {
+            settings.follow = if name.is_some() {
+                Some(FollowMode::Name)
+            } else {
+                Some(FollowMode::Descriptor)
+            };
+        }
+        settings.mode = FilterMode::from_obsolete_args(args);
+        let input = if let Some(name) = name {
+            Input::from(name.to_string())
+        } else {
+            Input::default()
+        };
+        settings.inputs.push_back(input);
+        settings
+    }
+
     pub fn from(matches: &clap::ArgMatches) -> UResult<Self> {
         let mut settings: Self = Self {
             sleep_sec: Duration::from_secs_f32(1.0),
@@ -308,37 +343,24 @@ impl Settings {
     }
 }
 
-pub fn arg_iterate<'a>(
-    mut args: impl uucore::Args + 'a,
-) -> UResult<Box<dyn Iterator<Item = OsString> + 'a>> {
-    // argv[0] is always present
-    let first = args.next().unwrap();
-    if let Some(second) = args.next() {
-        if let Some(s) = second.to_str() {
-            match parse::parse_obsolete(s) {
-                Some(Ok(iter)) => Ok(Box::new(vec![first].into_iter().chain(iter).chain(args))),
-                Some(Err(e)) => Err(USimpleError::new(
-                    1,
-                    match e {
-                        parse::ParseError::Overflow => format!(
-                            "invalid argument: {} Value too large for defined datatype",
-                            s.quote()
-                        ),
-                        parse::ParseError::Context => {
-                            format!(
-                                "option used in invalid context -- {}",
-                                s.chars().nth(1).unwrap_or_default()
-                            )
-                        }
-                    },
-                )),
-                None => Ok(Box::new(vec![first, second].into_iter().chain(args))),
-            }
-        } else {
-            Err(UUsageError::new(1, "bad argument encoding".to_owned()))
-        }
-    } else {
-        Ok(Box::new(vec![first].into_iter()))
+pub fn parse_obsolete(args: &str) -> UResult<Option<parse::ObsoleteArgs>> {
+    match parse::parse_obsolete(args) {
+        Some(Ok(args)) => Ok(Some(args)),
+        None => Ok(None),
+        Some(Err(e)) => Err(USimpleError::new(
+            1,
+            match e {
+                parse::ParseError::OutOfRange => format!(
+                    "invalid number: {}: Numerical result out of range",
+                    args.quote()
+                ),
+                parse::ParseError::Overflow => format!("invalid number: {}", args.quote()),
+                parse::ParseError::Context => format!(
+                    "option used in invalid context -- {}",
+                    args.chars().nth(1).unwrap_or_default()
+                ),
+            },
+        )),
     }
 }
 
@@ -366,9 +388,29 @@ fn parse_num(src: &str) -> Result<Signum, ParseSizeError> {
     })
 }
 
-pub fn parse_args(args: impl uucore::Args) -> UResult<Settings> {
-    let matches = uu_app().try_get_matches_from(arg_iterate(args)?)?;
-    Settings::from(&matches)
+pub fn parse_args(mut args: impl uucore::Args) -> UResult<Settings> {
+    let first = args.next().unwrap();
+    let second = match args.next() {
+        Some(second) => second,
+        None => return Settings::from(&uu_app().try_get_matches_from(vec![first])?),
+    };
+    let second_str = match second.to_str() {
+        Some(second_str) => second_str,
+        None => {
+            let second_string = second.to_string_lossy();
+            return Err(USimpleError::new(
+                1,
+                format!("bad argument encoding: '{second_string}'"),
+            ));
+        }
+    };
+    match parse_obsolete(second_str)? {
+        Some(obsolete_args) => Ok(Settings::from_obsolete_args(&obsolete_args, args.next())),
+        None => {
+            let args = vec![first, second].into_iter().chain(args);
+            Settings::from(&uu_app().try_get_matches_from(args)?)
+        }
+    }
 }
 
 pub fn uu_app() -> Command {
@@ -497,6 +539,8 @@ pub fn uu_app() -> Command {
 
 #[cfg(test)]
 mod tests {
+    use crate::parse::ObsoleteArgs;
+
     use super::*;
 
     #[test]
@@ -528,4 +572,14 @@ mod tests {
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), Signum::Negative(1));
     }
+
+    #[test]
+    fn test_parse_obsolete_settings_f() {
+        let args = ObsoleteArgs { follow: true, ..Default::default() };
+        let result = Settings::from_obsolete_args(&args, None);
+        assert_eq!(result.follow, Some(FollowMode::Descriptor));
+
+        let result = Settings::from_obsolete_args(&args, Some("test".into()));
+        assert_eq!(result.follow, Some(FollowMode::Name));
+    }
 }

src/uu/tail/src/parse.rs

@@ -3,16 +3,34 @@
 //  * For the full copyright and license information, please view the LICENSE
 //  * file that was distributed with this source code.
 
-use std::ffi::OsString;
+#[derive(PartialEq, Eq, Debug, Copy, Clone)]
+pub struct ObsoleteArgs {
+    pub num: u64,
+    pub plus: bool,
+    pub lines: bool,
+    pub follow: bool,
+}
+
+impl Default for ObsoleteArgs {
+    fn default() -> Self {
+        Self {
+            num: 10,
+            plus: false,
+            lines: true,
+            follow: false,
+        }
+    }
+}
 
 #[derive(PartialEq, Eq, Debug)]
 pub enum ParseError {
+    OutOfRange,
     Overflow,
     Context,
 }
 /// Parses obsolete syntax
 /// tail -\[NUM\]\[bl\]\[f\] and tail +\[NUM\]\[bcl\]\[f\] // spell-checker:disable-line
-pub fn parse_obsolete(src: &str) -> Option<Result<impl Iterator<Item = OsString>, ParseError>> {
+pub fn parse_obsolete(src: &str) -> Option<Result<ObsoleteArgs, ParseError>> {
     let mut chars = src.chars();
     let sign = chars.next()?;
     if sign != '+' && sign != '-' {
@@ -21,27 +39,27 @@ pub fn parse_obsolete(src: &str) -> Option<Result<impl Iterator<Item = OsString>
 
     let numbers: String = chars.clone().take_while(|&c| c.is_ascii_digit()).collect();
     let has_num = !numbers.is_empty();
-    let num: usize = if has_num {
+    let num: u64 = if has_num {
         if let Ok(num) = numbers.parse() {
             num
         } else {
-            return Some(Err(ParseError::Overflow));
+            return Some(Err(ParseError::OutOfRange));
         }
     } else {
         10
     };
 
     let mut follow = false;
-    let mut mode = None;
+    let mut mode = 'l';
     let mut first_char = true;
     for char in chars.skip_while(|&c| c.is_ascii_digit()) {
-        if sign == '-' && char == 'c' && !has_num {
-            // special case: -c should be handled by clap (is ambiguous)
+        if !has_num && first_char && sign == '-' && (char == 'c' || char == 'f') {
+            // special cases: -c, -f should be handled by clap (are ambiguous)
             return None;
         } else if char == 'f' {
             follow = true;
         } else if first_char && (char == 'b' || char == 'c' || char == 'l') {
-            mode = Some(char);
+            mode = char;
         } else if has_num && sign == '-' {
             return Some(Err(ParseError::Context));
         } else {
@@ -49,82 +67,81 @@ pub fn parse_obsolete(src: &str) -> Option<Result<impl Iterator<Item = OsString>
         }
         first_char = false;
     }
+    let multiplier = if mode == 'b' { 512 } else { 1 };
+    let num = match num.checked_mul(multiplier) {
+        Some(n) => n,
+        None => return Some(Err(ParseError::Overflow)),
+    };
 
-    let mut options = Vec::new();
-    if follow {
-        options.push(OsString::from("-f"));
-    }
-    let mode = mode.unwrap_or('l');
-    if mode == 'b' || mode == 'c' {
-        options.push(OsString::from("-c"));
-        let n = if mode == 'b' { 512 } else { 1 };
-        let num = match num.checked_mul(n) {
-            Some(n) => n,
-            None => return Some(Err(ParseError::Overflow)),
-        };
-        options.push(OsString::from(format!("{sign}{num}")));
-    } else {
-        options.push(OsString::from("-n"));
-        options.push(OsString::from(format!("{sign}{num}")));
-    }
-    Some(Ok(options.into_iter()))
+    Some(Ok(ObsoleteArgs {
+        num,
+        plus: sign == '+',
+        lines: mode == 'l',
+        follow,
+    }))
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    fn obsolete(src: &str) -> Option<Result<Vec<String>, ParseError>> {
-        let r = parse_obsolete(src);
-        match r {
-            Some(s) => match s {
-                Ok(v) => Some(Ok(v.map(|s| s.to_str().unwrap().to_owned()).collect())),
-                Err(e) => Some(Err(e)),
-            },
-            None => None,
-        }
-    }
-    fn obsolete_result(src: &[&str]) -> Option<Result<Vec<String>, ParseError>> {
-        Some(Ok(src.iter().map(|s| s.to_string()).collect()))
-    }
     #[test]
     fn test_parse_numbers_obsolete() {
-        assert_eq!(obsolete("+2c"), obsolete_result(&["-c", "+2"]));
-        assert_eq!(obsolete("-5"), obsolete_result(&["-n", "-5"]));
-        assert_eq!(obsolete("-100"), obsolete_result(&["-n", "-100"]));
-        assert_eq!(obsolete("-2b"), obsolete_result(&["-c", "-1024"]));
+        assert_eq!(
+            parse_obsolete("+2c"),
+            Some(Ok(ObsoleteArgs {
+                num: 2,
+                plus: true,
+                lines: false,
+                follow: false,
+            }))
+        );
+        assert_eq!(
+            parse_obsolete("-5"),
+            Some(Ok(ObsoleteArgs {
+                num: 5,
+                plus: false,
+                lines: true,
+                follow: false,
+            }))
+        );
+        assert_eq!(
+            parse_obsolete("+100f"),
+            Some(Ok(ObsoleteArgs {
+                num: 100,
+                plus: true,
+                lines: true,
+                follow: true,
+            }))
+        );
+        assert_eq!(
+            parse_obsolete("-2b"),
+            Some(Ok(ObsoleteArgs {
+                num: 1024,
+                plus: false,
+                lines: false,
+                follow: false,
+            }))
+        );
     }
     #[test]
     fn test_parse_errors_obsolete() {
-        assert_eq!(obsolete("-5n"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-5c5"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-1vzc"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-5m"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-1k"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-1mmk"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-105kzm"), Some(Err(ParseError::Context)));
-        assert_eq!(obsolete("-1vz"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-5n"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-5c5"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-1vzc"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-5m"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-1k"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-1mmk"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-105kzm"), Some(Err(ParseError::Context)));
+        assert_eq!(parse_obsolete("-1vz"), Some(Err(ParseError::Context)));
         assert_eq!(
-            obsolete("-1vzqvq"), // spell-checker:disable-line
+            parse_obsolete("-1vzqvq"), // spell-checker:disable-line
             Some(Err(ParseError::Context))
         );
     }
     #[test]
     fn test_parse_obsolete_no_match() {
-        assert_eq!(obsolete("-k"), None);
-        assert_eq!(obsolete("asd"), None);
-        assert_eq!(obsolete("-cc"), None);
-    }
-    #[test]
-    #[cfg(target_pointer_width = "64")]
-    fn test_parse_obsolete_overflow_x64() {
-        assert_eq!(
-            obsolete("-10000000000000000000000"),
-            Some(Err(ParseError::Overflow))
-        );
-    }
-    #[test]
-    #[cfg(target_pointer_width = "32")]
-    fn test_parse_obsolete_overflow_x32() {
-        assert_eq!(obsolete("-42949672960"), Some(Err(ParseError::Overflow)));
+        assert_eq!(parse_obsolete("-k"), None);
+        assert_eq!(parse_obsolete("asd"), None);
+        assert_eq!(parse_obsolete("-cc"), None);
     }
 }

tests/by-util/test_tail.rs

@@ -47,6 +47,13 @@ static FOLLOW_NAME_EXP: &str = "follow_name.expected";
 #[cfg(not(windows))]
 const DEFAULT_SLEEP_INTERVAL_MILLIS: u64 = 1000;
 
+// The binary integer "10000000" is *not* a valid UTF-8 encoding
+// of a character: https://en.wikipedia.org/wiki/UTF-8#Encoding
+#[cfg(unix)]
+const INVALID_UTF8: u8 = 0x80;
+#[cfg(windows)]
+const INVALID_UTF16: u16 = 0xD800;
+
 #[test]
 fn test_invalid_arg() {
     new_ucmd!().arg("--definitely-invalid").fails().code_is(1);
@@ -469,16 +476,13 @@ fn test_follow_non_utf8_bytes() {
 
     // Now append some bytes that are not valid UTF-8.
     //
-    // The binary integer "10000000" is *not* a valid UTF-8 encoding
-    // of a character: https://en.wikipedia.org/wiki/UTF-8#Encoding
-    //
     // We also write the newline character because our implementation
     // of `tail` is attempting to read a line of input, so the
     // presence of a newline character will force the `follow()`
     // function to conclude reading input bytes and start writing them
     // to output. The newline character is not fundamental to this
     // test, it is just a requirement of the current implementation.
-    let expected = [0b10000000, b'\n'];
+    let expected = [INVALID_UTF8, b'\n'];
     at.append_bytes(FOOBAR_TXT, &expected);
 
     child
@@ -4710,4 +4714,100 @@ fn test_gnu_args_err() {
         .no_stdout()
         .stderr_is("tail: option used in invalid context -- 5\n")
         .code_is(1);
+    scene
+        .ucmd()
+        .arg("-9999999999999999999b")
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: invalid number: '-9999999999999999999b'\n")
+        .code_is(1);
+    scene
+        .ucmd()
+        .arg("-999999999999999999999b")
+        .fails()
+        .no_stdout()
+        .stderr_is(
+            "tail: invalid number: '-999999999999999999999b': Numerical result out of range\n",
+        )
+        .code_is(1);
+}
+
+#[test]
+fn test_gnu_args_f() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let mut p = scene
+        .ucmd()
+        .set_stdin(Stdio::piped())
+        .arg("+f")
+        .run_no_wait();
+    p.make_assertion_with_delay(500).is_alive();
+    p.kill()
+        .make_assertion()
+        .with_all_output()
+        .no_stderr()
+        .no_stdout();
+
+    let source = "file";
+    at.touch(source);
+    let mut p = scene.ucmd().args(&["+f", source]).run_no_wait();
+    p.make_assertion_with_delay(500).is_alive();
+    p.kill()
+        .make_assertion()
+        .with_all_output()
+        .no_stderr()
+        .no_stdout();
+}
+
+#[test]
+#[cfg(unix)]
+fn test_obsolete_encoding_unix() {
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+
+    let scene = TestScenario::new(util_name!());
+    let invalid_utf8_arg = OsStr::from_bytes(&[b'-', INVALID_UTF8, b'b']);
+    let valid_utf8_arg = OsStr::from_bytes(&[b'-', b'b']);
+
+    scene
+        .ucmd()
+        .arg(invalid_utf8_arg)
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: bad argument encoding: '-<2D>b'\n")
+        .code_is(1);
+    scene
+        .ucmd()
+        .args(&[valid_utf8_arg, invalid_utf8_arg])
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: bad argument encoding\n")
+        .code_is(1);
+}
+
+#[test]
+#[cfg(windows)]
+fn test_obsolete_encoding_windows() {
+    use std::ffi::OsString;
+    use std::os::windows::ffi::OsStringExt;
+
+    let scene = TestScenario::new(util_name!());
+    let invalid_utf16_arg = OsString::from_wide(&['-' as u16, INVALID_UTF16, 'b' as u16]);
+    let valid_utf16_arg = OsString::from("-b");
+
+    scene
+        .ucmd()
+        .arg(&invalid_utf16_arg)
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: bad argument encoding: '-<2D>b'\n")
+        .code_is(1);
+    scene
+        .ucmd()
+        .args(&[&valid_utf16_arg, &invalid_utf16_arg])
+        .fails()
+        .no_stdout()
+        .stderr_is("tail: bad argument encoding\n")
+        .code_is(1);
 }