Merge pull request #7648 from drinkcat/parse_time-ebd

timeout: Use common parser to parse time duration

Author: sylvestre

src/uucore/src/lib/features/parser/num_parser.rs

@@ -294,8 +294,14 @@ fn construct_extended_big_decimal<'a>(
     scale: u64,
     exponent: BigInt,
 ) -> Result<ExtendedBigDecimal, ExtendedParserError<'a, ExtendedBigDecimal>> {
-    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]

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
+// 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<Duration, String> {
+    // 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());
@@ -58,27 +70,43 @@ pub fn from_str(string: &str) -> Result<Duration, String> {
         '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 = 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())),
     };
-    let num = numstr
-        .parse::<f64>()
-        .map_err(|e| format!("invalid time interval {}: {}", string.quote(), e))?;
 
-    if num < 0. {
-        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())),
+    };
+
+    // 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)]
@@ -98,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]
@@ -127,12 +196,31 @@ 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() {
         assert!(from_str("1S").is_err());
         assert!(from_str("1M").is_err());
         assert!(from_str("1H").is_err());
         assert!(from_str("1D").is_err());
+        assert!(from_str("INFD").is_err());
     }
 }

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!()