Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix overly permissive parsing after whitespace #59

Merged

Conversation

sholderbach
Copy link
Contributor

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 than 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.

Verified

This commit was signed with the committer’s verified signature.
blink1073 Steven Silvester
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.
More following the old choices of using `f64::from_str`
Copy link
Member

@robjtede robjtede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these test cases look sensible to fix, thanks 👍🏻

@robjtede robjtede merged commit 0e10422 into bytesize-rs:master Feb 9, 2025
11 checks passed
@robjtede robjtede mentioned this pull request Feb 9, 2025
robjtede added a commit that referenced this pull request Feb 9, 2025
robjtede added a commit that referenced this pull request Feb 9, 2025
robjtede added a commit that referenced this pull request Feb 9, 2025
robjtede added a commit that referenced this pull request Feb 9, 2025
robjtede added a commit to Andrew15-5/bytesize that referenced this pull request Feb 10, 2025
* 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>
@sholderbach
Copy link
Contributor Author

Thanks for your maintenance work!

sholderbach added a commit to sholderbach/nushell that referenced this pull request Feb 11, 2025
Closes nushell#14866

Incorporates bytesize-rs/bytesize#59 with
bytesize version 1.3.1

Added test with invalid input that was silently ignored before
sholderbach added a commit to sholderbach/nushell that referenced this pull request Feb 11, 2025
Closes nushell#14866

Incorporates bytesize-rs/bytesize#59 with
bytesize version 1.3.1

Added test with invalid input that was silently ignored before
sholderbach added a commit to nushell/nushell that referenced this pull request Feb 11, 2025
# Description
Closes #14866

Incorporates bytesize-rs/bytesize#59 with
bytesize version 1.3.1

# User-Facing Changes
Now rejected strings
```
"1.3 1.3 kB" | into filesize
"1 420 kB" | into filesize
```
# Tests + Formatting
Added test with invalid input that was silently ignored before
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants