timeout: Make use of fundu in timeout to parse the KILL_AFTER and DUR…

…ATION arguments
uutils · Jun 23, 2023 · 0209842 · 0209842
1 parent b787a2a
commit 0209842
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 14 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/src/uu/timeout/Cargo.toml b/src/uu/timeout/Cargo.toml
@@ -19,6 +19,7 @@ clap = { workspace = true }
 libc = { workspace = true }
 nix = { workspace = true, features = ["signal"] }
 uucore = { workspace = true, features = ["process", "signals"] }
+fundu = { workspace = true }
 
 [[bin]]
 name = "timeout"

diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs
@@ -10,6 +10,7 @@ mod status;
 
 use crate::status::ExitStatus;
 use clap::{crate_version, Arg, ArgAction, Command};
+use fundu::{DurationParser, SaturatingInto, TimeUnit};
 use std::io::ErrorKind;
 use std::os::unix::process::ExitStatusExt;
 use std::process::{self, Child, Stdio};
@@ -26,6 +27,15 @@ use uucore::{
     signals::{signal_by_name_or_value, signal_name_by_value},
 };
 
+const DURATION_PARSER: DurationParser = DurationParser::builder()
+    .time_units(&[
+        TimeUnit::Second,
+        TimeUnit::Minute,
+        TimeUnit::Hour,
+        TimeUnit::Day,
+    ])
+    .build();
+
 const ABOUT: &str = help_about!("timeout.md");
 const USAGE: &str = help_usage!("timeout.md");
 
@@ -72,17 +82,40 @@ impl Config {
 
         let kill_after = match options.get_one::<String>(options::KILL_AFTER) {
             None => None,
-            Some(kill_after) => match uucore::parse_time::from_str(kill_after) {
-                Ok(k) => Some(k),
-                Err(err) => return Err(UUsageError::new(ExitStatus::TimeoutFailed.into(), err)),
+            Some(kill_after) => match DURATION_PARSER.parse(kill_after.trim_start()) {
+                Ok(k) => Some(SaturatingInto::<Duration>::saturating_into(k)),
+                Err(err) => {
+                    let reason = match err {
+                        fundu::ParseError::Syntax(pos, description)
+                        | fundu::ParseError::TimeUnit(pos, description) => {
+                            format!("{description} at position {}", pos.saturating_add(1))
+                        }
+                        error => error.to_string(),
+                    };
+                    return Err(UUsageError::new(
+                        ExitStatus::TimeoutFailed.into(),
+                        format!("invalid time interval '{kill_after}': {reason}"),
+                    ));
+                }
             },
         };
 
-        let duration = match uucore::parse_time::from_str(
-            options.get_one::<String>(options::DURATION).unwrap(),
-        ) {
-            Ok(duration) => duration,
-            Err(err) => return Err(UUsageError::new(ExitStatus::TimeoutFailed.into(), err)),
+        let duration_str = options.get_one::<String>(options::DURATION).unwrap();
+        let duration = match DURATION_PARSER.parse(duration_str.trim_start()) {
+            Ok(duration) => SaturatingInto::<Duration>::saturating_into(duration),
+            Err(err) => {
+                let reason = match err {
+                    fundu::ParseError::Syntax(pos, description)
+                    | fundu::ParseError::TimeUnit(pos, description) => {
+                        format!("{description} at position {}", pos.saturating_add(1))
+                    }
+                    error => error.to_string(),
+                };
+                return Err(UUsageError::new(
+                    ExitStatus::TimeoutFailed.into(),
+                    format!("invalid time interval '{duration_str}': {reason}"),
+                ));
+            }
         };
 
         let preserve_status: bool = options.get_flag(options::PRESERVE_STATUS);

diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs
@@ -22,7 +22,7 @@ fn test_invalid_time_interval() {
         .args(&["xyz", "sleep", "0"])
         .fails()
         .code_is(125)
-        .usage_error("invalid time interval 'xyz'");
+        .usage_error("invalid time interval 'xyz': Invalid input: 'xyz' at position 1");
 }
 
 #[test]
@@ -31,7 +31,7 @@ fn test_invalid_kill_after() {
         .args(&["-k", "xyz", "1", "sleep", "0"])
         .fails()
         .code_is(125)
-        .usage_error("invalid time interval 'xyz'");
+        .usage_error("invalid time interval 'xyz': Invalid input: 'xyz' at position 1");
 }
 
 #[test]
@@ -75,7 +75,7 @@ fn test_command_empty_args() {
     new_ucmd!()
         .args(&["", ""])
         .fails()
-        .stderr_contains("timeout: empty string");
+        .usage_error("invalid time interval '': Empty input");
 }
 
 #[test]
@@ -110,7 +110,7 @@ fn test_negative_interval() {
     new_ucmd!()
         .args(&["--", "-1", "sleep", "0"])
         .fails()
-        .usage_error("invalid time interval '-1'");
+        .usage_error("invalid time interval '-1': Number was negative");
 }
 
 #[test]
@@ -126,7 +126,7 @@ fn test_invalid_multi_byte_characters() {
     new_ucmd!()
         .args(&["10€", "sleep", "0"])
         .fails()
-        .usage_error("invalid time interval '10€'");
+        .usage_error("invalid time interval '10€': Invalid time unit: '€' at position 3");
 }
 
 /// Test that the long form of the `--kill-after` argument is recognized.
@@ -152,5 +152,5 @@ fn test_kill_subprocess() {
         .fails()
         .code_is(124)
         .stdout_contains("xyz")
-        .stderr_contains("Terminated");
+        .stderr_is("Terminated\n");
 }