From 94a26e170ecda3d62d3194166a3e90a37c16de58 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Fri, 4 Apr 2025 10:42:31 +0200 Subject: [PATCH 1/4] uucore: parser: parse_time: Handle infinity and nan There were some missing corner cases when handling infinity and nan: - inf/infinity can be capitalized - nan must always be rejected, even if a suffix is provided Also, return Duration::MAX with infinite values, just for consistency (num.fract() returns 0 for infinity so technically we were just short of that). Add unit tests too. Fixes some of #7475. --- .../src/lib/features/parser/parse_time.rs | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/uucore/src/lib/features/parser/parse_time.rs b/src/uucore/src/lib/features/parser/parse_time.rs index 085dfac81..e0b4a6dc7 100644 --- a/src/uucore/src/lib/features/parser/parse_time.rs +++ b/src/uucore/src/lib/features/parser/parse_time.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (vars) NANOS numstr +// spell-checker:ignore (vars) NANOS numstr infinityh INFD nans nanh //! Parsing a duration from a string. //! //! Use the [`from_str`] function to parse a [`Duration`] from a string. @@ -58,22 +58,23 @@ pub fn from_str(string: &str) -> Result { 'h' => (slice, 60 * 60), 'd' => (slice, 60 * 60 * 24), val if !val.is_alphabetic() => (string, 1), - _ => { - if string == "inf" || string == "infinity" { - ("inf", 1) - } else { - return Err(format!("invalid time interval {}", string.quote())); - } - } + _ => match string.to_ascii_lowercase().as_str() { + "inf" | "infinity" => ("inf", 1), + _ => return Err(format!("invalid time interval {}", string.quote())), + }, }; let num = numstr .parse::() .map_err(|e| format!("invalid time interval {}: {}", string.quote(), e))?; - if num < 0. { + if num < 0. || num.is_nan() { return Err(format!("invalid time interval {}", string.quote())); } + if num.is_infinite() { + return Ok(Duration::MAX); + } + const NANOS_PER_SEC: u32 = 1_000_000_000; let whole_secs = num.trunc(); let nanos = (num.fract() * (NANOS_PER_SEC as f64)).trunc(); @@ -127,6 +128,24 @@ mod tests { assert!(from_str("-1").is_err()); } + #[test] + fn test_infinity() { + assert_eq!(from_str("inf"), Ok(Duration::MAX)); + assert_eq!(from_str("infinity"), Ok(Duration::MAX)); + assert_eq!(from_str("infinityh"), Ok(Duration::MAX)); + assert_eq!(from_str("INF"), Ok(Duration::MAX)); + assert_eq!(from_str("INFs"), Ok(Duration::MAX)); + } + + #[test] + fn test_nan() { + assert!(from_str("nan").is_err()); + assert!(from_str("nans").is_err()); + assert!(from_str("-nanh").is_err()); + assert!(from_str("NAN").is_err()); + assert!(from_str("-NAN").is_err()); + } + /// Test that capital letters are not allowed in suffixes. #[test] fn test_no_capital_letters() { @@ -134,5 +153,6 @@ mod tests { assert!(from_str("1M").is_err()); assert!(from_str("1H").is_err()); assert!(from_str("1D").is_err()); + assert!(from_str("INFD").is_err()); } } From 1d7e0eccc8f2cdba5c8ed592f2ebae307e716171 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Fri, 4 Apr 2025 12:04:47 +0200 Subject: [PATCH 2/4] uucore: parser: num_parser: Do not Underflow/Overflow when parsing 0 Values like 0e18172487393827593258 and 0e-18172487393827593258 should just be parsed as 0, and do not need to return an error. --- .../src/lib/features/parser/num_parser.rs | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/parser/num_parser.rs b/src/uucore/src/lib/features/parser/num_parser.rs index 083a6ca05..c072b8702 100644 --- a/src/uucore/src/lib/features/parser/num_parser.rs +++ b/src/uucore/src/lib/features/parser/num_parser.rs @@ -294,8 +294,14 @@ fn construct_extended_big_decimal<'a>( scale: u64, exponent: BigInt, ) -> Result> { - if digits == BigUint::zero() && negative { - return Ok(ExtendedBigDecimal::MinusZero); + if digits == BigUint::zero() { + // Return return 0 if the digits are zero. In particular, we do not ever + // return Overflow/Underflow errors in that case. + return Ok(if negative { + ExtendedBigDecimal::MinusZero + } else { + ExtendedBigDecimal::zero() + }); } let sign = if negative { Sign::Minus } else { Sign::Plus }; @@ -712,6 +718,24 @@ mod tests { ExtendedBigDecimal::MinusZero )) )); + + // But no Overflow/Underflow if the digits are 0. + assert_eq!( + ExtendedBigDecimal::extended_parse(&format!("0e{}", i64::MAX as u64 + 2)), + Ok(ExtendedBigDecimal::zero()), + ); + assert_eq!( + ExtendedBigDecimal::extended_parse(&format!("-0.0e{}", i64::MAX as u64 + 3)), + Ok(ExtendedBigDecimal::MinusZero) + ); + assert_eq!( + ExtendedBigDecimal::extended_parse(&format!("0.0000e{}", i64::MIN)), + Ok(ExtendedBigDecimal::zero()), + ); + assert_eq!( + ExtendedBigDecimal::extended_parse(&format!("-0e{}", i64::MIN + 2)), + Ok(ExtendedBigDecimal::MinusZero) + ); } #[test] From 3fc9c40c51116e04005e10d3d6a91531dd7101e2 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Fri, 4 Apr 2025 11:52:41 +0200 Subject: [PATCH 3/4] uucore: parser: parse_time: Use ExtendedBigDecimal parser Gives a little bit more flexibility in terms of allowed input for durations (e.g. in `timeout`), e.g. hex floating point numbers are now allowed. Fixes another part of #7475. --- .../src/lib/features/parser/parse_time.rs | 106 ++++++++++++++---- 1 file changed, 87 insertions(+), 19 deletions(-) diff --git a/src/uucore/src/lib/features/parser/parse_time.rs b/src/uucore/src/lib/features/parser/parse_time.rs index e0b4a6dc7..2fd7854d5 100644 --- a/src/uucore/src/lib/features/parser/parse_time.rs +++ b/src/uucore/src/lib/features/parser/parse_time.rs @@ -3,15 +3,22 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (vars) NANOS numstr infinityh INFD nans nanh +// spell-checker:ignore (vars) NANOS numstr infinityh INFD nans nanh bigdecimal extendedbigdecimal //! Parsing a duration from a string. //! //! Use the [`from_str`] function to parse a [`Duration`] from a string. +use crate::{ + display::Quotable, + extendedbigdecimal::ExtendedBigDecimal, + parser::num_parser::{ExtendedParser, ExtendedParserError}, +}; +use bigdecimal::BigDecimal; +use num_traits::Signed; +use num_traits::ToPrimitive; +use num_traits::Zero; use std::time::Duration; -use crate::display::Quotable; - /// Parse a duration from a string. /// /// The string may contain only a number, like "123" or "4.5", or it @@ -26,9 +33,10 @@ use crate::display::Quotable; /// * "h" for hours, /// * "d" for days. /// -/// This function uses [`Duration::saturating_mul`] to compute the -/// number of seconds, so it does not overflow. If overflow would have -/// occurred, [`Duration::MAX`] is returned instead. +/// This function does not overflow if large values are provided. If +/// overflow would have occurred, [`Duration::MAX`] is returned instead. +/// +/// If the value is smaller than 1 nanosecond, we return 1 nanosecond. /// /// # Errors /// @@ -45,6 +53,10 @@ use crate::display::Quotable; /// assert_eq!(from_str("2d"), Ok(Duration::from_secs(60 * 60 * 24 * 2))); /// ``` pub fn from_str(string: &str) -> Result { + // TODO: Switch to Duration::NANOSECOND if that ever becomes stable + // https://github.com/rust-lang/rust/issues/57391 + const NANOSECOND_DURATION: Duration = Duration::from_nanos(1); + let len = string.len(); if len == 0 { return Err("empty string".to_owned()); @@ -63,23 +75,38 @@ pub fn from_str(string: &str) -> Result { _ => return Err(format!("invalid time interval {}", string.quote())), }, }; - let num = numstr - .parse::() - .map_err(|e| format!("invalid time interval {}: {}", string.quote(), e))?; + let num = match ExtendedBigDecimal::extended_parse(numstr) { + Ok(ebd) | Err(ExtendedParserError::Overflow(ebd)) => ebd, + Err(ExtendedParserError::Underflow(_)) => return Ok(NANOSECOND_DURATION), + _ => return Err(format!("invalid time interval {}", string.quote())), + }; - if num < 0. || num.is_nan() { - return Err(format!("invalid time interval {}", string.quote())); - } + // Allow non-negative durations (-0 is fine), and infinity. + let num = match num { + ExtendedBigDecimal::BigDecimal(bd) if !bd.is_negative() => bd, + ExtendedBigDecimal::MinusZero => 0.into(), + ExtendedBigDecimal::Infinity => return Ok(Duration::MAX), + _ => return Err(format!("invalid time interval {}", string.quote())), + }; - if num.is_infinite() { - return Ok(Duration::MAX); + // Pre-multiply times to avoid precision loss + let num: BigDecimal = num * times; + + // Transform to nanoseconds (9 digits after decimal point) + let (nanos_bi, _) = num.with_scale(9).into_bigint_and_scale(); + + // If the value is smaller than a nanosecond, just return that. + if nanos_bi.is_zero() && !num.is_zero() { + return Ok(NANOSECOND_DURATION); } const NANOS_PER_SEC: u32 = 1_000_000_000; - let whole_secs = num.trunc(); - let nanos = (num.fract() * (NANOS_PER_SEC as f64)).trunc(); - let duration = Duration::new(whole_secs as u64, nanos as u32); - Ok(duration.saturating_mul(times)) + let whole_secs: u64 = match (&nanos_bi / NANOS_PER_SEC).try_into() { + Ok(whole_secs) => whole_secs, + Err(_) => return Ok(Duration::MAX), + }; + let nanos: u32 = (&nanos_bi % NANOS_PER_SEC).to_u32().unwrap(); + Ok(Duration::new(whole_secs, nanos)) } #[cfg(test)] @@ -99,8 +126,49 @@ mod tests { } #[test] - fn test_saturating_mul() { + fn test_overflow() { + // u64 seconds overflow (in Duration) assert_eq!(from_str("9223372036854775808d"), Ok(Duration::MAX)); + // ExtendedBigDecimal overflow + assert_eq!(from_str("1e92233720368547758080"), Ok(Duration::MAX)); + } + + #[test] + fn test_underflow() { + // TODO: Switch to Duration::NANOSECOND if that ever becomes stable + // https://github.com/rust-lang/rust/issues/57391 + const NANOSECOND_DURATION: Duration = Duration::from_nanos(1); + + // ExtendedBigDecimal underflow + assert_eq!(from_str("1e-92233720368547758080"), Ok(NANOSECOND_DURATION)); + // nanoseconds underflow (in Duration) + assert_eq!(from_str("0.0000000001"), Ok(NANOSECOND_DURATION)); + assert_eq!(from_str("1e-10"), Ok(NANOSECOND_DURATION)); + assert_eq!(from_str("9e-10"), Ok(NANOSECOND_DURATION)); + assert_eq!(from_str("1e-9"), Ok(NANOSECOND_DURATION)); + assert_eq!(from_str("1.9e-9"), Ok(NANOSECOND_DURATION)); + assert_eq!(from_str("2e-9"), Ok(Duration::from_nanos(2))); + } + + #[test] + fn test_zero() { + assert_eq!(from_str("0e-9"), Ok(Duration::ZERO)); + assert_eq!(from_str("0e-100"), Ok(Duration::ZERO)); + assert_eq!(from_str("0e-92233720368547758080"), Ok(Duration::ZERO)); + assert_eq!(from_str("0.000000000000000000000"), Ok(Duration::ZERO)); + } + + #[test] + fn test_hex_float() { + assert_eq!( + from_str("0x1.1p-1"), + Ok(Duration::from_secs_f64(0.53125f64)) + ); + assert_eq!( + from_str("0x1.1p-1d"), + Ok(Duration::from_secs_f64(0.53125f64 * 3600.0 * 24.0)) + ); + assert_eq!(from_str("0xfh"), Ok(Duration::from_secs(15 * 3600))); } #[test] From edc213d2c7f14a59916f56d38c10e17826ad7629 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Fri, 4 Apr 2025 12:15:16 +0200 Subject: [PATCH 4/4] test_timeout: Add tests for very short timeouts Note that unlike GNU coreutils, any value > 0 will not be treated as 0, even if the exponent is very large. --- tests/by-util/test_timeout.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 20d3e8fef..12da849db 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -125,6 +125,24 @@ fn test_dont_overflow() { .no_output(); } +#[test] +fn test_dont_underflow() { + new_ucmd!() + .args(&[".0000000001", "sleep", "1"]) + .fails_with_code(124) + .no_output(); + new_ucmd!() + .args(&["1e-100", "sleep", "1"]) + .fails_with_code(124) + .no_output(); + // Unlike GNU coreutils, we underflow to 1ns for very short timeouts. + // https://debbugs.gnu.org/cgi/bugreport.cgi?bug=77535 + new_ucmd!() + .args(&["1e-18172487393827593258", "sleep", "1"]) + .fails_with_code(124) + .no_output(); +} + #[test] fn test_negative_interval() { new_ucmd!()