Merge pull request #4439 from Joining7943/sleep-fix-panic-adding-durations

`sleep`: Fix parsing of multiple inputs panics on overflow, some whitespace issues ...

src/uu/sleep/src/sleep.rs

@@ -10,7 +10,7 @@ use std::time::Duration;
 
 use uucore::{
     error::{UResult, USimpleError, UUsageError},
-    format_usage, help_about, help_section, help_usage, show,
+    format_usage, help_about, help_section, help_usage, show_error,
 };
 
 use clap::{crate_version, Arg, ArgAction, Command};
@@ -61,15 +61,17 @@ pub fn uu_app() -> Command {
 
 fn sleep(args: &[&str]) -> UResult<()> {
     let mut arg_error = false;
-    let intervals = args.iter().map(|s| match uucore::parse_time::from_str(s) {
+    let sleep_dur = args
-        Ok(result) => result,
+        .iter()
-        Err(err) => {
+        .filter_map(|input| {
-            arg_error = true;
+            uucore::parse_time::from_str(input.trim()).ok().or_else(|| {
-            show!(USimpleError::new(1, err));
+                arg_error = true;
-            Duration::new(0, 0)
+                show_error!("invalid time interval '{input}'");
-        }
+                None
-    });
+            })
-    let sleep_dur = intervals.fold(Duration::new(0, 0), |acc, n| acc + n);
+        })
+        .fold(Duration::ZERO, |acc, n| acc.saturating_add(n));
+
     if arg_error {
         return Err(UUsageError::new(1, ""));
     };

tests/by-util/test_sleep.rs

@@ -1,3 +1,5 @@
+use rstest::rstest;
+
 // spell-checker:ignore dont
 use crate::common::util::*;
 
@@ -129,29 +131,96 @@ fn test_sleep_wrong_time() {
     new_ucmd!().args(&["0.1s", "abc"]).fails();
 }
 
-// TODO These tests would obviously block for a very long time. We
+#[test]
-// only want to verify that there is no error here, so we could just
+fn test_sleep_when_single_input_exceeds_max_duration_then_no_error() {
-// figure out a way to terminate the child process after a short
+    let mut child = new_ucmd!()
-// period of time.
+        .arg(format!("{}", u64::MAX as u128 + 1))
+        .timeout(Duration::from_secs(10))
+        .run_no_wait();
 
-// #[test]
+    #[cfg(unix)]
-#[allow(dead_code)]
+    child
-fn test_dont_overflow() {
+        .delay(100)
-    new_ucmd!()
+        .kill()
-        .arg("9223372036854775808d")
+        .make_assertion()
-        .succeeds()
+        .with_current_output()
-        .no_stderr()
+        .signal_is(9) // make sure it was us who terminated the process
-        .no_stdout();
+        .no_output();
+    #[cfg(windows)]
+    child
+        .delay(100)
+        .kill()
+        .make_assertion()
+        .with_current_output()
+        .failure()
+        .no_output();
 }
 
-// #[test]
+#[test]
-#[allow(dead_code)]
+fn test_sleep_when_multiple_inputs_exceed_max_duration_then_no_error() {
-fn test_sum_overflow() {
+    let mut child = new_ucmd!()
+        .arg(format!("{}", u64::MAX))
+        .arg("1")
+        .timeout(Duration::from_secs(10))
+        .run_no_wait();
+
+    #[cfg(unix)]
+    child
+        .delay(100)
+        .kill()
+        .make_assertion()
+        .with_current_output()
+        .signal_is(9) // make sure it was us who terminated the process
+        .no_output();
+    #[cfg(windows)]
+    child
+        .delay(100)
+        .kill()
+        .make_assertion()
+        .with_current_output()
+        .failure()
+        .no_output();
+}
+
+#[rstest]
+#[case::whitespace_prefix(" 0.1s")]
+#[case::multiple_whitespace_prefix("   0.1s")]
+#[case::whitespace_suffix("0.1s ")]
+#[case::mixed_newlines_spaces_tabs("\n\t0.1s \n ")]
+fn test_sleep_when_input_has_whitespace_then_no_error(#[case] input: &str) {
     new_ucmd!()
-        .args(&["100000000000000d", "100000000000000d", "100000000000000d"])
+        .arg(input)
+        .timeout(Duration::from_secs(10))
         .succeeds()
-        .no_stderr()
+        .no_output();
-        .no_stdout();
+}
+
+#[rstest]
+#[case::only_space(" ")]
+#[case::only_tab("\t")]
+#[case::only_newline("\n")]
+fn test_sleep_when_input_has_only_whitespace_then_error(#[case] input: &str) {
+    new_ucmd!()
+        .arg(input)
+        .timeout(Duration::from_secs(10))
+        .fails()
+        .usage_error(format!("invalid time interval '{input}'"));
+}
+
+#[test]
+fn test_sleep_when_multiple_input_some_with_error_then_shows_all_errors() {
+    let expected = "invalid time interval 'abc'\n\
+        sleep: invalid time interval '1years'\n\
+        sleep: invalid time interval ' '";
+
+    // Even if one of the arguments is valid, but the rest isn't, we should still fail and exit early.
+    // So, the timeout of 10 seconds ensures we haven't executed `thread::sleep` with the only valid
+    // interval of `100000.0` seconds.
+    new_ucmd!()
+        .args(&["abc", "100000.0", "1years", " "])
+        .timeout(Duration::from_secs(10))
+        .fails()
+        .usage_error(expected);
 }
 
 #[test]