rounding fails with "Invalid decimal: overflow from mantissa after rounding"

[CAD97]

Fun edge case. The decimal 792.281625142643375935439503356 hits

rust-decimal/src/str.rs

Lines 309 to 312 in 4b71761

	// Make sure that the mantissa isn't now overflowing
	if overflow_128(data) {
	return tail_error("Invalid decimal: overflow from mantissa after rounding");
	}

The correct rounding behavior can be achieved with

--- a/src/str.rs
+++ b/src/str.rs
@@ -306,9 +306,15 @@ fn handle_full_128<const POINT: bool, const NEG: bool, const ROUND: bool>(
                     if digit >= 5 {
                         data += 1;

-                        // Make sure that the mantissa isn't now overflowing
+                        // If the mantissa is now overflowing, round to the next
+                        // next least significant digit and discard precision
                         if overflow_128(data) {
-                            return tail_error("Invalid decimal: overflow from mantissa after rounding");
+                            if scale == 0 {
+                                return tail_error("Invalid decimal: overflow from mantissa after rounding");
+                            }
+                            data += 4;
+                            data /= 10;
+                            return handle_data::<NEG, true>(data, scale - 1);
                         }
                     }
                 } else {

---

With a similar input (7.92281625142643375935439503356), we can hit this same error in maybe_round, where a comment suggests it should be unreachable:

rust-decimal/src/str.rs

Lines 366 to 373 in 4b71761

	// Round at midpoint
	if digit >= 5 {
	data += 1;
	if overflow_128(data) {
	// Highly unlikely scenario which is more indicative of a bug
	return tail_error("Invalid decimal: overflow when rounding");
	}
	}

The same patch can be applied there, but I wonder if unifying the two rounding paths would make sense. I think that full patch would be

diff --git a/src/str.rs b/src/str.rs
index d8e68e4..dafd50a 100644
--- a/src/str.rs
+++ b/src/str.rs
@@ -303,18 +303,10 @@ fn handle_full_128<const POINT: bool, const NEG: bool, const ROUND: bool>(
                 }
 
                 if ROUND {
-                    if digit >= 5 {
-                        data += 1;
-
-                        // Make sure that the mantissa isn't now overflowing
-                        if overflow_128(data) {
-                            return tail_error("Invalid decimal: overflow from mantissa after rounding");
-                        }
-                    }
+                    maybe_round(data, next_byte, scale, POINT, NEG)
                 } else {
-                    return Err(Error::Underflow);
+                    Err(Error::Underflow)
                 }
-                handle_data::<NEG, true>(data, scale)
             } else {
                 data = next;
                 let scale = scale + POINT as u8;
@@ -355,7 +347,13 @@ fn handle_full_128<const POINT: bool, const NEG: bool, const ROUND: bool>(
 
 #[inline(never)]
 #[cold]
-fn maybe_round(mut data: u128, next_byte: u8, scale: u8, point: bool, negative: bool) -> Result<Decimal, crate::Error> {
+fn maybe_round(
+    mut data: u128,
+    next_byte: u8,
+    mut scale: u8,
+    point: bool,
+    negative: bool,
+) -> Result<Decimal, crate::Error> {
     let digit = match next_byte {
         b'0'..=b'9' => u32::from(next_byte - b'0'),
         b'_' => 0, // this should be an invalid string?
@@ -366,9 +364,16 @@ fn maybe_round(mut data: u128, next_byte: u8, scale: u8, point: bool, negative:
     // Round at midpoint
     if digit >= 5 {
         data += 1;
+
+        // If the mantissa is now overflowing, round to the next
+        // next least significant digit and discard precision
         if overflow_128(data) {
-            // Highly unlikely scenario which is more indicative of a bug
-            return tail_error("Invalid decimal: overflow when rounding");
+            if scale == 0 {
+                return tail_error("Invalid decimal: overflow from mantissa after rounding");
+            }
+            data += 4;
+            data /= 10;
+            scale -= 1;
         }
     }

I'm submitting this as a PR.

---

If I've deconstructed the control flow correctly, to hit these errors you need a decimal with a prefix in 0xF00000000000000000000000..0x1000000000000000000000000 and a following digit >= 5. If the decimal point appears anywhere within or directly after that prefix, this error is hit. The maybe_round version of the error is only hit when the decimal point is exactly at index 1.