diff --git a/crates/core/src/validation/number.rs b/crates/core/src/validation/number.rs index 304fd9f40..03fd195f6 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,24 @@ 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() { + // The exponent of the normalized form `0.d * 10^n` is the point + // offset plus one, and DynamoDB computes it in 64 bits, where + // i64::MAX + 1 wraps to a negative exponent: `0E9223372036854775807` + // is an underflow, while `0E9223372036854775806` and + // `0.0E9223372036854775807` are overflows. + if normalized_exponent_exceeds_i64(point_offset) { + return Err(underflow_err()); + } + check_magnitude(point_offset)?; + } return Ok("0".to_owned()); } @@ -76,32 +97,37 @@ pub fn validate_and_normalize_number(s: &str) -> Result { return Ok("0".to_owned()); } - if sig_trimmed.len() > MAX_SIGNIFICANT_DIGITS { - return Err(too_many_digits_err()); - } - // 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); - - if magnitude_exp > i64::from(MAX_EXPONENT) { + let magnitude_exp = digit_count(sig_trimmed) - 1 + exp; + + // DynamoDB reports an overflow before it counts the digits when the + // exponent of the normalized form, the magnitude plus one, is above + // i64::MAX. Measured with `1234567890123456789012345678901234567890` as + // the digits: `E9223372036854775768` (an order of magnitude of i64::MAX), + // `E9223372036854775769` (i64::MAX + 1) and `E9223372036854775807` + // (i64::MAX + 39) are overflows, and `E9223372036854775767` is too many + // digits. The one-digit `1E9223372036854775807` (i64::MAX) and + // `10E9223372036854775807` (i64::MAX + 1) are overflows as well. + if normalized_exponent_exceeds_i64(magnitude_exp) { return Err(overflow_err()); } - if magnitude_exp < i64::from(MIN_EXPONENT) { - return Err(underflow_err()); + + if sig_trimmed.len() > MAX_SIGNIFICANT_DIGITS { + return Err(too_many_digits_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()); - }; + + check_magnitude(magnitude_exp)?; + + // 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 +141,31 @@ 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) +} + +/// Whether the exponent of the normalized form `0.d * 10^n`, which is the +/// given order of magnitude plus one, is above `i64::MAX`. `DynamoDB` answers +/// such a nonzero value with an overflow before it counts the digits, and such +/// a zero with an underflow. Measured with forty digits at orders of magnitude +/// of `i64::MAX`, `i64::MAX + 1` and `i64::MAX + 39`, with one and two digits +/// at `i64::MAX` and `i64::MAX + 1`, and with the zero `0E9223372036854775807`. +fn normalized_exponent_exceeds_i64(magnitude_exp: i128) -> bool { + magnitude_exp + 1 > i128::from(i64::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 +417,175 @@ 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:?}"); + } + } + + // DynamoDB's answers to the values in the next two tests were measured + // with PutItem in us-east-1 on 2026-11-09, each value sent twice with the + // same answer both times. + const TOO_MANY_DIGITS: &str = "Attempting to store more than 38 significant digits in a Number"; + + #[test] + fn zero_with_exponent_at_i64_max_is_an_underflow() { + // The exponent of a zero's normalized form is its point offset plus + // one, computed in 64 bits: at i64::MAX the sum wraps to a negative + // exponent. A fraction digit lowers the point offset before the sum. + assert_refused("0E9223372036854775807", UNDERFLOW); + for n in ["0E9223372036854775806", "0.0E9223372036854775807"] { + assert_refused(n, OVERFLOW); + } + } + + #[test] + fn magnitude_at_i64_max_is_an_overflow_before_the_digit_count() { + // Forty digits with the order of magnitude at i64::MAX: the overflow + // is reported before the digits are counted. With the exponent one + // lower, the digit count is reported. + assert_refused( + "1234567890123456789012345678901234567890E9223372036854775768", + OVERFLOW, + ); + assert_refused( + "1234567890123456789012345678901234567890E9223372036854775767", + TOO_MANY_DIGITS, + ); + // The same digits with the order of magnitude at i64::MAX + 1 and at + // i64::MAX + 39. Measured on 2026-11-10, each value sent twice with + // the same answer both times. + for n in [ + "1234567890123456789012345678901234567890E9223372036854775769", + "1234567890123456789012345678901234567890E9223372036854775807", + ] { + assert_refused(n, OVERFLOW); + } + } } diff --git a/tests/rust/src/number_validation.rs b/tests/rust/src/number_validation.rs index a6d86a254..bf8214acc 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,125 @@ 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}")); + } +} + +// DynamoDB's answers below were measured with PutItem in us-east-1 on +// 2026-11-09, each value twice with the same answer both times, except the two +// values marked 2026-11-10. The checks match on the full message body, which +// DynamoDB prefixes with "1 validation error detected: " and ExtendDB does not. +const OVERFLOW_MESSAGE: &str = + "Number overflow. Attempting to store a number with magnitude larger than supported range"; +const UNDERFLOW_MESSAGE: &str = + "Number underflow. Attempting to store a number with magnitude smaller than supported range"; +const TOO_MANY_DIGITS_MESSAGE: &str = + "Attempting to store more than 38 significant digits in a Number"; + +#[tokio::test] +async fn put_item_zero_with_exponent_at_i64_max_is_underflow() { + let t = tables().await; + for (n, want) in [ + ("0E9223372036854775807", UNDERFLOW_MESSAGE), + ("0E9223372036854775806", OVERFLOW_MESSAGE), + ("0.0E9223372036854775807", OVERFLOW_MESSAGE), + ] { + let err = put_with_raw_number(&t.simple_key_string, n) + .await + .expect_err("zero with an exponent at the i64 limit must be rejected"); + assert!( + err.starts_with("ValidationException") && err.contains(want), + "{n}: got {err}" + ); + } +} + +#[tokio::test] +async fn put_item_magnitude_at_i64_max_is_overflow_before_digit_count() { + let t = tables().await; + for (n, want) in [ + ( + "1234567890123456789012345678901234567890E9223372036854775768", + OVERFLOW_MESSAGE, + ), + ( + "1234567890123456789012345678901234567890E9223372036854775767", + TOO_MANY_DIGITS_MESSAGE, + ), + // The order of magnitude at i64::MAX + 1 and at i64::MAX + 39. + // Measured on 2026-11-10, each value sent twice. + ( + "1234567890123456789012345678901234567890E9223372036854775769", + OVERFLOW_MESSAGE, + ), + ( + "1234567890123456789012345678901234567890E9223372036854775807", + OVERFLOW_MESSAGE, + ), + ] { + let err = put_with_raw_number(&t.simple_key_string, n) + .await + .expect_err("forty digits with an out-of-range magnitude must be rejected"); + assert!( + err.starts_with("ValidationException") && err.contains(want), + "{n}: got {err}" + ); + } +}