From 42a34456d54a0f7bb7d9029429bd0f1e3609bdfb Mon Sep 17 00:00:00 2001 From: sholderbach Date: Sun, 19 Jan 2025 19:05:38 +0100 Subject: [PATCH 1/3] Fix parsing logic after first whitespace The way the parse was implemented accepted additional numeric characters or `.` after the first valid `f64` literal but ignored them. This permitted more strings that are invalid following a strict grammar for byte sizes and silently ignores the further symbols without error. ``` 1.0 ...KB 1.0 42.0 B ``` This change makes those illegal. `1 000 B` was also subject to the bad `skip_while` ignoring the following `000`. In this version of the fix whitespace is not accepted as a digit separator. So it will raise an error if the user doesn't explicitly strip the whitespace instead of reporting a wrong value. Signed-off-by: sholderbach --- src/parse.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/parse.rs b/src/parse.rs index a5cca5b..f6309e6 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -10,9 +10,7 @@ impl std::str::FromStr for ByteSize { let number = take_while(value, |c| c.is_ascii_digit() || c == '.'); match number.parse::() { Ok(v) => { - let suffix = skip_while(value, |c| { - c.is_whitespace() || c.is_ascii_digit() || c == '.' - }); + let suffix = skip_while(&value[number.len()..], char::is_whitespace); match suffix.parse::() { Ok(u) => Ok(Self((v * u) as u64)), Err(error) => Err(format!( From 11b153bd9cfedc663c2ca4d6c146c89bd5da3643 Mon Sep 17 00:00:00 2001 From: sholderbach Date: Sun, 19 Jan 2025 19:06:31 +0100 Subject: [PATCH 2/3] Add tests for invalid chars after whitespace Signed-off-by: sholderbach --- src/parse.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/parse.rs b/src/parse.rs index f6309e6..3468829 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -218,6 +218,8 @@ mod tests { assert!(parse("").is_err()); assert!(parse("a124GB").is_err()); + assert!(parse("1.3 42.0 B").is_err()); + assert!(parse("1.3 ... B").is_err()); } #[test] From 9b525b4aba20578df3ac77725bde5a8ad4fe7b86 Mon Sep 17 00:00:00 2001 From: sholderbach Date: Sun, 19 Jan 2025 19:13:16 +0100 Subject: [PATCH 3/3] Add test documenting that whitespace is not a valid digit separator More following the old choices of using `f64::from_str` Signed-off-by: sholderbach --- src/parse.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/parse.rs b/src/parse.rs index 3468829..218fe8b 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -220,6 +220,9 @@ mod tests { assert!(parse("a124GB").is_err()); assert!(parse("1.3 42.0 B").is_err()); assert!(parse("1.3 ... B").is_err()); + // The original implementation did not account for the possibility that users may + // use whitespace to visually separate digits, thus treat it as an error + assert!(parse("1 000 B").is_err()); } #[test]