Merge pull request #3272 from jfinkels/uucore-parse-time-overflow

uucore, sleep: use Duration::saturating_mul and saturating_add to avoid overflow

src/uu/sleep/src/sleep.rs

@@ -63,7 +63,7 @@ fn sleep(args: &[&str]) -> UResult<()> {
         args.iter().try_fold(
             Duration::new(0, 0),
             |result, arg| match uucore::parse_time::from_str(&arg[..]) {
-                Ok(m) => Ok(m + result),
+                Ok(m) => Ok(m.saturating_add(result)),
                 Err(f) => Err(USimpleError::new(1, f)),
             },
         )?;

src/uucore/src/lib/parser/parse_time.rs

@@ -6,11 +6,39 @@
 // file that was distributed with this source code.
 
 // spell-checker:ignore (vars) NANOS numstr
+//! Parsing a duration from a string.
+//!
+//! Use the [`from_str`] function to parse a [`Duration`] from a string.
 
 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
+/// may contain a number with a unit specifier, like "123s" meaning
+/// one hundred twenty three seconds or "4.5d" meaning four and a half
+/// days. If no unit is specified, the unit is assumed to be seconds.
+///
+/// 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.
+///
+/// # Errors
+///
+/// This function returns an error if the input string is empty, the
+/// input is not a valid number, or the unit specifier is invalid or
+/// unknown.
+///
+/// # Examples
+///
+/// ```rust
+/// use std::time::Duration;
+/// use uucore::parse_time::from_str;
+/// assert_eq!(from_str("123"), Ok(Duration::from_secs(123)));
+/// assert_eq!(from_str("2d"), Ok(Duration::from_secs(60 * 60 * 24 * 2)));
+/// ```
 pub fn from_str(string: &str) -> Result<Duration, String> {
     let len = string.len();
     if len == 0 {
@@ -39,5 +67,42 @@ pub fn from_str(string: &str) -> Result<Duration, String> {
     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 * times)
+    Ok(duration.saturating_mul(times))
+}
+
+#[cfg(test)]
+mod tests {
+
+    use crate::parse_time::from_str;
+    use std::time::Duration;
+
+    #[test]
+    fn test_no_units() {
+        assert_eq!(from_str("123"), Ok(Duration::from_secs(123)));
+    }
+
+    #[test]
+    fn test_units() {
+        assert_eq!(from_str("2d"), Ok(Duration::from_secs(60 * 60 * 24 * 2)));
+    }
+
+    #[test]
+    fn test_saturating_mul() {
+        assert_eq!(from_str("9223372036854775808d"), Ok(Duration::MAX));
+    }
+
+    #[test]
+    fn test_error_empty() {
+        assert!(from_str("").is_err());
+    }
+
+    #[test]
+    fn test_error_invalid_unit() {
+        assert!(from_str("123X").is_err());
+    }
+
+    #[test]
+    fn test_error_invalid_magnitude() {
+        assert!(from_str("12abc3s").is_err());
+    }
 }

tests/by-util/test_sleep.rs

@@ -1,3 +1,4 @@
+// spell-checker:ignore dont
 use crate::common::util::*;
 
 use std::time::{Duration, Instant};
@@ -115,3 +116,28 @@ fn test_sleep_sum_duration_many() {
 fn test_sleep_wrong_time() {
     new_ucmd!().args(&["0.1s", "abc"]).fails();
 }
+
+// TODO These tests would obviously block for a very long time. We
+// only want to verify that there is no error here, so we could just
+// figure out a way to terminate the child process after a short
+// period of time.
+
+// #[test]
+#[allow(dead_code)]
+fn test_dont_overflow() {
+    new_ucmd!()
+        .arg("9223372036854775808d")
+        .succeeds()
+        .no_stderr()
+        .no_stdout();
+}
+
+// #[test]
+#[allow(dead_code)]
+fn test_sum_overflow() {
+    new_ucmd!()
+        .args(&["100000000000000d", "100000000000000d", "100000000000000d"])
+        .succeeds()
+        .no_stderr()
+        .no_stdout();
+}

tests/by-util/test_timeout.rs

@@ -1,3 +1,4 @@
+// spell-checker:ignore dont
 use crate::common::util::*;
 
 // FIXME: this depends on the system having true and false in PATH
@@ -64,3 +65,19 @@ fn test_preserve_status() {
         .no_stderr()
         .no_stdout();
 }
+
+#[test]
+fn test_dont_overflow() {
+    new_ucmd!()
+        .args(&["9223372036854775808d", "sleep", "0"])
+        .succeeds()
+        .code_is(0)
+        .no_stderr()
+        .no_stdout();
+    new_ucmd!()
+        .args(&["-k", "9223372036854775808d", "10", "sleep", "0"])
+        .succeeds()
+        .code_is(0)
+        .no_stderr()
+        .no_stdout();
+}