Fixes Display Overflow issue when using precision greater than 30 (#433)

* Prevent Display implementation overflow
* Branch display logic to allow precision greater than 28 to be used
* Bind deserialize to a maximum precision

Co-authored-by: Caio <c410.f3r@gmail.com>

src/constants.rs

@@ -21,9 +21,12 @@ pub const SIGN_SHIFT: u32 = 31;
 pub const MAX_STR_BUFFER_SIZE: usize = 32;
 
 // The maximum supported precision
-pub const MAX_PRECISION: u32 = 28;
+pub const MAX_PRECISION: u8 = 28;
 #[cfg(not(feature = "legacy-ops"))]
-pub const MAX_PRECISION_I32: i32 = 28;
+// u8 to i32 is infallible, therefore, this cast will never overflow
+pub const MAX_PRECISION_I32: i32 = MAX_PRECISION as _;
+// u8 to u32 is infallible, therefore, this cast will never overflow
+pub const MAX_PRECISION_U32: u32 = MAX_PRECISION as _;
 // 79,228,162,514,264,337,593,543,950,335
 pub const MAX_I128_REPR: i128 = 0x0000_0000_FFFF_FFFF_FFFF_FFFF_FFFF_FFFF;
 

src/db.rs

@@ -1,4 +1,4 @@
-use crate::constants::MAX_PRECISION;
+use crate::constants::MAX_PRECISION_U32;
 use crate::{
     ops::array::{div_by_u32, is_all_zero, mul_by_u32},
     Decimal,
@@ -64,12 +64,13 @@ impl Decimal {
             let start_fractionals = if weight < 0 { (-weight as u32) - 1 } else { 0 };
             for (i, digit) in digits.into_iter().enumerate() {
                 let fract_pow = 4 * (i as u32 + 1 + start_fractionals);
-                if fract_pow <= MAX_PRECISION {
+                if fract_pow <= MAX_PRECISION_U32 {
                     result += Decimal::new(digit as i64, 0) / Decimal::from_i128_with_scale(10i128.pow(fract_pow), 0);
-                } else if fract_pow == MAX_PRECISION + 4 {
+                } else if fract_pow == MAX_PRECISION_U32 + 4 {
                     // rounding last digit
                     if digit >= 5000 {
-                        result += Decimal::new(1_i64, 0) / Decimal::from_i128_with_scale(10i128.pow(MAX_PRECISION), 0);
+                        result +=
+                            Decimal::new(1_i64, 0) / Decimal::from_i128_with_scale(10i128.pow(MAX_PRECISION_U32), 0);
                     }
                 }
             }

src/decimal.rs

@@ -1,5 +1,5 @@
 use crate::constants::{
-    MAX_I128_REPR, MAX_PRECISION, POWERS_10, SCALE_MASK, SCALE_SHIFT, SIGN_MASK, SIGN_SHIFT, U32_MASK, U8_MASK,
+    MAX_I128_REPR, MAX_PRECISION_U32, POWERS_10, SCALE_MASK, SCALE_SHIFT, SIGN_MASK, SIGN_SHIFT, U32_MASK, U8_MASK,
     UNSIGN_MASK,
 };
 use crate::ops;
@@ -263,7 +263,7 @@ impl Decimal {
     /// assert!(max.is_err());
     /// ```
     pub const fn try_new(num: i64, scale: u32) -> crate::Result<Decimal> {
-        if scale > MAX_PRECISION {
+        if scale > MAX_PRECISION_U32 {
             return Err(Error::ScaleExceedsMaximumPrecision(scale));
         }
         let flags: u32 = scale << SCALE_SHIFT;
@@ -323,7 +323,7 @@ impl Decimal {
     /// assert!(max.is_err());
     /// ```
     pub const fn try_from_i128_with_scale(num: i128, scale: u32) -> crate::Result<Decimal> {
-        if scale > MAX_PRECISION {
+        if scale > MAX_PRECISION_U32 {
             return Err(Error::ScaleExceedsMaximumPrecision(scale));
         }
         let mut neg = false;
@@ -382,7 +382,7 @@ impl Decimal {
                 } else {
                     negative
                 },
-                scale % (MAX_PRECISION + 1),
+                scale % (MAX_PRECISION_U32 + 1),
             ),
         }
     }
@@ -442,7 +442,7 @@ impl Decimal {
                 // we've parsed 1.2 as the base and 10 as the exponent. To represent this within a
                 // Decimal type we effectively store the mantissa as 12,000,000,000 and scale as
                 // zero.
-                if exp > MAX_PRECISION {
+                if exp > MAX_PRECISION_U32 {
                     return Err(Error::ScaleExceedsMaximumPrecision(exp));
                 }
                 let mut exp = exp as usize;
@@ -606,7 +606,7 @@ impl Decimal {
     /// assert_eq!(one.to_string(), "0.00001");
     /// ```
     pub fn set_scale(&mut self, scale: u32) -> Result<(), Error> {
-        if scale > MAX_PRECISION {
+        if scale > MAX_PRECISION_U32 {
             return Err(Error::ScaleExceedsMaximumPrecision(scale));
         }
         self.flags = (scale << SCALE_SHIFT) | (self.flags & SIGN_MASK);
@@ -691,13 +691,25 @@ impl Decimal {
     /// * Bytes 9-12: mid portion of `m`
     /// * Bytes 13-16: high portion of `m`
     #[must_use]
-    pub const fn deserialize(bytes: [u8; 16]) -> Decimal {
-        Decimal {
-            flags: (bytes[0] as u32) | (bytes[1] as u32) << 8 | (bytes[2] as u32) << 16 | (bytes[3] as u32) << 24,
+    pub fn deserialize(bytes: [u8; 16]) -> Decimal {
+        // We can bound flags by a bitwise mask to correspond to:
+        //   Bits 0-15: unused
+        //   Bits 16-23: Contains "e", a value between 0-28 that indicates the scale
+        //   Bits 24-30: unused
+        //   Bit 31: the sign of the Decimal value, 0 meaning positive and 1 meaning negative.
+        let raw = Decimal {
+            flags: ((bytes[0] as u32) | (bytes[1] as u32) << 8 | (bytes[2] as u32) << 16 | (bytes[3] as u32) << 24)
+                & 0x801F_0000,
             lo: (bytes[4] as u32) | (bytes[5] as u32) << 8 | (bytes[6] as u32) << 16 | (bytes[7] as u32) << 24,
             mid: (bytes[8] as u32) | (bytes[9] as u32) << 8 | (bytes[10] as u32) << 16 | (bytes[11] as u32) << 24,
             hi: (bytes[12] as u32) | (bytes[13] as u32) << 8 | (bytes[14] as u32) << 16 | (bytes[15] as u32) << 24,
+        };
+        // Scale must be bound to maximum precision. The panic currently prevents this function being
+        // marked as a const function.
+        if raw.scale() > MAX_PRECISION_U32 {
+            panic!("{}", Error::ScaleExceedsMaximumPrecision(raw.scale()))
         }
+        raw
     }
 
     /// Returns `true` if the decimal is negative.
@@ -1454,7 +1466,7 @@ impl Decimal {
         // In order to bring exponent up to -MAX_PRECISION, the mantissa should
         // be divided by 10 to compensate. If the exponent10 is too small, this
         // will cause the mantissa to underflow and become 0.
-        while exponent10 < -(MAX_PRECISION as i32) {
+        while exponent10 < -(MAX_PRECISION_U32 as i32) {
             let rem10 = ops::array::div_by_u32(bits, 10);
             exponent10 += 1;
             if ops::array::is_all_zero(bits) {
@@ -2053,8 +2065,13 @@ impl core::convert::TryFrom<Decimal> for f64 {
 
 impl fmt::Display for Decimal {
     fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        let rep = crate::str::to_str_internal(self, false, f.precision());
-        f.pad_integral(self.is_sign_positive(), "", rep.as_str())
+        let (rep, additional) = crate::str::to_str_internal(self, false, f.precision());
+        if let Some(additional) = additional {
+            let value = [rep.as_str(), "0".repeat(additional).as_str()].concat();
+            f.pad_integral(self.is_sign_positive(), "", value.as_str())
+        } else {
+            f.pad_integral(self.is_sign_positive(), "", rep.as_str())
+        }
     }
 }
 

src/error.rs

@@ -1,4 +1,4 @@
-use crate::constants::MAX_PRECISION;
+use crate::constants::MAX_PRECISION_U32;
 use alloc::string::String;
 use core::fmt;
 
@@ -38,7 +38,7 @@ impl fmt::Display for Error {
                 write!(
                     f,
                     "Scale exceeds the maximum precision allowed: {} > {}",
-                    scale, MAX_PRECISION
+                    scale, MAX_PRECISION_U32
                 )
             }
         }

src/ops/legacy.rs

@@ -1,5 +1,5 @@
 use crate::{
-    constants::{MAX_PRECISION, POWERS_10, U32_MASK},
+    constants::{MAX_PRECISION_U32, POWERS_10, U32_MASK},
     decimal::{CalculationResult, Decimal},
     ops::array::{
         add_by_internal, cmp_internal, div_by_u32, is_all_zero, mul_by_u32, mul_part, rescale_internal, shl1_internal,
@@ -171,16 +171,16 @@ pub(crate) fn div_impl(d1: &Decimal, d2: &Decimal) -> CalculationResult {
 
     // Check for underflow
     let mut final_scale: u32 = quotient_scale as u32;
-    if final_scale > MAX_PRECISION {
+    if final_scale > MAX_PRECISION_U32 {
         let mut remainder = 0;
 
         // Division underflowed. We must remove some significant digits over using
         //  an invalid scale.
-        while final_scale > MAX_PRECISION && !is_all_zero(&quotient) {
+        while final_scale > MAX_PRECISION_U32 && !is_all_zero(&quotient) {
             remainder = div_by_u32(&mut quotient, 10);
             final_scale -= 1;
         }
-        if final_scale > MAX_PRECISION {
+        if final_scale > MAX_PRECISION_U32 {
             // Result underflowed so set to zero
             final_scale = 0;
             quotient_negative = false;
@@ -228,8 +228,8 @@ pub(crate) fn mul_impl(d1: &Decimal, d2: &Decimal) -> CalculationResult {
         let mut u64_result = u64_to_array(u64::from(my[0]) * u64::from(ot[0]));
 
         // If we're above max precision then this is a very small number
-        if final_scale > MAX_PRECISION {
-            final_scale -= MAX_PRECISION;
+        if final_scale > MAX_PRECISION_U32 {
+            final_scale -= MAX_PRECISION_U32;
 
             // If the number is above 19 then this will equate to zero.
             // This is because the max value in 64 bits is 1.84E19
@@ -258,7 +258,7 @@ pub(crate) fn mul_impl(d1: &Decimal, d2: &Decimal) -> CalculationResult {
                 u64_result[0] += 1;
             }
 
-            final_scale = MAX_PRECISION;
+            final_scale = MAX_PRECISION_U32;
         }
         return CalculationResult::Ok(Decimal::from_parts(
             u64_result[0],
@@ -350,17 +350,17 @@ pub(crate) fn mul_impl(d1: &Decimal, d2: &Decimal) -> CalculationResult {
 
     // If we're still above max precision then we'll try again to
     // reduce precision - we may be dealing with a limit of "0"
-    if final_scale > MAX_PRECISION {
+    if final_scale > MAX_PRECISION_U32 {
         // We're in an underflow situation
         // The easiest way to remove precision is to divide off the result
-        while final_scale > MAX_PRECISION && !is_all_zero(&product) {
+        while final_scale > MAX_PRECISION_U32 && !is_all_zero(&product) {
             div_by_u32(&mut product, 10);
             final_scale -= 1;
         }
         // If we're still at limit then we can't represent any
         // significant decimal digits and will return an integer only
         // Can also be invoked while representing 0.
-        if final_scale > MAX_PRECISION {
+        if final_scale > MAX_PRECISION_U32 {
             final_scale = 0;
         }
     } else if !(product[3] == 0 && product[4] == 0 && product[5] == 0) {

src/ops/mul.rs

@@ -1,4 +1,4 @@
-use crate::constants::{BIG_POWERS_10, MAX_I64_SCALE, MAX_PRECISION, U32_MAX};
+use crate::constants::{BIG_POWERS_10, MAX_I64_SCALE, MAX_PRECISION_U32, U32_MAX};
 use crate::decimal::{CalculationResult, Decimal};
 use crate::ops::common::Buf24;
 
@@ -18,15 +18,15 @@ pub(crate) fn mul_impl(d1: &Decimal, d2: &Decimal) -> CalculationResult {
         if d2.hi() | d2.mid() == 0 {
             // We're multiplying two 32 bit integers, so we can take some liberties to optimize this.
             let mut low64 = d1.lo() as u64 * d2.lo() as u64;
-            if scale > MAX_PRECISION {
+            if scale > MAX_PRECISION_U32 {
                 // We've exceeded maximum scale so we need to start reducing the precision (aka
                 // rounding) until we have something that fits.
                 // If we're too big then we effectively round to zero.
-                if scale > MAX_PRECISION + MAX_I64_SCALE {
+                if scale > MAX_PRECISION_U32 + MAX_I64_SCALE {
                     return CalculationResult::Ok(Decimal::ZERO);
                 }
 
-                scale -= MAX_PRECISION + 1;
+                scale -= MAX_PRECISION_U32 + 1;
                 let mut power = BIG_POWERS_10[scale as usize];
 
                 let tmp = low64 / power;
@@ -39,7 +39,7 @@ pub(crate) fn mul_impl(d1: &Decimal, d2: &Decimal) -> CalculationResult {
                     low64 += 1;
                 }
 
-                scale = MAX_PRECISION;
+                scale = MAX_PRECISION_U32;
             }
 
             // Early exit
@@ -129,7 +129,7 @@ pub(crate) fn mul_impl(d1: &Decimal, d2: &Decimal) -> CalculationResult {
     // We may want to "rescale". This is the case if the mantissa is > 96 bits or if the scale
     // exceeds the maximum precision.
     let upper_word = product.upper_word();
-    if upper_word > 2 || scale > MAX_PRECISION {
+    if upper_word > 2 || scale > MAX_PRECISION_U32 {
         scale = if let Some(new_scale) = product.rescale(upper_word, scale) {
             new_scale
         } else {

src/serde.rs

@@ -174,7 +174,8 @@ impl serde::Serialize for Decimal {
     where
         S: serde::Serializer,
     {
-        serializer.serialize_str(crate::str::to_str_internal(self, true, None).as_ref())
+        let value = crate::str::to_str_internal(self, true, None);
+        serializer.serialize_str(value.0.as_ref())
     }
 }
 

src/str.rs

@@ -1,5 +1,5 @@
 use crate::{
-    constants::MAX_STR_BUFFER_SIZE,
+    constants::{MAX_PRECISION, MAX_STR_BUFFER_SIZE},
     error::Error,
     ops::array::{add_by_internal, add_one_internal, div_by_u32, is_all_zero, mul_by_10, mul_by_u32},
     Decimal,
@@ -15,7 +15,7 @@ pub(crate) fn to_str_internal(
     value: &Decimal,
     append_sign: bool,
     precision: Option<usize>,
-) -> ArrayString<[u8; MAX_STR_BUFFER_SIZE]> {
+) -> (ArrayString<[u8; MAX_STR_BUFFER_SIZE]>, Option<usize>) {
     // Get the scale - where we need to put the decimal point
     let scale = value.scale() as usize;
 
@@ -30,9 +30,16 @@ pub(crate) fn to_str_internal(
         chars.push('0');
     }
 
-    let prec = match precision {
-        Some(prec) => prec,
-        None => scale,
+    let (prec, additional) = match precision {
+        Some(prec) => {
+            let max: usize = MAX_PRECISION.into();
+            if prec > max {
+                (max, Some(prec - max))
+            } else {
+                (prec, None)
+            }
+        }
+        None => (scale, None),
     };
 
     let len = chars.len();
@@ -66,7 +73,7 @@ pub(crate) fn to_str_internal(
         rep.push('0');
     }
 
-    rep
+    (rep, additional)
 }
 
 pub(crate) fn fmt_scientific_notation(
@@ -531,3 +538,18 @@ pub(crate) fn parse_str_radix_n(str: &str, radix: u32) -> Result<Decimal, crate:
 
     Ok(Decimal::from_parts(data[0], data[1], data[2], negative, scale))
 }
+
+#[cfg(test)]
+mod test {
+    use crate::Decimal;
+    use arrayvec::ArrayString;
+    use core::{fmt::Write, str::FromStr};
+
+    #[test]
+    fn display_does_not_overflow_max_capacity() {
+        let num = Decimal::from_str("1.2").unwrap();
+        let mut buffer = ArrayString::<[u8; 64]>::new();
+        let _ = buffer.write_fmt(format_args!("{:.31}", num)).unwrap();
+        assert_eq!("1.2000000000000000000000000000000", buffer.as_str());
+    }
+}

tests/decimal_tests.rs

@@ -132,6 +132,23 @@ fn it_can_serialize_deserialize() {
     }
 }
 
+#[test]
+#[should_panic(expected = "Scale exceeds the maximum precision allowed: 30 > 28")]
+fn it_panics_deserializing_unbounded_values() {
+    let _ = Decimal::deserialize([1u8, 0, 30, 206, 97, 81, 216, 182, 20, 30, 165, 78, 18, 155, 169, 62]);
+}
+
+#[test]
+fn it_can_deserialize_bounded_values() {
+    let tests = [[1u8, 0, 28, 206, 97, 81, 216, 182, 20, 30, 165, 78, 18, 155, 169, 62]];
+    for &bytes in &tests {
+        let dec = Decimal::deserialize(bytes);
+        let string = format!("{:.9999}", dec);
+        let dec2 = Decimal::from_str(&string).unwrap();
+        assert_eq!(dec, dec2);
+    }
+}
+
 // Formatting
 
 #[test]