diff --git a/Cargo.lock b/Cargo.lock index 25414fb03ac..6b3cecdf681 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3270,6 +3270,7 @@ name = "uu_timeout" version = "0.0.19" dependencies = [ "clap", + "fundu", "libc", "nix", "uucore", diff --git a/src/uu/timeout/Cargo.toml b/src/uu/timeout/Cargo.toml index 43cadf3e5d8..4958f0945f4 100644 --- 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 index 531f29e8243..300ce3ab425 100644 --- 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::(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::::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::(options::DURATION).unwrap(), - ) { - Ok(duration) => duration, - Err(err) => return Err(UUsageError::new(ExitStatus::TimeoutFailed.into(), err)), + let duration_str = options.get_one::(options::DURATION).unwrap(); + let duration = match DURATION_PARSER.parse(duration_str.trim_start()) { + Ok(duration) => SaturatingInto::::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 index 8e6e5db5572..bd8df910620 100644 --- 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.