search

akubera / bigdecimal-rs

Arbitrary precision decimal crate for Rust
Other
284 stars 71 forks source link

Subtraction overflow leads to panic in `from_str_radix` #115

Closed fritzrehde closed 1 year ago

fritzrehde commented 1 year ago

The issue https://github.com/uutils/coreutils/issues/3318 provides an example for which parsing into a BigDecimal fails:

coreutils seq 1e-9223372036854775808
thread 'main' panicked at 'attempt to subtract with overflow', /home/fritz/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bigdecimal-0.4.0/src/lib.rs:2059:21

If we take a look at the location where the panic occurs, we see that the reason is that we don't check whether the subtraction

        let scale = decimal_offset - exponent_value;

overflows, because we are subtracting a very small (large absolute value with negative sign) exponent. I would suggest using checked_sub instead of the regular subtraction operator -. That returns None if an overflow occurred. There is probably some slight performance penalty, but I think correctness (i.e. not panicking and instead throwing an error) should be prioritized. What behaviour/error would we want to implement/throw in case such an overflow occurs? I can create a PR with this fix if you think it's a good idea.

akubera commented 1 year ago

Thanks for the report. This has been fixed in the recently released v0.4.2 (relevant test).

The Error contains the source string, which may or may not be what the user wants. It might be redundant, but I'm defaulting to more-information vs more user effort. I could use user input in this regard. The ParseBigDecimalError hasn't been changed since 2017l, and could probably use a thorough review.