From bfde5fd3364eb7dbe2bebcd3176ace97e5800f68 Mon Sep 17 00:00:00 2001 From: Lee Hannigan <29015211+LeeroyHannigan@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:49:44 +0000 Subject: [PATCH] fix(core): range check number exponents as DynamoDB does validate_and_normalize_number parsed the exponent as an i32. #394 moved its exponent sums to saturating i64 arithmetic, which fixed the overflow panics of #616, but an exponent outside the i32 range was still refused as malformed. The exponent is now parsed as an i64 and the sums are done in i128 instead of saturating i64, so none of them can overflow. DynamoDB reads the exponent the same way: 1E2147483648 is a Number overflow and 1E9223372036854775808 cannot be converted. A zero whose exponent does not fit in an i32 is range checked too; main refused those as malformed. A zero whose exponent fits in an i32 stays accepted as 0, although DynamoDB refuses 0E126 and 0.0E-130. A refused zero keeps its raw spelling in AttributeValue, and only the PutItem item and UpdateItem's update values are validated afterwards, so UpdateItem's key and the other operations would store 0E126 as a key separate from 0. #618 covers both. Closes #616 --- crates/core/src/validation/number.rs | 192 +++++++++++++++++++++++---- tests/rust/src/number_validation.rs | 66 ++++++++- 2 files changed, 232 insertions(+), 26 deletions(-) diff --git a/crates/core/src/validation/number.rs b/crates/core/src/validation/number.rs index 304fd9f40..d0e47eefc 100644 --- a/crates/core/src/validation/number.rs +++ b/crates/core/src/validation/number.rs @@ -22,8 +22,11 @@ pub fn validate_and_normalize_number(s: &str) -> Result { None => (false, s.strip_prefix('+').unwrap_or(s)), }; + // DynamoDB reads the exponent as a signed 64-bit integer: one outside that + // range is malformed (`1E9223372036854775808`), one inside it is range + // checked below (`1E2147483648` is an overflow). let (mantissa_str, explicit_exp) = match rest.split_once(['e', 'E']) { - Some((m, e)) => (m, e.parse::().map_err(|_| numeric_value_err(s))?), + Some((m, e)) => (m, e.parse::().map_err(|_| numeric_value_err(s))?), None => (rest, 0), }; @@ -53,10 +56,10 @@ pub fn validate_and_normalize_number(s: &str) -> Result { // Combine into a single digit string and track where the decimal point is. // The value is: . * 10^explicit_exp // Which equals: * 10^(explicit_exp - frac_part.len()) - // The exponent arithmetic is done in i64: the explicit exponent may be at - // either end of i32, and adding a digit count to it overflows i32. + // The exponent arithmetic is done in i128: an i64 exponent plus or minus a + // digit count cannot leave that range. let all_digits = format!("{int_part}{frac_part}"); - let point_offset = i64::from(explicit_exp).saturating_sub(len_i64(frac_part.len())); + let point_offset = i128::from(explicit_exp) - digit_count(frac_part); // Strip leading zeros to get significant digits let sig_start = all_digits @@ -65,6 +68,16 @@ pub fn validate_and_normalize_number(s: &str) -> Result { let significant = &all_digits[sig_start..]; if significant.is_empty() { + // DynamoDB range checks a zero on its exponent less its fraction + // digits (`0E126` is an overflow, `0.0E-130` an underflow). Only a zero + // whose exponent does not fit in an i32 is checked here, since those + // were already refused: a zero refused here keeps its raw spelling in + // `AttributeValue`, and only the PutItem item and UpdateItem's update + // values are validated afterwards (#618), so the other operations, and + // UpdateItem's key, would store and look up `0E126` as a key of its own. + if i32::try_from(explicit_exp).is_err() { + check_magnitude(point_offset)?; + } return Ok("0".to_owned()); } @@ -83,25 +96,16 @@ pub fn validate_and_normalize_number(s: &str) -> Result { // The exponent for the normalized form: // value = all_digits_as_integer * 10^point_offset // = significant_as_integer * 10^point_offset (leading zeros don't change value) - // = sig_trimmed_as_integer * 10^(point_offset + trailing_zeros) - let trailing_zeros = significant.len() - sig_end; - let exp = point_offset.saturating_add(len_i64(trailing_zeros)); + // = sig_trimmed_as_integer * 10^(point_offset + the trailing zeros of significant) + let exp = point_offset + digit_count(&significant[sig_end..]); // Magnitude: the number is sig_trimmed * 10^exp, so its order of magnitude // is sig_trimmed.len() - 1 + exp - let magnitude_exp = (len_i64(sig_trimmed.len()) - 1).saturating_add(exp); + check_magnitude(digit_count(sig_trimmed) - 1 + exp)?; - if magnitude_exp > i64::from(MAX_EXPONENT) { - return Err(overflow_err()); - } - if magnitude_exp < i64::from(MIN_EXPONENT) { - return Err(underflow_err()); - } - // In range: the magnitude is within [-130, 125] and there are at most 38 - // significant digits, so `exp` is within [-167, 125]. - let Ok(exp) = i32::try_from(exp) else { - return Err(overflow_err()); - }; + // The magnitude is in range and `sig_trimmed` has at most 38 digits, so + // `exp` is between MIN_EXPONENT - 37 and MAX_EXPONENT and fits in an i32. + let exp = i32::try_from(exp).map_err(|_| overflow_err())?; // Format the normalized number // sig_trimmed represents an integer, and we multiply by 10^exp @@ -115,10 +119,21 @@ pub fn validate_and_normalize_number(s: &str) -> Result { Ok(result) } -/// A length as `i64` for the exponent arithmetic. A `str` is at most -/// `isize::MAX` bytes, so this never saturates. -fn len_i64(len: usize) -> i64 { - i64::try_from(len).unwrap_or(i64::MAX) +/// The length of an ASCII digit string, as an i128 for the exponent arithmetic. +fn digit_count(digits: &str) -> i128 { + // A `usize` always fits in an `i128`. + i128::try_from(digits.len()).unwrap_or(i128::MAX) +} + +/// Refuse an order of magnitude outside `DynamoDB`'s supported range. +fn check_magnitude(magnitude_exp: i128) -> Result<(), DynamoDbError> { + if magnitude_exp > i128::from(MAX_EXPONENT) { + return Err(overflow_err()); + } + if magnitude_exp < i128::from(MIN_EXPONENT) { + return Err(underflow_err()); + } + Ok(()) } /// Format digits * 10^exp as a plain decimal string (no scientific notation). @@ -370,4 +385,135 @@ mod tests { "The parameter cannot be converted to a numeric value" ); } + + // Every value in these tests was sent to DynamoDB's PutItem twice on + // 2026-10-27, with the same answer both times. Each test expects + // DynamoDB's answer, except the last, which says why. + const OVERFLOW: &str = + "Number overflow. Attempting to store a number with magnitude larger than supported range"; + const UNDERFLOW: &str = "Number underflow. Attempting to store a number with magnitude smaller than supported range"; + + fn assert_refused(input: &str, want: &str) { + match validate_and_normalize_number(input) { + Ok(n) => panic!("{input:?} accepted as {n:?}, want {want:?}"), + Err(e) => assert_eq!(e.to_string(), want, "{input:?}"), + } + } + + #[test] + fn magnitude_counts_every_significant_digit() { + for n in [ + "12E124", "99E124", "10E124", "100E123", "1.2E-130", "10E-131", + ] { + assert!(validate_and_normalize_number(n).is_ok(), "{n:?}"); + } + for n in ["12E125", "10E125"] { + assert_refused(n, OVERFLOW); + } + for n in ["1.2E-131", "10E-132"] { + assert_refused(n, UNDERFLOW); + } + } + + #[test] + fn exponent_near_i32_limits_is_an_overflow_or_underflow() { + for n in [ + "10E2147483647", + "100E2147483647", + "12E2147483647", + "-10E2147483647", + "1E+2147483647", + "0.1E2147483647", + "1E2147483647", + ] { + assert_refused(n, OVERFLOW); + } + for n in [ + "1.5E-2147483648", + "-1.5E-2147483648", + "0.01E-2147483648", + "1E-2147483648", + ] { + assert_refused(n, UNDERFLOW); + } + } + + #[test] + fn exponent_is_read_as_i64() { + for n in [ + "1E2147483648", + "1E4294967296", + "1E9223372036854775807", + "10E9223372036854775807", + ] { + assert_refused(n, OVERFLOW); + } + for n in [ + "1E-2147483649", + "1E-9223372036854775808", + "0.1E-9223372036854775808", + ] { + assert_refused(n, UNDERFLOW); + } + for n in [ + "1E9223372036854775808", + "1E-9223372036854775809", + "1E99999999999999999999", + ] { + assert_refused( + n, + &format!("The parameter cannot be converted to a numeric value: {n}"), + ); + } + } + + #[test] + fn zero_with_exponent_outside_i32_is_range_checked() { + // DynamoDB accepts each of these as 0. + for n in [ + "0E125", + "0.0E126", + "000.0E126", + "-0.0E126", + "0E-130", + "0.0E-129", + "0E0", + "0.0", + ] { + assert_eq!(validate_and_normalize_number(n).unwrap(), "0", "{n:?}"); + } + // A zero whose exponent does not fit in an i32. DynamoDB also refuses a + // zero such as `0E126` whose exponent fits; see the comment in + // `validate_and_normalize_number` for why those are not checked. + for n in ["0E2147483648", "0E9223372036854775806"] { + assert_refused(n, OVERFLOW); + } + for n in ["0E-9223372036854775807", "0E-9223372036854775808"] { + assert_refused(n, UNDERFLOW); + } + } + + // DynamoDB refuses these zeros (`0E126` overflow, `0.0E-130` underflow). + // They stay accepted as 0 until every operation validates numbers (#618): + // refusing them here would make BatchWriteItem and the other operations + // that skip validation store and look up `0E126` as a key of its own. + // Remove this test and add the rows to the one above when #618 is fixed. + #[test] + fn zero_with_i32_exponent_stays_accepted_until_618() { + for n in [ + "0E126", + "00E126", + "-0E126", + "0E2147483647", + "00.000E2147483647", + "0.0E-130", + "0E-131", + "-0E-131", + "0.00E-131", + "0E-2147483648", + "0.0E-2147483648", + ] { + assert_eq!(validate_and_normalize_number(n).unwrap(), "0", "{n:?}"); + } + } } diff --git a/tests/rust/src/number_validation.rs b/tests/rust/src/number_validation.rs index a6d86a254..eb79cb79f 100644 --- a/tests/rust/src/number_validation.rs +++ b/tests/rust/src/number_validation.rs @@ -1,9 +1,9 @@ // Copyright 2026 ExtendDB contributors // SPDX-License-Identifier: Apache-2.0 -//! Number (`N`) attribute validation: whitespace and bare-dot inputs must be -//! rejected with a ValidationException, matching real DynamoDB. Covers the -//! parity fix that stopped trimming/accepting malformed numeric values. +//! Number (`N`) attribute validation: malformed values (whitespace, a bare +//! dot, an exponent outside i64) and out-of-range magnitudes must be rejected +//! with a ValidationException, as DynamoDB does. use crate::test_base::*; use std::collections::HashMap; @@ -63,3 +63,63 @@ async fn put_item_accepts_valid_number() { .await .expect("valid number must be accepted"); } + +// DynamoDB's answers below were measured with PutItem on 2026-10-27, each value +// twice. It prefixes each message with "1 validation error detected: ", which +// ExtendDB does not, so the checks match on the message body. + +#[tokio::test] +async fn put_item_exponent_near_i32_limits_is_overflow_or_underflow() { + let t = tables().await; + for (n, want) in [ + ("10E2147483647", "Number overflow."), + ("12E2147483647", "Number overflow."), + ("1E2147483648", "Number overflow."), + ("1.5E-2147483648", "Number underflow."), + ("1E-2147483649", "Number underflow."), + ] { + let err = put_with_raw_number(&t.simple_key_string, n) + .await + .expect_err("out-of-range number must be rejected"); + assert!( + err.starts_with("ValidationException") && err.contains(want), + "{n}: got {err}" + ); + } +} + +#[tokio::test] +async fn put_item_exponent_outside_i64_is_malformed() { + let t = tables().await; + let n = "1E9223372036854775808"; + let err = put_with_raw_number(&t.simple_key_string, n) + .await + .expect_err("exponent outside i64 must be rejected"); + assert!( + err.starts_with("ValidationException") + && err.contains(&format!("cannot be converted to a numeric value: {n}")), + "got {err}" + ); +} + +#[tokio::test] +async fn put_item_zero_with_exponent_outside_i32_is_rejected() { + let t = tables().await; + for (n, want) in [ + ("0E2147483648", "Number overflow."), + ("0E-9223372036854775808", "Number underflow."), + ] { + let err = put_with_raw_number(&t.simple_key_string, n) + .await + .expect_err("zero with an out-of-range exponent must be rejected"); + assert!( + err.starts_with("ValidationException") && err.contains(want), + "{n}: got {err}" + ); + } + for n in ["0E125", "0.0E126", "0E-130"] { + put_with_raw_number(&t.simple_key_string, n) + .await + .unwrap_or_else(|e| panic!("{n} must be accepted: {e}")); + } +}