Skip to content

Commit

Permalink
tail: Fix parsing of sleep interval to match gnu's tail and do not pa…
Browse files Browse the repository at this point in the history
…nic when value is too big.
  • Loading branch information
Joining7943 authored and sylvestre committed Dec 26, 2022
1 parent b8a755a commit fde84f9
Show file tree
Hide file tree
Showing 2 changed files with 229 additions and 29 deletions.
210 changes: 206 additions & 4 deletions src/uu/tail/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<String>(options::SLEEP_INT) {
settings.sleep_sec = match s.parse::<f32>() {
Ok(s) => Duration::from_secs_f32(s),
if let Some(string) = matches.get_one::<String>(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),
))
}
}
Expand Down Expand Up @@ -362,6 +362,73 @@ fn parse_num(src: &str) -> Result<Signum, ParseSizeError> {
})
}

/// 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<Duration, String> {
let trimmed = src.trim();
match src.parse::<f64>() {
// 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<Settings> {
let matches = uu_app().try_get_matches_from(arg_iterate(args)?)?;
Settings::from(&matches)
Expand Down Expand Up @@ -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());
}
}
48 changes: 23 additions & 25 deletions tests/by-util/test_tail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

0 comments on commit fde84f9

Please sign in to comment.