Skip to content
New issue

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

#

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

Already on GitHub? # 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.

# for free to join this conversation on GitHub. Already have an account? # 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