Skip to content

Decimal::parse does not check the result against MAX_DIGITS, so a 39 digit decimal parses #774

Description

@tamnd

crates/zu-common/src/decimal.rs says MAX_DIGITS is 38, which is the widest decimal this engine holds and the widest DECIMAL(p, s) that may be declared. Decimal::parse does not compare what it built against it.

The limit that actually applies inside parse is the i128, which holds up to about 1.70e38. A 39 digit number below that fits:

let d = Decimal::parse(&"1".repeat(39), 0).unwrap();
assert_eq!(d.digits(), 39);   // wider than MAX_DIGITS, and parsed anyway

So the value is one the engine will not store, handed back by the function whose job is to say whether the text is a decimal this engine holds. Every client that parses text has to add the check itself, and one that forgets gets a value that fails later and somewhere else.

Two things would fix it, and they are worth having together:

  1. parse refuses a result whose digits() exceeds MAX_DIGITS, the same way it already refuses an overflow.
  2. A constructor that reads the scale out of the text rather than taking it as an argument. parse takes a scale, so a caller who has only text has to count the digits after the point, apply any exponent to that count, and hand the answer back to the function that is about to read the same text. zu-go, zu-node and zu-python each reimplemented that arithmetic, three times, three ways.

Found while adding decimal support to the three clients.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions