Skip to content

Commit

Permalink
Fix overly permissive parsing after whitespace (#59)
Browse files Browse the repository at this point in the history
* 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.

* Add tests for invalid chars after whitespace

* Add test documenting that whitespace is not a valid digit separator

More following the old choices of using `f64::from_str`

---------

Co-authored-by: Rob Ede <robjtede@icloud.com>
  • Loading branch information
sholderbach and robjtede authored Feb 9, 2025
1 parent 6b0eb20 commit 0e10422
Showing 1 changed file with 6 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ impl std::str::FromStr for ByteSize {
let number = take_while(value, |c| c.is_ascii_digit() || c == '.');
match number.parse::<f64>() {
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::<Unit>() {
Ok(u) => Ok(Self((v * u) as u64)),
Err(error) => Err(format!(
Expand Down Expand Up @@ -220,6 +218,11 @@ 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());
// 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]
Expand Down

0 comments on commit 0e10422

Please sign in to comment.