From fde84f9c93092f251792b22592264f6240f2e648 Mon Sep 17 00:00:00 2001 From: Joining7943 <111500881+Joining7943@users.noreply.github.com> Date: Wed, 2 Nov 2022 01:11:51 +0100 Subject: [PATCH] tail: Fix parsing of sleep interval to match gnu's tail and do not panic when value is too big. --- src/uu/tail/src/args.rs | 210 ++++++++++++++++++++++++++++++++++++- tests/by-util/test_tail.rs | 48 ++++----- 2 files changed, 229 insertions(+), 29 deletions(-) diff --git a/src/uu/tail/src/args.rs b/src/uu/tail/src/args.rs index 6e972d25f2a..e320028e26e 100644 --- a/src/uu/tail/src/args.rs +++ b/src/uu/tail/src/args.rs @@ -158,13 +158,13 @@ impl Settings { settings.retry = matches.get_flag(options::RETRY) || matches.get_flag(options::FOLLOW_RETRY); - if let Some(s) = matches.get_one::(options::SLEEP_INT) { - settings.sleep_sec = match s.parse::() { - Ok(s) => Duration::from_secs_f32(s), + if let Some(string) = matches.get_one::(options::SLEEP_INT) { + settings.sleep_sec = match parse_duration(string) { + Ok(duration) => duration, Err(_) => { return Err(UUsageError::new( 1, - format!("invalid number of seconds: {}", s.quote()), + format!("invalid number of seconds: '{}'", string), )) } } @@ -362,6 +362,73 @@ fn parse_num(src: &str) -> Result { }) } +/// Parses a string into a [`Duration`] by accepting a [`f64::MAX`] value in the `src` string. +/// +/// However, internally the parsed [`Duration`] is capped at `seconds == u64::MAX`, `nanos (max) == +/// .999999999` and `nanos (min if not 0) == .000000001`. Infinity values like `inf`, `+infinity` +/// etc. are valid input and resolve to `Duration::MAX`. [`Duration::try_from_secs_f64`] stabilizes +/// in Rust 1.66. Try to replace this function with it as soon as MSRV is 1.66 +/// +/// # Arguments +/// +/// * `src` - A string slice that contains a f64 value +/// +/// # Errors +/// +/// This function will return an error when parsing fails (See also [`f64::from_str`]) or the `src` +/// was negative (-0.0 counts as not negative). +/// +/// # Examples +/// +/// ```ignore +/// use std::time::Duration; +/// +/// let duration = parse_duration("+1.09e1").unwrap(); +/// assert_eq!(duration, Duration::new(10, 900_000_000)); +/// ``` +/// +/// We accept the same max values like gnu's tail on the command line by parsing into `f64`. +/// Internally we can only use [`Duration::MAX`] and cap the seconds at [`u64::MAX`]. By doing the +/// conversion like below, we avoid the panic when using [`Duration::from_secs_f64`] because the +/// value is too big. +/// +/// [`f64::from_str`]: https://doc.rust-lang.org/std/primitive.f64.html#method.from_str +fn parse_duration(src: &str) -> Result { + let trimmed = src.trim(); + match src.parse::() { + // We're working here with the string representation instead of num.fract() to avoid further + // floating point precision problems where we can. + Ok(num) if num.is_finite() && num >= 0.0 => match num.to_string().split_once('.') { + Some((_, fract)) => { + let mut multi = 100_000_000; + let mut nanos: u32 = 0; + for c in fract.chars().take(9) { + nanos += c.to_digit(10).unwrap() * multi; + multi /= 10; + } + Ok(Duration::new(num as u64, nanos)) + } + None if num <= u64::MAX as f64 => Ok(Duration::new(num as u64, 0)), + None => Ok(Duration::MAX), + }, + Ok(num) + // If `src > f64::MAX` then `num` would be infinite. To match gnu's tail behavior only + // interpret `src = +inf` etc. as maximum Duration. + if num.is_infinite() + && (trimmed.eq_ignore_ascii_case("inf") + || trimmed.eq_ignore_ascii_case("+inf") + || trimmed.eq_ignore_ascii_case("infinity") + || trimmed.eq_ignore_ascii_case("+infinity")) => + { + Ok(Duration::MAX) + } + Ok(num) if num.is_sign_negative() => Err(String::from("Number was negative")), + // positive infinite (if src > f64::MAX) or NaN. + Ok(_) => Err(String::from("Not a number")), + Err(error) => Err(error.to_string()), + } +} + pub fn parse_args(args: impl uucore::Args) -> UResult { let matches = uu_app().try_get_matches_from(arg_iterate(args)?)?; Settings::from(&matches) @@ -524,4 +591,139 @@ mod tests { assert!(result.is_ok()); assert_eq!(result.unwrap(), Signum::Negative(1)); } + + #[test] + fn test_parse_duration_when_simple_arguments_are_valid() { + let duration = parse_duration("0").unwrap(); + assert!(duration.is_zero()); + + let duration = parse_duration("0.0").unwrap(); + assert!(duration.is_zero()); + + let duration = parse_duration(".0").unwrap(); + assert!(duration.is_zero()); + + let duration = parse_duration("0.").unwrap(); + assert!(duration.is_zero()); + + let duration = parse_duration("1").unwrap(); + assert_eq!(duration, Duration::new(1, 0)); + + let duration = parse_duration("1.0").unwrap(); + assert_eq!(duration, Duration::new(1, 0)); + + let duration = parse_duration("1.1").unwrap(); + assert_eq!(duration, Duration::new(1, 100_000_000)); + + let duration = parse_duration("0.999999999").unwrap(); + assert_eq!(duration, Duration::new(0, 999_999_999)); + + let duration = parse_duration("1.999999999").unwrap(); + assert_eq!(duration, Duration::new(1, 999_999_999)); + + let duration = parse_duration("1234.123456789").unwrap(); + assert_eq!(duration, Duration::new(1234, 123_456_789)); + + let duration = parse_duration(&u64::MAX.to_string()).unwrap(); + assert_eq!(duration, Duration::new(u64::MAX, 0)); + } + + #[test] + fn test_parse_duration_when_arguments_contain_exponent() { + let duration = parse_duration("1.09e1").unwrap(); + assert_eq!(duration, Duration::new(10, 900_000_000)); + + let duration = parse_duration("0.0000000001e1").unwrap(); + assert_eq!(duration, Duration::new(0, 1)); + + let duration = parse_duration("0.0000000001e+1").unwrap(); + assert_eq!(duration, Duration::new(0, 1)); + + let duration = parse_duration("10.00000001e-1").unwrap(); + assert_eq!(duration, Duration::new(1, 1)); + } + + #[test] + fn test_parse_duration_when_precision_of_float_is_insufficient_then_rounds() { + let duration = parse_duration("1.99999999999999999").unwrap(); + assert_eq!(duration, Duration::new(2, 0)); + + let duration = parse_duration(&format!("{}.1", u64::MAX)).unwrap(); + assert_eq!(duration, Duration::new(u64::MAX, 0)); + } + + #[test] + fn test_parse_duration_when_arguments_are_capped_then_max_duration_or_min_nanos() { + let duration = parse_duration("1.00000000001").unwrap(); + assert_eq!(duration, Duration::new(1, 0)); + + let duration = parse_duration("1.9999999999999").unwrap(); + assert_eq!(duration, Duration::new(1, 999_999_999)); + + let duration = parse_duration(&format!("{}", u128::MAX)).unwrap(); + assert_eq!(duration, Duration::MAX); + + let duration = parse_duration(&format!("{}.0", u128::MAX)).unwrap(); + assert_eq!(duration, Duration::MAX); + + let duration = parse_duration(&format!("{}", f64::MAX)).unwrap(); + assert_eq!(duration, Duration::MAX); + + let duration = parse_duration(&format!("{}.0", f64::MAX)).unwrap(); + assert_eq!(duration, Duration::MAX); + } + + #[test] + fn test_parse_duration_when_arguments_have_a_sign() { + let duration = parse_duration("+0").unwrap(); + assert!(duration.is_zero()); + + let duration = parse_duration("+0.0").unwrap(); + assert!(duration.is_zero()); + + let duration = parse_duration("+1.0").unwrap(); + assert_eq!(duration, Duration::new(1, 0)); + + let duration = parse_duration("-0").unwrap(); + assert!(duration.is_zero()); + + let duration = parse_duration("-0.0").unwrap(); + assert!(duration.is_zero()); + } + + #[test] + fn test_parse_duration_when_arguments_are_infinity_values() { + let duration = parse_duration("inf").unwrap(); + assert_eq!(duration, Duration::MAX); + + let duration = parse_duration("+iNf").unwrap(); + assert_eq!(duration, Duration::MAX); + + let duration = parse_duration("infinity").unwrap(); + assert_eq!(duration, Duration::MAX); + + let duration = parse_duration("+InFinitY").unwrap(); + assert_eq!(duration, Duration::MAX); + } + + #[test] + fn test_parse_duration_when_arguments_are_negative_infinity_values_then_error() { + let result = parse_duration("-inf"); + assert!(result.is_err()); + + let result = parse_duration("-infinity"); + assert!(result.is_err()); + + let result = parse_duration("-InFinitY"); + assert!(result.is_err()); + } + + #[test] + fn test_parse_duration_when_arguments_are_negative_then_error() { + let result = parse_duration("-1.0"); + assert!(result.is_err()); + + let result = parse_duration("-0.1"); + assert!(result.is_err()); + } } diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 6ea075541ed..593fb0ca5f5 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -13,6 +13,7 @@ use crate::common::random::*; use crate::common::util::*; use pretty_assertions::assert_eq; use rand::distributions::Alphanumeric; +use rstest::rstest; use std::char::from_digit; use std::fs::File; use std::io::Write; @@ -4497,29 +4498,26 @@ fn test_follow_when_files_are_pointing_to_same_relative_file_and_file_stays_same .stdout_only(expected_stdout); } -#[test] -#[cfg(disable_until_fixed)] -fn test_args_sleep_interval_when_illegal_argument_then_usage_error() { - let scene = TestScenario::new(util_name!()); - for interval in [ - &format!("{}0", f64::MAX), - &format!("{}0.0", f64::MAX), - "1_000", - ".", - "' '", - "", - " ", - "0,0", - "one.zero", - ".zero", - "one.", - "0..0", - ] { - scene - .ucmd() - .args(&["--sleep-interval", interval]) - .run() - .usage_error(format!("invalid number of seconds: '{}'", interval)) - .code_is(1); - } +#[rstest] +#[case::exceed_float_max(&format!("{}0", f64::MAX))] +#[case::exceed_float_max_with_fraction(&format!("{}0.0", f64::MAX))] +#[case::exponent_exceed_float_max("1.0e2048")] +#[case::underscore_delimiter("1_000")] +#[case::only_point(".")] +#[case::space_in_primes("' '")] +#[case::space(" ")] +#[case::empty("")] +#[case::comma_separator("0,0")] +#[case::words_nominator_fract("one.zero")] +#[case::words_fract(".zero")] +#[case::words_nominator("one.")] +#[case::two_points("0..0")] +#[case::seconds_unit("1.0s")] +#[case::circumflex_exponent("1.0e^1000")] +fn test_args_sleep_interval_when_illegal_argument_then_usage_error(#[case] sleep_interval: &str) { + new_ucmd!() + .args(&["--sleep-interval", sleep_interval]) + .run() + .usage_error(format!("invalid number of seconds: '{}'", sleep_interval)) + .code_is(1); }