From 27dd59635af4d32b5898077c25b03eaf78a93ddb Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Mon, 16 May 2022 16:39:20 +0200 Subject: [PATCH] uucore: add InvalidSuffix to ParseSizeError --- src/uu/dd/src/parseargs.rs | 2 +- src/uu/df/src/df.rs | 5 +++ src/uu/du/src/du.rs | 4 ++- src/uu/od/src/od.rs | 4 ++- src/uu/sort/src/sort.rs | 8 +++-- src/uucore/src/lib/parser/parse_size.rs | 41 +++++++++++++++---------- tests/by-util/test_df.rs | 13 ++++++++ tests/by-util/test_du.rs | 8 ++++- tests/by-util/test_od.rs | 13 +++++++- tests/by-util/test_sort.rs | 26 ++++++++-------- 10 files changed, 89 insertions(+), 35 deletions(-) diff --git a/src/uu/dd/src/parseargs.rs b/src/uu/dd/src/parseargs.rs index 0a2fae99a22..53adce3f326 100644 --- a/src/uu/dd/src/parseargs.rs +++ b/src/uu/dd/src/parseargs.rs @@ -388,7 +388,7 @@ fn parse_bytes_no_x(s: &str) -> Result { let (num, multiplier) = match (s.find('c'), s.rfind('w'), s.rfind('b')) { (None, None, None) => match uucore::parse_size::parse_size(s) { Ok(n) => (n, 1), - Err(ParseSizeError::ParseFailure(s)) => { + Err(ParseSizeError::InvalidSuffix(s)) | Err(ParseSizeError::ParseFailure(s)) => { return Err(ParseError::MultiplierStringParseFailure(s)) } Err(ParseSizeError::SizeTooBig(s)) => { diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 1f59982b21a..793138f7dd2 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -121,6 +121,7 @@ impl Default for Options { enum OptionsError { BlockSizeTooLarge(String), InvalidBlockSize(String), + InvalidSuffix(String), /// An error getting the columns to display in the output table. ColumnError(ColumnError), @@ -139,6 +140,9 @@ impl fmt::Display for OptionsError { // TODO This needs to vary based on whether `--block-size` // or `-B` were provided. Self::InvalidBlockSize(s) => write!(f, "invalid --block-size argument {}", s), + // TODO This needs to vary based on whether `--block-size` + // or `-B` were provided. + Self::InvalidSuffix(s) => write!(f, "invalid suffix in --block-size argument {}", s), Self::ColumnError(ColumnError::MultipleColumns(s)) => write!( f, "option --output: field {} used more than once", @@ -174,6 +178,7 @@ impl Options { show_local_fs: matches.is_present(OPT_LOCAL), show_all_fs: matches.is_present(OPT_ALL), block_size: block_size_from_matches(matches).map_err(|e| match e { + ParseSizeError::InvalidSuffix(s) => OptionsError::InvalidSuffix(s), ParseSizeError::SizeTooBig(_) => OptionsError::BlockSizeTooLarge( matches.value_of(OPT_BLOCKSIZE).unwrap().to_string(), ), diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index d358d3e8f99..ec1c735b033 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -953,8 +953,10 @@ impl Threshold { fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String { // NOTE: // GNU's du echos affected flag, -B or --block-size (-t or --threshold), depending user's selection - // GNU's du does distinguish between "invalid (suffix in) argument" match error { + ParseSizeError::InvalidSuffix(_) => { + format!("invalid suffix in --{} argument {}", option, s.quote()) + } ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()), ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()), } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 673035ea27d..73917b99f76 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -679,8 +679,10 @@ fn open_input_peek_reader( fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String { // NOTE: // GNU's od echos affected flag, -N or --read-bytes (-j or --skip-bytes, etc.), depending user's selection - // GNU's od does distinguish between "invalid (suffix in) argument" match error { + ParseSizeError::InvalidSuffix(_) => { + format!("invalid suffix in --{} argument {}", option, s.quote()) + } ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()), ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()), } diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index c3d66d05715..15e36a59535 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -362,8 +362,10 @@ impl GlobalSettings { size )) }) + } else if size_string.starts_with(|c: char| c.is_ascii_digit()) { + Err(ParseSizeError::InvalidSuffix("invalid suffix".to_string())) } else { - Err(ParseSizeError::ParseFailure("invalid suffix".to_string())) + Err(ParseSizeError::ParseFailure("parse failure".to_string())) } } @@ -1833,8 +1835,10 @@ fn open(path: impl AsRef) -> UResult> { fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String { // NOTE: // GNU's sort echos affected flag, -S or --buffer-size, depending user's selection - // GNU's sort does distinguish between "invalid (suffix in) argument" match error { + ParseSizeError::InvalidSuffix(_) => { + format!("invalid suffix in --{} argument {}", option, s.quote()) + } ParseSizeError::ParseFailure(_) => format!("invalid --{} argument {}", option, s.quote()), ParseSizeError::SizeTooBig(_) => format!("--{} argument {} too large", option, s.quote()), } diff --git a/src/uucore/src/lib/parser/parse_size.rs b/src/uucore/src/lib/parser/parse_size.rs index de316c3d2dc..557010584c6 100644 --- a/src/uucore/src/lib/parser/parse_size.rs +++ b/src/uucore/src/lib/parser/parse_size.rs @@ -72,7 +72,8 @@ pub fn parse_size(size: &str) -> Result { "EB" | "eB" => (1000, 6), "ZB" | "zB" => (1000, 7), "YB" | "yB" => (1000, 8), - _ => return Err(ParseSizeError::parse_failure(size)), + _ if numeric_string.is_empty() => return Err(ParseSizeError::parse_failure(size)), + _ => return Err(ParseSizeError::invalid_suffix(size)), }; let factor = match u64::try_from(base.pow(exponent)) { Ok(n) => n, @@ -85,13 +86,15 @@ pub fn parse_size(size: &str) -> Result { #[derive(Debug, PartialEq, Eq)] pub enum ParseSizeError { - ParseFailure(String), // Syntax - SizeTooBig(String), // Overflow + InvalidSuffix(String), // Suffix + ParseFailure(String), // Syntax + SizeTooBig(String), // Overflow } impl Error for ParseSizeError { fn description(&self) -> &str { match *self { + ParseSizeError::InvalidSuffix(ref s) => &*s, ParseSizeError::ParseFailure(ref s) => &*s, ParseSizeError::SizeTooBig(ref s) => &*s, } @@ -101,7 +104,9 @@ impl Error for ParseSizeError { impl fmt::Display for ParseSizeError { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { let s = match self { - ParseSizeError::ParseFailure(s) | ParseSizeError::SizeTooBig(s) => s, + ParseSizeError::InvalidSuffix(s) + | ParseSizeError::ParseFailure(s) + | ParseSizeError::SizeTooBig(s) => s, }; write!(f, "{}", s) } @@ -111,6 +116,10 @@ impl fmt::Display for ParseSizeError { // but there's a lot of downstream code that constructs these errors manually // that would be affected impl ParseSizeError { + fn invalid_suffix(s: &str) -> Self { + Self::InvalidSuffix(format!("{}", s.quote())) + } + fn parse_failure(s: &str) -> Self { // stderr on linux (GNU coreutils 8.32) (LC_ALL=C) // has to be handled in the respective uutils because strings differ, e.g.: @@ -237,20 +246,20 @@ mod tests { ); } + #[test] + fn invalid_suffix() { + let test_strings = ["328hdsf3290", "5mib", "1e2", "1H", "1.2"]; + for &test_string in &test_strings { + assert_eq!( + parse_size(test_string).unwrap_err(), + ParseSizeError::InvalidSuffix(format!("{}", test_string.quote())) + ); + } + } + #[test] fn invalid_syntax() { - let test_strings = [ - "328hdsf3290", - "5MiB nonsense", - "5mib", - "biB", - "-", - "+", - "", - "-1", - "1e2", - "∞", - ]; + let test_strings = ["biB", "-", "+", "", "-1", "∞"]; for &test_string in &test_strings { assert_eq!( parse_size(test_string).unwrap_err(), diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 9ff8e103ddc..6f8de3bfad5 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -566,6 +566,19 @@ fn test_invalid_block_size() { .stderr_contains("invalid --block-size argument '0K'"); } +#[test] +fn test_invalid_block_size_suffix() { + new_ucmd!() + .arg("--block-size=1H") + .fails() + .stderr_contains("invalid suffix in --block-size argument '1H'"); + + new_ucmd!() + .arg("--block-size=1.2") + .fails() + .stderr_contains("invalid suffix in --block-size argument '1.2'"); +} + #[test] fn test_output_selects_columns() { let output = new_ucmd!() diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index bf506c8b538..b421d5e84d2 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -94,7 +94,13 @@ fn test_du_invalid_size() { .arg("/tmp") .fails() .code_is(1) - .stderr_only(format!("du: invalid --{} argument '1fb4t'", s)); + .stderr_only(format!("du: invalid suffix in --{} argument '1fb4t'", s)); + ts.ucmd() + .arg(format!("--{}=x", s)) + .arg("/tmp") + .fails() + .code_is(1) + .stderr_only(format!("du: invalid --{} argument 'x'", s)); #[cfg(not(target_pointer_width = "128"))] ts.ucmd() .arg(format!("--{}=1Y", s)) diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index 3272971d748..e688813099c 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -827,7 +827,8 @@ fn test_traditional_only_label() { #[test] fn test_od_invalid_bytes() { - const INVALID_SIZE: &str = "1fb4t"; + const INVALID_SIZE: &str = "x"; + const INVALID_SUFFIX: &str = "1fb4t"; const BIG_SIZE: &str = "1Y"; // NOTE: @@ -852,6 +853,16 @@ fn test_od_invalid_bytes() { option, INVALID_SIZE )); + new_ucmd!() + .arg(format!("{}={}", option, INVALID_SUFFIX)) + .arg("file") + .fails() + .code_is(1) + .stderr_only(format!( + "od: invalid suffix in {} argument '{}'", + option, INVALID_SUFFIX + )); + #[cfg(not(target_pointer_width = "128"))] new_ucmd!() .arg(format!("{}={}", option, BIG_SIZE)) diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 24846d20706..ff4c7942777 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -56,18 +56,20 @@ fn test_buffer_sizes() { #[test] fn test_invalid_buffer_size() { - let buffer_sizes = ["asd", "100f"]; - for invalid_buffer_size in &buffer_sizes { - new_ucmd!() - .arg("-S") - .arg(invalid_buffer_size) - .fails() - .code_is(2) - .stderr_only(format!( - "sort: invalid --buffer-size argument '{}'", - invalid_buffer_size - )); - } + new_ucmd!() + .arg("-S") + .arg("asd") + .fails() + .code_is(2) + .stderr_only("sort: invalid --buffer-size argument 'asd'"); + + new_ucmd!() + .arg("-S") + .arg("100f") + .fails() + .code_is(2) + .stderr_only("sort: invalid suffix in --buffer-size argument '100f'"); + #[cfg(not(target_pointer_width = "128"))] new_ucmd!() .arg("-n")