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.

Sign up for GitHub

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

Jump to bottom

BigUint::from_str_radix panics with invalid string containing "+" #268

Closed
birkenfeld opened this issue Mar 5, 2017 · 1 comment · Fixed by #269
Closed

BigUint::from_str_radix panics with invalid string containing "+" #268

birkenfeld opened this issue Mar 5, 2017 · 1 comment · Fixed by #269
Labels
M-bigint T-bug

Comments

@birkenfeld
Copy link

birkenfeld commented Mar 5, 2017 •
edited
Loading

Test case:

use num_traits::Num;
num_bigint::BigUint::from_str_radix("0+2", 10);

will panic instead of returning an Err:

stack backtrace:
...
  10:     0x5629031d9e61 - <core::result::Result<T, E>>::unwrap_err::ha2ff9ebcf57c662c
                        at /checkout/src/libcore/result.rs:789
  11:     0x5629031e38db - <num_bigint::biguint::BigUint as num_traits::Num>::from_str_radix::h2b33fa225070b5b3
                        at .../num-bigint-0.1.36/src/biguint.rs:245

Cause is that num tries to create and unwrap_err a ParseIntError from std by calling parse on s[i..] where i is the index of the +. This will not error out if there are only valid digits after the +.

@Manishearth for the trophy case - found by using cargo-fuzz on serde-pickle
cuviper added a commit to cuviper/num that referenced this issue Mar 6, 2017
@cuviper
bigint: Create the parsing error better for nested +
0b6cae0 
If a `+` is encountered in the middle of parsing a BigUint, this should
generate an `ParseIntError::InvalidDigit`.  Since we can't create that
directly, we get it by trying to parse a `u64` from this point, but of
course `+` is a perfectly valid prefix to a `u64`.

Now we include the previous character in the string passed to `u64`, so
it has proper parsing context to understand what's in error.

Fixes rust-num#268.
@cuviper cuviper mentioned this issue Mar 6, 2017
Merged
@homu homu closed this as completed in #269 Mar 8, 2017
homu added a commit that referenced this issue Mar 8, 2017
@homu
Auto merge of #269 - cuviper:biguint-parse-plus, r=cuviper
fe21c72 
bigint: Create the parsing error better for nested `+`

If a `+` is encountered in the middle of parsing a BigUint, this should
generate an `ParseIntError::InvalidDigit`.  Since we can't create that
directly, we get it by trying to parse a `u64` from this point, but of
course `+` is a perfectly valid prefix to a `u64`.

Now we include the previous character in the string passed to `u64`, so it
has proper parsing context to understand what's in error.

Fixes #268.
@cuviper
Copy link
Member

cuviper commented Mar 8, 2017

Thanks, reports like this are welcome! This is now fixed and published in num-bigint v0.1.37.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
M-bigint T-bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bigint: Create the parsing error better for nested + cuviper/num
3 participants
@cuviper @birkenfeld @hauleth