diff --git a/crates/core/src/expression/evaluator.rs b/crates/core/src/expression/evaluator.rs index e023af96e..7db0635b0 100755 --- a/crates/core/src/expression/evaluator.rs +++ b/crates/core/src/expression/evaluator.rs @@ -14,7 +14,9 @@ use crate::error::DynamoDbError; use crate::types::AttributeValue; use super::ast::{CompareOp, Expr, PathElement}; -use super::resolver::{ExpressionMaps, attribute_type_code, resolve_path}; +use super::resolver::{ + ExpressionMaps, ParsedNumber, attribute_type_code, number_value, resolve_path, +}; /// Evaluate a condition expression against an item. /// @@ -127,13 +129,11 @@ fn resolve_to_value<'a>( } } -/// Look up a pre-parsed `BigDecimal` for a placeholder expression. +/// Look up a pre-parsed number for a placeholder expression. /// -/// Returns `None` if the expression is not a placeholder or has no pre-parsed value. -fn placeholder_numeric<'a>( - expr: &Expr, - maps: &'a ExpressionMaps, -) -> Option<&'a bigdecimal::BigDecimal> { +/// Returns `None` if the expression is not a placeholder or has no pre-parsed +/// value. +fn placeholder_numeric<'a>(expr: &Expr, maps: &'a ExpressionMaps) -> Option> { if let Expr::Placeholder(name) = expr { maps.get_parsed_numeric(name) } else { @@ -141,51 +141,47 @@ fn placeholder_numeric<'a>( } } +/// One side of a numeric comparison as a decimal: the placeholder's +/// pre-parsed value when there is one, else `n` read with [`number_value`]. +/// `None` when the number is not one the service can store. +fn comparison_number<'a>( + n: &str, + parsed: Option>, +) -> Option> { + match parsed { + Some(ParsedNumber::Number(d)) => Some(Cow::Borrowed(d)), + Some(ParsedNumber::Refused) => None, + None => number_value(n).map(Cow::Owned), + } +} + /// Compare two `AttributeValue`s using the given operator. /// /// `DynamoDB` comparison rules: /// - Same-type comparisons only (except N vs N which is numeric) /// - S: lexicographic UTF-8 -/// - N: numeric comparison via `BigDecimal` (pre-parsed values used when available) +/// - N: numeric comparison via `BigDecimal` (pre-parsed values used when available). +/// An `N` that is not a number the service can store compares as a value of +/// another type: only `<>` is true, so `a <> b` agrees with `NOT (a = b)`. /// - B: lexicographic byte comparison /// - BOOL, NULL, L, M, SS, NS, BS: only equality -// Allow single_match_else: the match arms bind an owned fallback value -// and return a reference, which doesn't simplify to if-let. -#[allow(clippy::single_match_else)] fn compare_values( left: &AttributeValue, right: &AttributeValue, op: CompareOp, - left_parsed: Option<&bigdecimal::BigDecimal>, - right_parsed: Option<&bigdecimal::BigDecimal>, + left_parsed: Option>, + right_parsed: Option>, ) -> bool { match (left, right) { (AttributeValue::S(l), AttributeValue::S(r)) => apply_op(l.cmp(r), op), (AttributeValue::N(l), AttributeValue::N(r)) => { - // Use pre-parsed values when available; fall back to parsing. - let l_owned; - let ld = match left_parsed { - Some(d) => d, - None => { - let Ok(d) = l.parse::() else { - return false; - }; - l_owned = d; - &l_owned - } + let Some(ld) = comparison_number(l, left_parsed) else { + return matches!(op, CompareOp::Ne); }; - let r_owned; - let rd = match right_parsed { - Some(d) => d, - None => { - let Ok(d) = r.parse::() else { - return false; - }; - r_owned = d; - &r_owned - } + let Some(rd) = comparison_number(r, right_parsed) else { + return matches!(op, CompareOp::Ne); }; - apply_op(ld.cmp(rd), op) + apply_op(ld.as_ref().cmp(rd.as_ref()), op) } (AttributeValue::B(l), AttributeValue::B(r)) => apply_op(l.cmp(r), op), // For non-orderable types, only equality is meaningful diff --git a/crates/core/src/expression/evaluator_tests.rs b/crates/core/src/expression/evaluator_tests.rs index 2eb32b0c0..b08150044 100755 --- a/crates/core/src/expression/evaluator_tests.rs +++ b/crates/core/src/expression/evaluator_tests.rs @@ -4,6 +4,7 @@ use super::*; use crate::expression::parser::parse_condition; use crate::expression::resolver::ExpressionMaps; +use crate::expression::test_timeout::within_10_s; use crate::expression::tokenizer::tokenize; use std::collections::HashMap; @@ -438,3 +439,98 @@ fn size_of_null_attribute_yields_no_value() { values.insert("zero".into(), AttributeValue::N("0".into())); assert!(!eval("size(nu) > :zero", &item, HashMap::new(), values).unwrap()); } + +/// Evaluate `expr_str` on another thread and return its answer, or fail the +/// test after 10 seconds. +fn eval_within_10_s( + expr_str: &'static str, + item: BTreeMap, + values: HashMap, +) -> Result { + within_10_s(&format!("`{expr_str}`"), move || { + eval(expr_str, &item, HashMap::new(), values) + }) +} + +/// A number with millions of digits, in a placeholder or in the item, is not +/// parsed as written: bigdecimal's parse is quadratic in the digit count, and +/// neither the placeholders nor a stored item are checked for numbers on +/// every path before a comparison. Such a number is not one the service can +/// store, and only `<>` is true for it. +#[test] +fn a_number_with_millions_of_digits_is_not_parsed() { + let long = format!("1{}", "0".repeat(2_000_000)); + let item = BTreeMap::from([("n".to_owned(), AttributeValue::N("1".to_owned()))]); + let values = HashMap::from([("v".to_owned(), AttributeValue::N(long.clone()))]); + assert!(!eval_within_10_s("n < :v", item.clone(), values.clone()).unwrap()); + assert!(eval_within_10_s("n <> :v", item, values).unwrap()); + + let item = BTreeMap::from([("n".to_owned(), AttributeValue::N(long))]); + let values = HashMap::from([("v".to_owned(), AttributeValue::N("1".to_owned()))]); + assert!(!eval_within_10_s("n > :v", item, values).unwrap()); +} + +/// A placeholder refused when the maps are built stays refused: a filter +/// does not read its text again for each item it compares. +#[test] +fn a_refused_placeholder_is_read_once_per_request() { + let long = format!("1{}", "0".repeat(2_000_000)); + let matched = within_10_s("100 items against one refused placeholder", move || { + let tokens = tokenize("n < :v").unwrap(); + let expr = parse_condition(&tokens).unwrap(); + let values = HashMap::from([("v".to_owned(), AttributeValue::N(long))]); + let maps = ExpressionMaps::new(HashMap::new(), values); + // The refusal is recorded, whatever the build's speed. + assert_eq!(maps.get_parsed_numeric("v"), Some(ParsedNumber::Refused)); + (0..100) + .map(|i| BTreeMap::from([("n".to_owned(), AttributeValue::N(i.to_string()))])) + .filter(|item| evaluate_condition(&expr, item, &maps).unwrap()) + .count() + }); + assert_eq!(matched, 0); +} + +/// A stored number the service cannot store, as an older release could +/// store it, compares as a value of another type does: only `<>` is true, so +/// `<>` and `NOT (... = ...)` agree. +#[test] +fn a_stored_number_out_of_range_compares_only_as_not_equal() { + for stored in ["1E126", "12345678901234567890123456789012345678901", "abc"] { + let item = BTreeMap::from([("n".to_owned(), AttributeValue::N(stored.to_owned()))]); + for (expr, expected) in [ + ("n = :v", false), + ("n <> :v", true), + ("NOT (n = :v)", true), + ("n > :v", false), + ("n < :v", false), + ("n IN (:v)", false), + ("n BETWEEN :v AND :v", false), + ] { + let values = HashMap::from([("v".to_owned(), AttributeValue::N("0".to_owned()))]); + assert_eq!( + eval(expr, &item, HashMap::new(), values).unwrap(), + expected, + "{expr}, n = {stored}" + ); + } + } +} + +/// A number the service can store still compares numerically, in any +/// spelling. +#[test] +fn numbers_in_any_spelling_compare_by_value() { + let item = BTreeMap::from([("n".to_owned(), AttributeValue::N("150".to_owned()))]); + for (expr, v) in [ + ("n = :v", "1.5E2"), + ("n = :v", "+150.00"), + ("n < :v", "1E125"), + ("n > :v", "1E-130"), + ] { + let values = HashMap::from([("v".to_owned(), AttributeValue::N(v.to_owned()))]); + assert!( + eval(expr, &item, HashMap::new(), values).unwrap(), + "{expr} with :v = {v}" + ); + } +} diff --git a/crates/core/src/expression/mod.rs b/crates/core/src/expression/mod.rs index 4ef721471..2bf8020b5 100755 --- a/crates/core/src/expression/mod.rs +++ b/crates/core/src/expression/mod.rs @@ -18,6 +18,8 @@ mod projection; mod reserved_words; mod resolver; mod search_condition; +#[cfg(test)] +mod test_timeout; mod tokenizer; mod update_evaluator; mod update_parser; diff --git a/crates/core/src/expression/resolver.rs b/crates/core/src/expression/resolver.rs index 3c7414d5d..41637b9cd 100755 --- a/crates/core/src/expression/resolver.rs +++ b/crates/core/src/expression/resolver.rs @@ -35,9 +35,19 @@ pub struct ExpressionMaps { /// sizes a transacted Update, and not by the statement as it sizes /// `UpdateItem`. pub finished_item_sizing: bool, - /// Pre-parsed `BigDecimal` values for numeric placeholders. + /// Pre-parsed `BigDecimal` values for numeric placeholders, `None` for a + /// placeholder whose `N` is not a number the service can store. /// Populated eagerly by `new()` and refreshed by `pre_parse_numerics()`. - parsed_numerics: HashMap, + parsed_numerics: HashMap>, +} + +/// A numeric placeholder as [`ExpressionMaps`] pre-parsed it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ParsedNumber<'a> { + /// The placeholder's `N` is a number the service can store. + Number(&'a bigdecimal::BigDecimal), + /// The placeholder's `N` is not a number the service can store. + Refused, } impl ExpressionMaps { @@ -99,28 +109,47 @@ impl ExpressionMaps { /// Pre-parse all numeric placeholder values into `BigDecimal`. /// /// Call once per request before evaluating filter expressions against - /// multiple items. Invalid numeric strings are silently skipped — they - /// will produce `false` comparisons at evaluation time, matching the - /// existing behavior. + /// multiple items. A value that is not a number the service can store is + /// recorded as refused (see [`number_value`]), so a filter does not read + /// it again for each item. pub fn pre_parse_numerics(&mut self) { for (key, value) in &self.values { - if let AttributeValue::N(n) = value - && let Ok(d) = n.parse::() - { - self.parsed_numerics.insert(key.clone(), d); + if let AttributeValue::N(n) = value { + self.parsed_numerics.insert(key.clone(), number_value(n)); } } } - /// Look up a pre-parsed `BigDecimal` for a numeric placeholder. - /// - /// Returns `None` if the placeholder was not numeric or failed to parse. + /// Look up a numeric placeholder as it was pre-parsed, or `None` when the + /// placeholder is not an `N` in these maps. #[must_use] - pub fn get_parsed_numeric(&self, placeholder: &str) -> Option<&bigdecimal::BigDecimal> { - self.parsed_numerics.get(placeholder) + pub(crate) fn get_parsed_numeric(&self, placeholder: &str) -> Option> { + self.parsed_numerics.get(placeholder).map(|parsed| { + parsed + .as_ref() + .map_or(ParsedNumber::Refused, ParsedNumber::Number) + }) } } +/// A number in an expression as a decimal, or `None` when `n` is not a number +/// the service can store. +/// +/// The decimal is parsed from the number check's normalized text, which is +/// at most 170 characters for every number the service accepts. Parsing the +/// text as written takes time quadratic in its length: a million digits took +/// about a second (bigdecimal 0.4.10, release build), and a request body can +/// carry 16 MiB. `UpdateItem` and `TransactWriteItems` build their +/// `ExpressionMaps` before they check numbers, `Query` and `Scan` do not check +/// them (#544, #562), and a stored item can hold a number an older release +/// stored without the check (#618). +pub(crate) fn number_value(n: &str) -> Option { + crate::validation::number::validate_and_normalize_number(n) + .ok()? + .parse() + .ok() +} + /// Resolve a `#name` reference or return the bare name. /// /// Shared by the condition evaluator and update evaluator for resolving diff --git a/crates/core/src/expression/test_timeout.rs b/crates/core/src/expression/test_timeout.rs new file mode 100644 index 000000000..8f7896662 --- /dev/null +++ b/crates/core/src/expression/test_timeout.rs @@ -0,0 +1,29 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! A time limit for tests of work that could run for minutes when a number +//! is not range checked before arithmetic or a comparison. + +use std::sync::mpsc::{self, RecvTimeoutError}; +use std::time::Duration; + +/// Run `work` on another thread and return its answer, or fail the test after +/// 10 seconds with `what` in the message. A panic in `work` fails the test as +/// that panic. +pub(crate) fn within_10_s(what: &str, work: F) -> T +where + T: Send + 'static, + F: FnOnce() -> T + Send + 'static, +{ + let (tx, rx) = mpsc::channel(); + let worker = std::thread::spawn(move || { + let _ = tx.send(work()); + }); + match rx.recv_timeout(Duration::from_secs(10)) { + Ok(answer) => answer, + Err(RecvTimeoutError::Disconnected) => { + std::panic::resume_unwind(worker.join().expect_err("no answer and no panic")) + } + Err(RecvTimeoutError::Timeout) => panic!("{what} ran for 10 s"), + } +} diff --git a/crates/core/src/expression/update_evaluator.rs b/crates/core/src/expression/update_evaluator.rs index 52f2a149b..309be9ddc 100755 --- a/crates/core/src/expression/update_evaluator.rs +++ b/crates/core/src/expression/update_evaluator.rs @@ -163,27 +163,72 @@ fn evaluate_arithmetic( )); }; - let ld: bigdecimal::BigDecimal = l.parse().map_err(|_| { - DynamoDbError::ValidationException("Invalid numeric value in expression".to_owned()) - })?; - let rd: bigdecimal::BigDecimal = r.parse().map_err(|_| { - DynamoDbError::ValidationException("Invalid numeric value in expression".to_owned()) - })?; - - let result = match op { - ArithOp::Add => ld + rd, - ArithOp::Sub => ld - rd, - }; + let (l, r) = (arithmetic_operand(l)?, arithmetic_operand(r)?); + arithmetic_result(&match op { + ArithOp::Add => l + r, + ArithOp::Sub => l - r, + }) +} - let result_str = result.to_string(); - // Validate the result is within DynamoDB's number range - crate::validation::number::validate_and_normalize_number(&result_str).map_err(|_| { - DynamoDbError::ValidationException( - "Number overflow. Attempting to store a number with magnitude larger than supported range".to_owned(), - ) +/// An operand of `+`, `-` or a number `ADD`, as a decimal, once it is known +/// to be a number the service can store. +/// +/// `bigdecimal` aligns two operands to the larger scale before it adds them, +/// so the cost of a sum grows with the gap between the exponents: +/// `1 + 1E999999999` builds an integer of about 10^9 digits. `UpdateItem` and +/// `TransactWriteItems` check `ExpressionAttributeValues` before an update +/// runs, but an operand can also be read from the stored item, and an item +/// stored by an older release can hold a number that was never checked +/// (#618). A number the service accepts has a magnitude between -129 and 126 +/// and at most 38 digits, which keeps a sum to a few hundred digits. +/// +/// The decimal is parsed from the normalized text, not from `n`. The number +/// check accepts a zero whose exponent fits in 32 bits without a range check +/// and normalizes it to `0`; parsed as written, `1 + 0E-999999999` would +/// scale `1` by 10^999999999. +/// +/// # Errors +/// +/// Returns the number check's `ValidationException` when `n` is out of range +/// or has more than 38 digits. A value that is not a number is refused with +/// the service's text for one, without the value: it can come from the +/// stored item, which the caller may not be allowed to read. +fn arithmetic_operand(n: &str) -> Result { + use crate::validation::number::NumberFault; + let not_a_number = + || DynamoDbError::ValidationException(NumberFault::Malformed.text().to_owned()); + let normalized = crate::validation::number::normalize_number(n).map_err(|fault| { + if fault.is_not_a_number() { + not_a_number() + } else { + fault.error(n) + } })?; + // Normalized text always parses; the error is not reachable. + normalized.parse().map_err(|_| not_a_number()) +} - Ok(AttributeValue::N(result_str)) +/// The result of `+`, `-` or a number `ADD`, spelled as the service stores +/// it: `2.25 - 0.25` is `2` and `1.5 - 1.5` is `0` (measured 2026-11-03, +/// us-east-1, `UpdateItem` and `TransactWriteItems`, each twice). +/// +/// A result outside the range or with more than 38 digits is a +/// `Number overflow`, a result below the range included: the service answers +/// `Number overflow` for `1.1E-130 - 1E-130` and for `1E125 + 1` (measured +/// the same way). +/// +/// # Errors +/// +/// Returns `ValidationException` with the overflow message. +fn arithmetic_result(result: &bigdecimal::BigDecimal) -> Result { + crate::validation::number::validate_and_normalize_number(&result.to_string()) + .map(AttributeValue::N) + .map_err(|_| { + DynamoDbError::ValidationException( + "Number overflow. Attempting to store a number with magnitude larger than supported range" + .to_owned(), + ) + }) } /// Evaluate SET functions: `if_not_exists(path, value)`. @@ -267,21 +312,7 @@ fn apply_add( ) => value, // Number: add to existing (Some(AttributeValue::N(existing_n)), AttributeValue::N(add_n)) => { - let ed: bigdecimal::BigDecimal = existing_n.parse().map_err(|_| { - DynamoDbError::ValidationException("Invalid numeric value in expression".to_owned()) - })?; - let ad: bigdecimal::BigDecimal = add_n.parse().map_err(|_| { - DynamoDbError::ValidationException("Invalid numeric value in expression".to_owned()) - })?; - let result_str = (ed + ad).to_string(); - crate::validation::number::validate_and_normalize_number(&result_str).map_err( - |_| { - DynamoDbError::ValidationException( - "Number overflow. Attempting to store a number with magnitude larger than supported range".to_owned(), - ) - }, - )?; - AttributeValue::N(result_str) + arithmetic_result(&(arithmetic_operand(existing_n)? + arithmetic_operand(add_n)?))? } // Set: union with existing (Some(AttributeValue::SS(existing_set)), AttributeValue::SS(add_set)) => { diff --git a/crates/core/src/expression/update_evaluator_tests.rs b/crates/core/src/expression/update_evaluator_tests.rs index 968e6d356..16434fe3a 100755 --- a/crates/core/src/expression/update_evaluator_tests.rs +++ b/crates/core/src/expression/update_evaluator_tests.rs @@ -660,3 +660,172 @@ fn set_parenthesised_arithmetic() { apply("SET c = (c - :v)", &mut item, HashMap::new(), values).unwrap(); assert_eq!(item.get("c"), Some(&AttributeValue::N("7".into()))); } + +/// Number arithmetic in `SET` and `ADD`: each operand is range checked before +/// the sum is built, and the result is stored as the service spells it. +mod number_arithmetic { + use super::*; + use crate::expression::test_timeout::within_10_s; + + 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"; + const DIGITS: &str = "Attempting to store more than 38 significant digits in a Number"; + + /// Apply `expression` to an item whose `n` holds `stored`, with `:v` set + /// to `value`, and return the answer and the `n` left in the item. The + /// update runs on another thread, so a sum built from an operand that + /// was not range checked fails the test after 10 seconds instead of + /// hanging the suite, and a panic in the update is reported as one. + fn update_n( + expression: &'static str, + stored: &str, + value: &str, + ) -> (Result<(), DynamoDbError>, Option) { + let mut item = BTreeMap::from([("n".to_owned(), AttributeValue::N(stored.to_owned()))]); + let values = HashMap::from([("v".to_owned(), AttributeValue::N(value.to_owned()))]); + within_10_s(&format!("`{expression}` with n = {stored}"), move || { + let result = apply(expression, &mut item, HashMap::new(), values); + (result, item.remove("n")) + }) + } + + fn validation_message(result: Result<(), DynamoDbError>) -> String { + match result { + Err(DynamoDbError::ValidationException(message)) => message, + other => panic!("expected a ValidationException, got {other:?}"), + } + } + + const EXPRESSIONS: [&str; 4] = [ + "SET n = n + :v", + "SET n = :v - n", + "SET n = if_not_exists(n, :v) + :v", + "ADD n :v", + ]; + + /// A stored number outside the range, as an older release could store + /// it, is refused with the number check's message before it is added: + /// `1 + 1E999999999` would otherwise build an integer of about 10^9 + /// digits. The item keeps its value. + #[test] + fn a_stored_operand_out_of_range_is_refused_before_the_sum() { + let digits = "1".repeat(39); + for (stored, expected) in [ + ("1E999999999", OVERFLOW), + ("-1E999999999", OVERFLOW), + ("1E-999999999", UNDERFLOW), + ("1E126", OVERFLOW), + ("1E-131", UNDERFLOW), + (digits.as_str(), DIGITS), + ] { + for expression in EXPRESSIONS { + let (result, n) = update_n(expression, stored, "1"); + assert_eq!( + validation_message(result), + expected, + "`{expression}`, n = {stored}" + ); + assert_eq!( + n, + Some(AttributeValue::N(stored.to_owned())), + "`{expression}`" + ); + } + } + } + + /// A stored value that is not a number is refused with the service's + /// text for one, without the stored value: the caller may not be allowed + /// to read the item. + #[test] + fn a_stored_operand_that_is_not_a_number_is_refused_without_its_text() { + for stored in ["abc", " 5", "1E9223372036854775808", ""] { + for expression in EXPRESSIONS { + let (result, n) = update_n(expression, stored, "1"); + assert_eq!( + validation_message(result), + "The parameter cannot be converted to a numeric value", + "`{expression}`, n = {stored:?}" + ); + assert_eq!( + n, + Some(AttributeValue::N(stored.to_owned())), + "`{expression}`" + ); + } + } + } + + /// A zero whose exponent fits in 32 bits is read as `0`, as the number + /// check reads it, so `1 + 0E-999999999` is `1`. Read as written, the + /// zero's scale of 999999999 would scale the `1` to match it. + #[test] + fn a_stored_zero_adds_as_zero_whatever_its_exponent() { + for stored in ["0E-999999999", "0E999999999", "-0.0E-999999999"] { + for expression in EXPRESSIONS { + let (result, n) = update_n(expression, stored, "1"); + result.unwrap_or_else(|e| panic!("`{expression}`, n = {stored}: {e:?}")); + assert_eq!(n, Some(AttributeValue::N("1".to_owned())), "`{expression}`"); + } + } + } + + /// The result is stored as the service spells it (measured 2026-11-03, + /// us-east-1): no trailing zeros after the point, no exponent, and `0` + /// for a zero. + #[test] + fn the_result_is_stored_as_the_service_spells_it() { + let min = format!("0.{}1", "0".repeat(129)); + let twice_max_magnitude = format!("2{}", "0".repeat(125)); + for (stored, expression, value, expected) in [ + ("2.25", "SET n = n - :v", "0.25", "2"), + ("2.25", "ADD n :v", "-0.25", "2"), + ("1.5", "SET n = n - :v", "1.5", "0"), + ("1.5", "SET n = n + :v", "1.5", "3"), + ("1.5", "ADD n :v", "1.5", "3"), + ("2E-130", "SET n = n - :v", "1E-130", min.as_str()), + ( + "1E125", + "SET n = n + :v", + "1E125", + twice_max_magnitude.as_str(), + ), + ] { + let (result, n) = update_n(expression, stored, value); + result.unwrap_or_else(|e| panic!("`{expression}`, n = {stored}: {e:?}")); + assert_eq!( + n, + Some(AttributeValue::N(expected.to_owned())), + "`{expression}`, n = {stored}" + ); + } + } + + /// A result outside the range or with more than 38 digits is a + /// `Number overflow`, a result below the range included (measured + /// 2026-11-03, us-east-1). + #[test] + fn a_result_out_of_range_is_an_overflow() { + for (stored, expression, value) in [ + ("5E125", "SET n = n + :v", "5E125"), + ("5E125", "ADD n :v", "5E125"), + ("1E125", "SET n = n + :v", "1"), + ("1.1E-130", "SET n = n - :v", "1E-130"), + ("1.1E-130", "ADD n :v", "-1E-130"), + ("-1.1E-130", "SET n = n + :v", "1E-130"), + ] { + let (result, n) = update_n(expression, stored, value); + assert_eq!( + validation_message(result), + OVERFLOW, + "`{expression}`, n = {stored}" + ); + assert_eq!( + n, + Some(AttributeValue::N(stored.to_owned())), + "`{expression}`" + ); + } + } +} diff --git a/crates/core/src/validation/mod.rs b/crates/core/src/validation/mod.rs index 09d3f13b4..9c1bcbb21 100755 --- a/crates/core/src/validation/mod.rs +++ b/crates/core/src/validation/mod.rs @@ -1194,12 +1194,10 @@ pub fn validate_update_item( // Validate number values supplied via ExpressionAttributeValues. Unlike a // PutItem item (validated by validate_item_numbers), these are checked here // because the AttributeValue deserializer intentionally stores malformed - // numbers raw and defers rejection to the validation layer. Real DynamoDB - // wraps the error as: - // "ExpressionAttributeValues contains invalid value: for key " - if let Some(values) = &input.expression_attribute_values { - validate_expression_attribute_value_numbers(values)?; - } + // numbers raw and defers rejection to the validation layer. The service + // answers `1 validation error detected: ` and the number message, with no + // ExpressionAttributeValues wrapper (measured 2026-11-11). + number::operations::update_item_values(input)?; if let Some(updates) = &input.attribute_updates { validate_attribute_values_nesting_depth(updates.values().filter_map(|u| u.value.as_ref()))?; @@ -1215,29 +1213,6 @@ pub fn validate_update_item( Ok(()) } -/// Validate number values provided via `ExpressionAttributeValues`, matching the -/// service's wrapped message: `ExpressionAttributeValues contains invalid value: -/// for key `. Keys are visited in sorted order so the -/// reported placeholder is deterministic when several values are invalid. -fn validate_expression_attribute_value_numbers( - values: &std::collections::HashMap, -) -> Result<(), DynamoDbError> { - let mut keys: Vec<&String> = values.keys().collect(); - keys.sort(); - for key in keys { - match validate_attribute_number(&values[key]) { - Ok(()) => {} - Err(DynamoDbError::ValidationException(inner)) => { - return Err(DynamoDbError::ValidationException(format!( - "ExpressionAttributeValues contains invalid value: {inner} for key {key}" - ))); - } - Err(other) => return Err(other), - } - } - Ok(()) -} - /// The request form an attribute name arrived in, which selects the wording /// of the size refusal (REQ-LIM-004). /// @@ -4076,7 +4051,9 @@ mod tests { #[test] fn update_item_rejects_malformed_number_in_eav() { // The AttributeValue deserializer stores malformed numbers raw and - // defers rejection here. Matches real DynamoDB's wrapped message. + // defers rejection here. The service answers the prefix and the + // number message, with no ExpressionAttributeValues wrapper (measured + // 2026-11-11, `12e` and `abc`). let limits = LimitsConfig::default(); let key_schema = vec![make_ks("pk", KeyType::Hash)]; let attr_defs = vec![make_ad("pk", ScalarAttributeType::S)]; @@ -4084,13 +4061,14 @@ mod tests { let err = validate_update_item(&input, &limits, &key_schema, &attr_defs).unwrap_err(); assert_eq!( err.to_string(), - "ExpressionAttributeValues contains invalid value: \ - The parameter cannot be converted to a numeric value: 12e for key :v" + "1 validation error detected: \ + The parameter cannot be converted to a numeric value: 12e" ); } #[test] fn update_item_rejects_number_overflow_in_eav() { + // Measured 2026-11-11 with `1E126` and `1E2147483648`. let limits = LimitsConfig::default(); let key_schema = vec![make_ks("pk", KeyType::Hash)]; let attr_defs = vec![make_ad("pk", ScalarAttributeType::S)]; @@ -4099,14 +4077,15 @@ mod tests { let err = validate_update_item(&input, &limits, &key_schema, &attr_defs).unwrap_err(); assert_eq!( err.to_string(), - "ExpressionAttributeValues contains invalid value: \ + "1 validation error detected: \ Number overflow. Attempting to store a number with magnitude larger than \ - supported range for key :v" + supported range" ); } #[test] fn update_item_rejects_invalid_number_set_member_in_eav() { + // Measured 2026-11-11 with `NS [1, abc]`. let limits = LimitsConfig::default(); let key_schema = vec![make_ks("pk", KeyType::Hash)]; let attr_defs = vec![make_ad("pk", ScalarAttributeType::S)]; @@ -4116,8 +4095,8 @@ mod tests { let err = validate_update_item(&input, &limits, &key_schema, &attr_defs).unwrap_err(); assert_eq!( err.to_string(), - "ExpressionAttributeValues contains invalid value: \ - The parameter cannot be converted to a numeric value: abc for key :v" + "1 validation error detected: \ + The parameter cannot be converted to a numeric value: abc" ); } diff --git a/crates/core/src/validation/number.rs b/crates/core/src/validation/number.rs index 4680005fd..b50094252 100644 --- a/crates/core/src/validation/number.rs +++ b/crates/core/src/validation/number.rs @@ -3,18 +3,107 @@ use crate::error::DynamoDbError; +pub mod operations; + const MAX_SIGNIFICANT_DIGITS: usize = 38; const MAX_EXPONENT: i32 = 125; const MIN_EXPONENT: i32 = -130; +/// The largest and smallest magnitude, the precision less the scale, that +/// `DynamoDB` accepts: one more than the largest and smallest order of +/// magnitude, so 126 for 9.99E125 and -129 for 1E-130. +pub(super) const MAX_MAGNITUDE: i64 = MAX_EXPONENT as i64 + 1; +pub(super) const MIN_MAGNITUDE: i64 = MIN_EXPONENT as i64 + 1; + +/// The service's text for a value that is not a number. A malformed value +/// is named after a colon; an empty one is not. +const NOT_A_NUMBER: &str = "The parameter cannot be converted to a numeric value"; + +/// Why a string is not a number `DynamoDB` can store, as +/// [`normalize_number`] reports it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NumberFault { + /// An empty string, e.g. `{"N":""}`. + Empty, + /// A string that is not a number. + Malformed, + /// A magnitude above the supported range. + Overflow, + /// A magnitude below the supported range. + Underflow, + /// More than 38 significant digits. + TooManyDigits, +} + +impl NumberFault { + /// Whether the string is not a number at all, where the other faults are + /// numbers out of range or with too many digits. + #[must_use] + pub fn is_not_a_number(self) -> bool { + matches!(self, Self::Empty | Self::Malformed) + } + + /// The service's message for this fault, without the offending value. + /// `DynamoDB` names the value only for a malformed one; see + /// [`NumberFault::message`]. + #[must_use] + pub fn text(self) -> &'static str { + match self { + Self::Empty | Self::Malformed => NOT_A_NUMBER, + Self::Overflow => { + "Number overflow. Attempting to store a number with magnitude larger than supported range" + } + Self::Underflow => { + "Number underflow. Attempting to store a number with magnitude smaller than supported range" + } + Self::TooManyDigits => { + "Attempting to store more than 38 significant digits in a Number" + } + } + } + + /// The service's message for this fault in the number `s`: a malformed + /// value is appended after a colon, as `DynamoDB` reports it, and the + /// other faults carry no value. + #[must_use] + pub fn message(self, s: &str) -> String { + match self { + Self::Malformed => format!("{NOT_A_NUMBER}: {s}"), + _ => self.text().to_owned(), + } + } + + /// The fault in the number `s` as the `ValidationException` the service + /// answers. + #[must_use] + pub fn error(self, s: &str) -> DynamoDbError { + DynamoDbError::ValidationException(self.message(s)) + } +} + /// Validate and normalize a `DynamoDB` number string. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's message +/// for the number's [`NumberFault`]. pub fn validate_and_normalize_number(s: &str) -> Result { + normalize_number(s).map_err(|fault| fault.error(s)) +} + +/// Validate and normalize a `DynamoDB` number string, with the fault as a +/// [`NumberFault`] for a caller that acts on its kind. +/// +/// # Errors +/// +/// Returns the [`NumberFault`] of the first rule `s` breaks. +pub fn normalize_number(s: &str) -> Result { // DynamoDB does not trim: leading or trailing whitespace makes the number // malformed rather than acceptable (e.g. " 5" and "5 " are rejected). A // genuinely empty string is reported with a distinct message from a // malformed-but-non-empty one. if s.is_empty() { - return Err(empty_numeric_err()); + return Err(NumberFault::Empty); } let (negative, rest) = match s.strip_prefix('-') { @@ -26,19 +115,19 @@ pub fn validate_and_normalize_number(s: &str) -> Result { // 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(|_| NumberFault::Malformed)?), None => (rest, 0), }; if mantissa_str.is_empty() || !mantissa_str.chars().all(|c| c.is_ascii_digit() || c == '.') { - return Err(numeric_value_err(s)); + return Err(NumberFault::Malformed); } // The mantissa must contain at least one digit. A bare "." (also "+.", // ".e5") passes the character check above since '.' is allowed, but has no // digits and is not a valid number. if !mantissa_str.bytes().any(|b| b.is_ascii_digit()) { - return Err(numeric_value_err(s)); + return Err(NumberFault::Malformed); } // Split into integer and fractional parts @@ -50,7 +139,7 @@ pub fn validate_and_normalize_number(s: &str) -> Result { if !int_part.chars().all(|c| c.is_ascii_digit()) || !frac_part.chars().all(|c| c.is_ascii_digit()) { - return Err(numeric_value_err(s)); + return Err(NumberFault::Malformed); } // Combine into a single digit string and track where the decimal point is. @@ -70,11 +159,11 @@ pub fn validate_and_normalize_number(s: &str) -> Result { 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. + // whose exponent does not fit in an i32 is checked here. A refused + // zero keeps its raw spelling in `AttributeValue`, as every refused + // number does, and Query and Scan (#544, #562) do not validate + // numbers yet: refusing `0E126` here would make them look it up as a + // key of its own instead of `0` (#618). 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 @@ -82,7 +171,7 @@ pub fn validate_and_normalize_number(s: &str) -> Result { // is an underflow, while `0E9223372036854775806` and // `0.0E9223372036854775807` are overflows. if normalized_exponent_exceeds_i64(point_offset) { - return Err(underflow_err()); + return Err(NumberFault::Underflow); } check_magnitude(point_offset)?; } @@ -116,7 +205,7 @@ pub fn validate_and_normalize_number(s: &str) -> Result { // 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()); + return Err(NumberFault::Overflow); } // The small side mirrors it: an underflow is reported before the digits @@ -132,18 +221,18 @@ pub fn validate_and_normalize_number(s: &str) -> Result { // `E-9223372036854775808` (i64::MIN + 40). The answers are the // same with 39 digits and with a minus sign. if normalized_exponent_is_below_i64(magnitude_exp) { - return Err(underflow_err()); + return Err(NumberFault::Underflow); } if sig_trimmed.len() > MAX_SIGNIFICANT_DIGITS { - return Err(too_many_digits_err()); + return Err(NumberFault::TooManyDigits); } 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())?; + let exp = i32::try_from(exp).map_err(|_| NumberFault::Overflow)?; // Format the normalized number // sig_trimmed represents an integer, and we multiply by 10^exp @@ -186,12 +275,12 @@ fn normalized_exponent_is_below_i64(magnitude_exp: i128) -> bool { } /// Refuse an order of magnitude outside `DynamoDB`'s supported range. -fn check_magnitude(magnitude_exp: i128) -> Result<(), DynamoDbError> { +fn check_magnitude(magnitude_exp: i128) -> Result<(), NumberFault> { if magnitude_exp > i128::from(MAX_EXPONENT) { - return Err(overflow_err()); + return Err(NumberFault::Overflow); } if magnitude_exp < i128::from(MIN_EXPONENT) { - return Err(underflow_err()); + return Err(NumberFault::Underflow); } Ok(()) } @@ -231,52 +320,6 @@ fn format_plain(negative: bool, digits: &str, exp: i32) -> String { result } -/// Empty number string, e.g. `{"N":""}`. Real `DynamoDB` reports the generic -/// numeric-conversion failure with no offending value appended (distinct from -/// the malformed-but-non-empty message, which appends the input). -fn empty_numeric_err() -> DynamoDbError { - DynamoDbError::ValidationException( - "The parameter cannot be converted to a numeric value".to_owned(), - ) -} - -/// More than 38 significant digits. Real `DynamoDB`: -/// "Attempting to store more than 38 significant digits in a Number". -fn too_many_digits_err() -> DynamoDbError { - DynamoDbError::ValidationException( - "Attempting to store more than 38 significant digits in a Number".to_owned(), - ) -} - -/// Magnitude above the supported exponent ceiling. Real `DynamoDB`: -/// "Number overflow. Attempting to store a number with magnitude larger than -/// supported range". -fn overflow_err() -> DynamoDbError { - DynamoDbError::ValidationException( - "Number overflow. Attempting to store a number with magnitude larger than supported range" - .to_owned(), - ) -} - -/// Magnitude below the supported exponent floor. Real `DynamoDB`: -/// "Number underflow. Attempting to store a number with magnitude smaller than -/// supported range". -fn underflow_err() -> DynamoDbError { - DynamoDbError::ValidationException( - "Number underflow. Attempting to store a number with magnitude smaller than supported range" - .to_owned(), - ) -} - -/// Error for a non-empty but malformed number string. Mirrors real DynamoDB, -/// which reports `The parameter cannot be converted to a numeric value: ` -/// for syntactically invalid numbers, distinct from the empty-value message. -fn numeric_value_err(s: &str) -> DynamoDbError { - DynamoDbError::ValidationException(format!( - "The parameter cannot be converted to a numeric value: {s}" - )) -} - #[cfg(test)] mod tests { use super::*; @@ -446,6 +489,54 @@ mod tests { ); } + #[test] + fn normalize_number_names_the_fault() { + // The kind of fault, which callers act on, and the message each kind + // gives, which is the text `validate_and_normalize_number` reports. + for (input, fault, message) in [ + ( + "", + NumberFault::Empty, + "The parameter cannot be converted to a numeric value", + ), + ( + "abc", + NumberFault::Malformed, + "The parameter cannot be converted to a numeric value: abc", + ), + ( + "1E126", + NumberFault::Overflow, + "Number overflow. Attempting to store a number with magnitude larger than supported range", + ), + ( + "1E-131", + NumberFault::Underflow, + "Number underflow. Attempting to store a number with magnitude smaller than supported range", + ), + ( + "1234567890123456789012345678901234567890", + NumberFault::TooManyDigits, + "Attempting to store more than 38 significant digits in a Number", + ), + ] { + assert_eq!(normalize_number(input), Err(fault), "{input:?}"); + assert_eq!(fault.message(input), message, "{input:?}"); + assert_eq!( + validate_and_normalize_number(input) + .unwrap_err() + .to_string(), + message, + "{input:?}" + ); + assert_eq!( + fault.is_not_a_number(), + message.starts_with("The parameter") + ); + } + assert_eq!(normalize_number("1.50E2").unwrap(), "150"); + } + // 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. @@ -548,18 +639,27 @@ mod tests { for n in ["0E2147483648", "0E9223372036854775806"] { assert_refused(n, OVERFLOW); } - for n in ["0E-9223372036854775807", "0E-9223372036854775808"] { + for n in [ + "0E-9223372036854775807", + "0E-9223372036854775808", + // Fraction digits lower the exponent further below the 64-bit + // minimum, and the zero stays an underflow (sent to GetItem on + // 2026-10-27, twice each). + "0.00E-9223372036854775808", + "0.000E-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. + // DynamoDB refuses each of these zeros: `0E126` and the next four are a + // `Number overflow`, `0.0E-130` and the rest a `Number underflow`. They + // stay accepted as 0 until Query and Scan (#544, #562) validate numbers, + // which #618 still tracks: a zero refused by this function keeps its raw + // spelling, and those operations would then look up `0E126` as a key of + // its own. Move these rows to the refused lists above when they do. #[test] - fn zero_with_i32_exponent_stays_accepted_until_618() { + fn zero_with_i32_exponent_stays_accepted_until_query_and_scan_validate() { for n in [ "0E126", "00E126", diff --git a/crates/core/src/validation/number/operations.rs b/crates/core/src/validation/number/operations.rs new file mode 100644 index 000000000..35364e22a --- /dev/null +++ b/crates/core/src/validation/number/operations.rs @@ -0,0 +1,533 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Number checks for the keys, items and expression attribute values that the +//! `PutItem` item check and the `UpdateItem` value checks do not cover, with +//! the text each operation answers. +//! +//! Every operation here reads a number as [`validate_and_normalize_number`] +//! does, with a 64-bit exponent. Measured in us-east-1, every request twice +//! with the same answer both times: `GetItem`, `BatchGetItem`, `UpdateItem` +//! and `DeleteItem` on 2026-10-27, and `PutItem`, `BatchWriteItem`, +//! `TransactWriteItems` and `TransactGetItems` on 2026-11-11, where +//! `10E2147483647` and `1E2147483648` are a `Number overflow` on all four +//! paths. An empty value answers `The parameter cannot be converted to a +//! numeric value: `, with nothing after the colon, on every path here except +//! the keys and items of `BatchWriteItem`, which answer the message without +//! the colon (measured 2026-11-11 on the keys of `GetItem`, `BatchGetItem`, +//! `DeleteItem`, `UpdateItem`, `BatchWriteItem`, `TransactGetItems` and +//! `TransactWriteItems`, the items of `BatchWriteItem` and +//! `TransactWriteItems`, `Expected` values and `ExpressionAttributeValues`). +//! On 2026-10-27 the keys and items of `TransactGetItems` and +//! `TransactWriteItems` answered without the colon too. +//! +//! `ImportTable` reads the numbers of a row with a 32-bit exponent and scale +//! (measured 2026-11-03 and 2026-11-11; see [`import_row`] and +//! [`validate_number_i32_scale`]). +//! +//! `PutItem`, `UpdateItem` and `DeleteItem` start the message with +//! `1 validation error detected: `, and the other operations do not. A +//! number in `ExpressionAttributeValues`, or inside an `M`, `L` or `NS` value +//! there, answers the number message in the same form on each operation, with +//! no wrapper (measured 2026-11-11; on 2026-10-27 `TransactWriteItems` +//! answered `ExpressionAttributeValues contains invalid value: for +//! key `). `TransactWriteItems` checks those values for every +//! `Update` and `ConditionCheck`, and for a `Put` or `Delete` only when the +//! action has a `ConditionExpression`, empty or not. +//! +//! When several placeholders in `ExpressionAttributeValues` are malformed, the +//! service named a different one on two sends of the same request, so these +//! checks visit placeholders in name order. The attributes of a key or an item +//! are visited in the order of the map. +//! +//! [`validate_and_normalize_number`]: super::validate_and_normalize_number + +use std::collections::HashMap; + +use super::{MAX_MAGNITUDE, MAX_SIGNIFICANT_DIGITS, MIN_MAGNITUDE, NumberFault, normalize_number}; +use crate::error::DynamoDbError; +use crate::types::{ + AttributeValue, BatchWriteItemInput, DeleteItemInput, ExpectedAttributeValue, GetItemInput, + Item, PutItemInput, TransactGetItem, TransactWriteItem, UpdateItemInput, +}; + +/// The prefix `PutItem`, `UpdateItem` and `DeleteItem` put on the message. +const SINGLE_WRITE_PREFIX: &str = "1 validation error detected: "; + +/// The text an operation gives for an empty number. +#[derive(Clone, Copy)] +enum EmptyText { + /// The message and a colon with nothing after it: keys of `GetItem`, + /// `BatchGetItem`, `DeleteItem` and `UpdateItem`, keys and items of + /// `TransactGetItems` and `TransactWriteItems`, `Expected` values and + /// `ExpressionAttributeValues` (measured 2026-11-11; `TransactGetItems` + /// and `TransactWriteItems` keys and items had no colon on 2026-10-27). + WithColon, + /// The message without the colon: keys and items of `BatchWriteItem`, an + /// `NS` member included (measured 2026-10-27 and 2026-11-11). + WithoutColon, +} + +/// Refuse a `GetItem` key holding a number the service cannot read. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn get_item(input: &GetItemInput) -> Result<(), DynamoDbError> { + item_numbers(EmptyText::WithColon, &input.key).map_err(bare) +} + +/// Refuse one `BatchGetItem` table's keys when one holds a number the service +/// cannot read. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn batch_get_keys(keys: &[Item]) -> Result<(), DynamoDbError> { + keys.iter() + .try_for_each(|key| item_numbers(EmptyText::WithColon, key)) + .map_err(bare) +} + +/// Refuse a `DeleteItem` key holding a number the service cannot read. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn delete_item_key(input: &DeleteItemInput) -> Result<(), DynamoDbError> { + item_numbers(EmptyText::WithColon, &input.key).map_err(single_write) +} + +/// Refuse `DeleteItem` `ExpressionAttributeValues` holding a value nested too +/// deep or a number the service cannot read, whether or not an expression +/// reads the value. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn delete_item_values(input: &DeleteItemInput) -> Result<(), DynamoDbError> { + values_nesting_then_numbers(input.expression_attribute_values.as_ref()).map_err(single_write) +} + +/// Refuse an `UpdateItem` key holding a number the service cannot read. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn update_item_key(input: &UpdateItemInput) -> Result<(), DynamoDbError> { + item_numbers(EmptyText::WithColon, &input.key).map_err(single_write) +} + +/// Refuse `PutItem` `ExpressionAttributeValues` holding a value nested too +/// deep or a number the service cannot read, whether or not an expression +/// reads the value. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn put_item_values(input: &PutItemInput) -> Result<(), DynamoDbError> { + values_nesting_then_numbers(input.expression_attribute_values.as_ref()).map_err(single_write) +} + +/// Refuse a legacy `Expected` map of `PutItem`, `DeleteItem` or `UpdateItem` +/// holding a number the service cannot read in a `Value` or an +/// `AttributeValueList` member. +/// +/// Measured 2026-11-11 (us-east-1, each request twice) on items that do not +/// exist: `abc` and `1E2147483648` in `Expected {a: {Value: N}}` answer a +/// `ValidationException` on `PutItem`, `DeleteItem` and `UpdateItem`, and in +/// `Expected {a: {ComparisonOperator: EQ, AttributeValueList: [N]}}` on +/// `PutItem`, with the prefix and the number message. An empty `N` in the +/// same places answers the prefix and `The parameter cannot be converted to a +/// numeric value: `, with nothing after the colon. `1E125` in the same +/// `PutItem` request answers `ConditionalCheckFailedException`, so the number +/// is read ahead of the condition. Attributes are visited in name order, and +/// a `Value` ahead of the list. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn expected_values( + expected: Option<&HashMap>, +) -> Result<(), DynamoDbError> { + let Some(expected) = expected else { + return Ok(()); + }; + let mut names: Vec<&String> = expected.keys().collect(); + names.sort(); + for name in names { + let entry = &expected[name]; + entry + .value + .iter() + .chain(entry.attribute_value_list.iter().flatten()) + .try_for_each(|value| attribute_numbers(EmptyText::WithColon, value)) + .map_err(single_write)?; + } + Ok(()) +} + +/// Refuse a `PutItem` item holding a number the service cannot read or a +/// value nested too deep, ahead of the mixed-parameter check, the table lookup +/// and the key schema checks (measured 2026-10-27). +/// +/// Numbers come before nesting depth, the order `validate_put_item` checks +/// them in and so the order `PutItem` answered before this early check. The +/// service reports whichever of the two comes first in the request body, +/// which `Item` does not keep, on the two rows measured that answered the +/// same on both sends; two other rows answered nesting on one send and the +/// number on the other. +/// +/// The message carries no prefix, as `PutItem`'s item errors already ship +/// without one. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the item's text. +pub fn put_item_item(input: &PutItemInput) -> Result<(), DynamoDbError> { + crate::validation::validate_item_numbers(&input.item)?; + crate::validation::validate_item_nesting_depth(&input.item) +} + +/// Refuse a `TransactGetItems` key holding a number the service cannot read, +/// with no prefix. An empty `N` answers with the colon (measured +/// 2026-11-11). +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn transact_get_item(item: &TransactGetItem) -> Result<(), DynamoDbError> { + item_numbers(EmptyText::WithColon, &item.get.key).map_err(bare) +} + +/// Refuse a `BatchWriteItem` request when a put item or a delete key holds a +/// value nested too deep or a number the service cannot read. +/// +/// Within one write request the service reports a value nested too deep ahead +/// of a malformed number, and across requests the earlier request's fault +/// (measured 2026-10-27). Tables are visited in name order so that a request +/// answers the same way each time `RequestItems`, a map, is read. The +/// service's order for two tables was measured as neither body order nor +/// name order: with two tables each holding a different malformed value, it +/// named the same table's value for both body orders (2026-10-27, four sends +/// each). +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn batch_write_item(input: &BatchWriteItemInput) -> Result<(), DynamoDbError> { + let mut table_names: Vec<&String> = input.request_items.keys().collect(); + table_names.sort(); + for table_name in table_names { + for request in &input.request_items[table_name] { + if let Some(put) = &request.put_request { + crate::validation::validate_item_nesting_depth(&put.item)?; + item_numbers(EmptyText::WithoutColon, &put.item).map_err(bare)?; + } + if let Some(delete) = &request.delete_request { + item_numbers(EmptyText::WithoutColon, &delete.key).map_err(bare)?; + } + } + } + Ok(()) +} + +/// Refuse one `TransactWriteItems` action holding a number the service cannot +/// read: its `ExpressionAttributeValues` first, then its key or item. Both +/// answer the number message with no prefix and no wrapper, and an empty `N` +/// with the colon (measured 2026-11-11 on `Update`, `Put`, `Delete` and +/// `ConditionCheck` values, on a `Delete` key and on a `Put` item). +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn transact_write_item(item: &TransactWriteItem) -> Result<(), DynamoDbError> { + let (values, key_or_item) = if let Some(put) = &item.put { + let values = put.expression_attribute_values.as_ref(); + ( + values.filter(|_| put.condition_expression.is_some()), + &put.item, + ) + } else if let Some(delete) = &item.delete { + let values = delete.expression_attribute_values.as_ref(); + ( + values.filter(|_| delete.condition_expression.is_some()), + &delete.key, + ) + } else if let Some(update) = &item.update { + (update.expression_attribute_values.as_ref(), &update.key) + } else if let Some(check) = &item.condition_check { + (check.expression_attribute_values.as_ref(), &check.key) + } else { + return Ok(()); + }; + value_numbers(EmptyText::WithColon, values).map_err(bare)?; + item_numbers(EmptyText::WithColon, key_or_item).map_err(bare) +} + +/// Refuse `UpdateItem` `ExpressionAttributeValues` holding a number the +/// service cannot read, with the prefix and the number message. Placeholders +/// are visited in name order. +/// +/// Measured 2026-11-11 (us-east-1, each request twice) with `SET n = :v`: +/// `abc` and `12e` answer `1 validation error detected: The parameter cannot +/// be converted to a numeric value: abc` (`12e` likewise), an empty `N` the +/// same message with nothing after the colon, and `1E126` and +/// `1E2147483648` the prefix and `Number overflow`. A number inside an `M`, +/// `L` or `NS` value answers the inner number's message in the same form. No +/// answer names the placeholder. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn update_item_values(input: &UpdateItemInput) -> Result<(), DynamoDbError> { + value_numbers( + EmptyText::WithColon, + input.expression_attribute_values.as_ref(), + ) + .map_err(single_write) +} + +/// Why `ImportTable` refuses a row holding a number the service cannot read. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ImportRowFault { + /// A value that is not a number. The service refuses the row and does not + /// process the rows after it ("Expected a valid numeric. Remainder of the + /// file will not be processed."). + Malformed, + /// A number outside the supported range or with more than 38 digits, with + /// its fault. The service refuses the row and imports the rows after it. + OutOfRange(NumberFault), +} + +/// Refuse an `ImportTable` row holding a number the service cannot read. +/// +/// Measured 2026-11-03 (us-east-1), one import from S3 per value, each file a +/// row with a valid number, the row under test, and another valid row, and +/// measured again on 2026-11-11 for the exponents at the 32-bit and 64-bit +/// limits: +/// +/// - The service reads a row's numbers with a 32-bit exponent and scale; see +/// [`validate_number_i32_scale`]. `BatchWriteItem` read them that way too +/// until November, and on 2026-11-11 read them as `PutItem` does while +/// `ImportTable` still read 32 bits. `abc`, ` 5`, an empty value, `12e`, +/// `1E2147483648`, `1E-2147483648` and `1E9223372036854775808` are not +/// numbers, and `1E2147483647` and `10E2147483647` are a `Number +/// underflow` (the last three of the not-a-number values and both +/// underflows measured on 2026-11-11, one file each). +/// - A row holding a value that is not a number stops the import of the +/// file: the row and the rows after it are not imported, and the row is +/// the last one counted as processed. This holds whatever else the row +/// holds: a number out of range ahead of it in the row, or no key. +/// - A row holding a number out of range, or with more than 38 digits, is +/// not imported and the rows after it are. +/// +/// Either way the row counts as one error, and numbers in an `NS`, `L` or `M` +/// count as the row's numbers. A row with no key and the number `1E126` was +/// logged as `Number overflow`, and a row whose key has the wrong type and +/// the number `1E126` as the type mismatch; the caller checks numbers before +/// keys, so only the second row's log line differs. +/// +/// Two values are read differently from the service: +/// +/// - A zero whose exponent fits in 32 bits reaches this check as `0` (see +/// [`validate_number_i32_scale`]), so a row holding `0E126`, which the +/// service refuses as `Number overflow`, is imported with `0`. +/// - The service reads a Unicode decimal digit as the digit (a row holding +/// `\u{ff15}` was imported with `5`). This check does not, as no other +/// number check does, so such a row ends the import of the file. +/// +/// # Errors +/// +/// Returns the row's [`ImportRowFault`]: `Malformed` when any value is not a +/// number, and otherwise the first number out of range, in the order of the +/// item's map. +pub fn import_row(item: &Item) -> Result<(), ImportRowFault> { + let mut numbers = Vec::new(); + for value in item.values() { + collect_numbers(value, &mut numbers); + } + let mut out_of_range = None; + for n in numbers { + match validate_number_i32_scale(n) { + Ok(()) => {} + Err(fault) if fault.is_not_a_number() => return Err(ImportRowFault::Malformed), + Err(fault) => { + out_of_range.get_or_insert(fault); + } + } + } + out_of_range.map_or(Ok(()), |fault| Err(ImportRowFault::OutOfRange(fault))) +} + +/// Every number in `value`: an `N`, the members of an `NS`, and the numbers +/// nested in an `L` or `M`. +fn collect_numbers<'a>(value: &'a AttributeValue, numbers: &mut Vec<&'a str>) { + match value { + AttributeValue::N(n) => numbers.push(n), + AttributeValue::NS(set) => numbers.extend(set.iter().map(String::as_str)), + AttributeValue::L(list) => list.iter().for_each(|v| collect_numbers(v, numbers)), + AttributeValue::M(map) => map.values().for_each(|v| collect_numbers(v, numbers)), + _ => {} + } +} + +/// The first fault in an `ExpressionAttributeValues` map: a value nested too +/// deep, then a number the service cannot read. That is the order `PutItem` +/// and `DeleteItem` answered before these checks; the service answered the +/// nesting on one send and the number on the other (measured 2026-10-27). +fn values_nesting_then_numbers( + values: Option<&HashMap>, +) -> Result<(), String> { + let Some(values) = values else { + return Ok(()); + }; + crate::validation::validate_attribute_values_nesting_depth(values.values()).map_err(message)?; + value_numbers(EmptyText::WithColon, Some(values)) +} + +/// The first number in `item` the service cannot read, as its message. +fn item_numbers(empty: EmptyText, item: &Item) -> Result<(), String> { + item.values() + .try_for_each(|value| attribute_numbers(empty, value)) +} + +/// The first number in `values` the service cannot read, as its message. +/// Placeholders are visited in name order. +fn value_numbers( + empty: EmptyText, + values: Option<&HashMap>, +) -> Result<(), String> { + let Some(values) = values else { + return Ok(()); + }; + let mut keys: Vec<&String> = values.keys().collect(); + keys.sort(); + keys.into_iter() + .try_for_each(|key| attribute_numbers(empty, &values[key])) +} + +/// The first number inside `value` (an `N`, an `NS` member, or a value nested +/// in an `L` or `M`) the service cannot read, as its message. +fn attribute_numbers(empty: EmptyText, value: &AttributeValue) -> Result<(), String> { + match value { + AttributeValue::N(n) => number_text(empty, n), + AttributeValue::NS(set) => set.iter().try_for_each(|n| number_text(empty, n)), + AttributeValue::L(list) => list.iter().try_for_each(|v| attribute_numbers(empty, v)), + AttributeValue::M(map) => map.values().try_for_each(|v| attribute_numbers(empty, v)), + _ => Ok(()), + } +} + +/// One number's text, as the message the service gives when it cannot read +/// it, with the operation's text for an empty value. +fn number_text(empty: EmptyText, n: &str) -> Result<(), String> { + match empty { + EmptyText::WithColon if n.is_empty() => Err(format!("{}: ", NumberFault::Empty.text())), + // `NumberFault::message` gives an empty value without the colon. + _ => normalize_number(n) + .map(drop) + .map_err(|fault| fault.message(n)), + } +} + +/// Check `s` as the service reads a number in an `ImportTable` row, and +/// return the message for the first rule it breaks. These rules were measured +/// on `BatchWriteItem` on 2026-10-27 (us-east-1, every value twice), which +/// read numbers this way then, and the `ImportTable` rows measured on +/// 2026-11-03 follow them (see [`import_row`]). On 2026-11-11 `BatchWriteItem` +/// read numbers as `PutItem` does, and `ImportTable` still read them this way: +/// `1E2147483648`, `1E-2147483648` and `1E9223372036854775808` were not +/// numbers, and `1E2147483647` and `10E2147483647` a `Number underflow`. +/// +/// 1. `s` is an optional sign, digits with at most one point and at least one +/// digit, and an optional exponent: `e` or `E`, an optional sign and +/// digits. The exponent must fit in a 32-bit integer, and so must the +/// scale, the number of fraction digits less the exponent. Otherwise the +/// value cannot be converted, as `1E2147483648`, `1E-2147483648` and +/// `1.5E-2147483648` cannot. +/// 2. The magnitude is the precision less the scale, where the precision is +/// the number of digits without the leading zeros (1 for a zero), computed +/// in 32-bit arithmetic that wraps. Above 126 is a `Number overflow` +/// (`1E126`) and below -129 a `Number underflow` (`1E-131`). The magnitude +/// of `1E2147483647` and `10E2147483647` wraps past the 32-bit maximum, so +/// each is an underflow. +/// 3. More than 38 digits, not counting leading and trailing zeros, is refused +/// after the magnitude: 39 ones answer the 38-digit message, and 39 ones +/// with the exponent 200 answer `Number overflow`. +/// +/// The service applies these rules to a zero too (`0E126` is an overflow), but +/// a zero whose exponent fits in 32 bits does not reach this check: the +/// request's `AttributeValue` holds `0` for it, since +/// [`validate_and_normalize_number`] range checks only a zero whose exponent +/// does not fit (#618). +/// +/// Java's `new BigDecimal(s)` reads a number this way. It also reads any +/// Unicode decimal digit as a digit, and so does the service (`\u{ff15}` is +/// stored as `5` on every operation measured). This check refuses those, as +/// [`validate_and_normalize_number`] does. +/// +/// [`validate_and_normalize_number`]: super::validate_and_normalize_number +fn validate_number_i32_scale(s: &str) -> Result<(), NumberFault> { + if s.is_empty() { + return Err(NumberFault::Empty); + } + let unsigned = s.strip_prefix(['+', '-']).unwrap_or(s); + let (mantissa, exponent) = match unsigned.split_once(['e', 'E']) { + Some((mantissa, exponent)) => ( + mantissa, + exponent + .parse::() + .map_err(|_| NumberFault::Malformed)?, + ), + None => (unsigned, 0), + }; + let (int_part, frac_part) = mantissa.split_once('.').unwrap_or((mantissa, "")); + let digits_only = |part: &str| part.bytes().all(|b| b.is_ascii_digit()); + if (int_part.is_empty() && frac_part.is_empty()) + || !digits_only(int_part) + || !digits_only(frac_part) + { + return Err(NumberFault::Malformed); + } + let scale = i64::try_from(frac_part.len()) + .ok() + .and_then(|fraction_digits| i32::try_from(fraction_digits - i64::from(exponent)).ok()) + .ok_or(NumberFault::Malformed)?; + + let digits = format!("{int_part}{frac_part}"); + let unscaled = digits.trim_start_matches('0'); + let precision = i32::try_from(unscaled.len().max(1)).map_err(|_| NumberFault::Malformed)?; + let magnitude = precision.wrapping_sub(scale); + if i64::from(magnitude) > MAX_MAGNITUDE { + return Err(NumberFault::Overflow); + } + if i64::from(magnitude) < MIN_MAGNITUDE { + return Err(NumberFault::Underflow); + } + if unscaled.trim_end_matches('0').len() > MAX_SIGNIFICANT_DIGITS { + return Err(NumberFault::TooManyDigits); + } + Ok(()) +} + +/// The text of a validation error. +fn message(err: DynamoDbError) -> String { + match err { + DynamoDbError::ValidationException(message) => message, + other => other.to_string(), + } +} + +/// A message the operation reports as it is. +fn bare(message: String) -> DynamoDbError { + DynamoDbError::ValidationException(message) +} + +/// A message with the prefix `PutItem`, `UpdateItem` and `DeleteItem` use. +fn single_write(message: String) -> DynamoDbError { + DynamoDbError::ValidationException(format!("{SINGLE_WRITE_PREFIX}{message}")) +} + +#[cfg(test)] +mod tests; diff --git a/crates/core/src/validation/number/operations/tests.rs b/crates/core/src/validation/number/operations/tests.rs new file mode 100644 index 000000000..2fde7e61e --- /dev/null +++ b/crates/core/src/validation/number/operations/tests.rs @@ -0,0 +1,840 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +use serde_json::{Value, json}; + +use super::*; +use crate::types::{BatchWriteItemInput, TransactGetItem, TransactWriteItem}; +use crate::validation::number::validate_and_normalize_number; + +// Every value below was sent to DynamoDB twice on 2026-10-27 (us-east-1) on +// the operation path the test names, with the same answer both times, except +// where a comment gives another date or says the test pins a choice of these +// checks. +const NUMERIC: &str = "The parameter cannot be converted to a numeric value"; +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"; +const DIGITS: &str = "Attempting to store more than 38 significant digits in a Number"; + +fn refused(result: Result<(), DynamoDbError>) -> String { + match result { + Err(DynamoDbError::ValidationException(message)) => message, + other => panic!("expected ValidationException, got {other:?}"), + } +} + +// The `ImportTable` reader. Its rows were measured on `BatchWriteItem` on +// 2026-10-27, when that operation read numbers this way; the `ImportTable` +// rows measured on 2026-11-03, in the `import_row` tests below, follow the +// same rules. + +fn i32_scale(n: &str) -> Result<(), String> { + validate_number_i32_scale(n).map_err(|fault| fault.message(n)) +} + +#[test] +fn i32_scale_accepts_what_the_service_accepts() { + for n in [ + "6180".to_owned(), + format!("1{}", "0".repeat(38)), + format!("1{}", "0".repeat(100)), + format!("1.{}", "0".repeat(40)), + format!("{}0", "1".repeat(38)), + format!("{}.0", "1".repeat(38)), + "0".repeat(40), + format!("0.{}", "0".repeat(40)), + format!("{}1", "0".repeat(40)), + "+5".to_owned(), + "5.".to_owned(), + ".5".to_owned(), + "+.5".to_owned(), + "-.5".to_owned(), + "1E+5".to_owned(), + format!("1E{}5", "0".repeat(24)), + format!("1E-{}5", "0".repeat(24)), + "0.0E126".to_owned(), + "0E125".to_owned(), + "0E-130".to_owned(), + ] { + assert_eq!(i32_scale(&n), Ok(()), "{n:?}"); + } + // A lowercase exponent marker: accepted on BatchWriteItem and PutItem and + // stored as the 64-bit reader writes it (measured 2026-10-27, twice each). + for (n, stored) in [ + ("1e5", "100000"), + ("1e+5", "100000"), + ("1.5e-3", "0.0015"), + ("0e125", "0"), + ] { + assert_eq!(i32_scale(n), Ok(()), "{n:?}"); + assert_eq!(validate_and_normalize_number(n).unwrap(), stored, "{n:?}"); + } +} + +#[test] +fn i32_scale_refuses_an_exponent_or_scale_outside_32_bits_as_malformed() { + for n in [ + "abc", + " 5", + "1E2147483648", + "1E4294967296", + "1E9223372036854775807", + "1E9223372036854775808", + "1E-2147483648", + "1.5E-2147483648", + "1E-2147483649", + "0E2147483648", + "0E9223372036854775807", + "1E", + "1E+", + ".", + "-", + "1E5.5", + "\u{b2}", + "1.5a", + "1.\u{b2}", + "1.5.5", + "1.e", + ] { + assert_eq!(i32_scale(n), Err(format!("{NUMERIC}: {n}")), "{n:?}"); + } + // An empty value answers the message with no colon. + assert_eq!(i32_scale(""), Err(NUMERIC.to_owned())); +} + +#[test] +fn i32_scale_magnitude_wraps_and_comes_before_the_digit_count() { + let ones = "1".repeat(39); + for n in [ + "1E126".to_owned(), + format!("1E{}126", "0".repeat(24)), + format!("{ones}E200"), + ] { + assert_eq!(i32_scale(&n), Err(OVERFLOW.to_owned()), "{n:?}"); + } + for n in [ + "1E-131".to_owned(), + "1E-2147483647".to_owned(), + // The magnitude passes the 32-bit maximum and wraps to a negative. + "10E2147483647".to_owned(), + "1E2147483647".to_owned(), + format!("{ones}E2147483647"), + format!("{ones}E-200"), + ] { + assert_eq!(i32_scale(&n), Err(UNDERFLOW.to_owned()), "{n:?}"); + } + assert_eq!(i32_scale(&ones), Err(DIGITS.to_owned())); +} + +/// One number on four paths: a `PutItem` item value, a `BatchWriteItem` put +/// item value, a `TransactWriteItems` `Put` item value and a +/// `TransactGetItems` key value, as the answer each path gives. +fn four_paths(n: &str) -> [Result<(), String>; 4] { + let answer = |result: Result<(), DynamoDbError>| match result { + Ok(()) => Ok(()), + Err(DynamoDbError::ValidationException(message)) => Err(message), + Err(other) => panic!("expected ValidationException, got {other:?}"), + }; + let item = json!({"pk": {"S": "a"}, "n": {"N": n}}); + let put: PutItemInput = + serde_json::from_value(json!({"TableName": "t", "Item": item.clone()})).unwrap(); + let batch: BatchWriteItemInput = serde_json::from_value( + json!({"RequestItems": {"t": [{"PutRequest": {"Item": item.clone()}}]}}), + ) + .unwrap(); + let transact_put: TransactWriteItem = + serde_json::from_value(json!({"Put": {"TableName": "t", "Item": item}})).unwrap(); + let transact_get: TransactGetItem = + serde_json::from_value(json!({"Get": {"TableName": "t", "Key": {"pk": {"N": n}}}})) + .unwrap(); + [ + answer(put_item_item(&put)), + answer(batch_write_item(&batch)), + answer(transact_write_item(&transact_put)), + answer(transact_get_item(&transact_get)), + ] +} + +#[test] +fn put_batch_and_transact_paths_read_a_number_alike() { + // Measured in us-east-1 on 2026-11-11, each value twice on PutItem, + // BatchWriteItem, TransactWriteItems and TransactGetItems with the same + // answer on each path. The service puts `1 validation error detected: ` + // ahead of the PutItem message, and ExtendDB's PutItem item errors carry + // no prefix. `1E125` is accepted on each path. + let convert = |n: &str| format!("{NUMERIC}: {n}"); + let forty = "1234567890123456789012345678901234567890"; + for (n, want) in [ + ("10E2147483647".to_owned(), Err(OVERFLOW.to_owned())), + ("1E2147483648".to_owned(), Err(OVERFLOW.to_owned())), + ("1E2147483647".to_owned(), Err(OVERFLOW.to_owned())), + ("1E-2147483648".to_owned(), Err(UNDERFLOW.to_owned())), + ("1E-2147483649".to_owned(), Err(UNDERFLOW.to_owned())), + ("1E9223372036854775807".to_owned(), Err(OVERFLOW.to_owned())), + ( + "1E9223372036854775808".to_owned(), + Err(convert("1E9223372036854775808")), + ), + ( + "10E9223372036854775807".to_owned(), + Err(OVERFLOW.to_owned()), + ), + ( + "1E-9223372036854775808".to_owned(), + Err(UNDERFLOW.to_owned()), + ), + ( + "1E-9223372036854775809".to_owned(), + Err(convert("1E-9223372036854775809")), + ), + ("1E125".to_owned(), Ok(())), + // Zeros and forty digits, also sent to UpdateItem as an + // ExpressionAttributeValues value with the same answer. The forty + // digits without an exponent were sent to BatchWriteItem alone. + ("0E2147483648".to_owned(), Err(OVERFLOW.to_owned())), + ("0E-2147483649".to_owned(), Err(UNDERFLOW.to_owned())), + ( + "0E9223372036854775807".to_owned(), + Err(UNDERFLOW.to_owned()), + ), + (format!("{forty}E200"), Err(DIGITS.to_owned())), + (format!("{forty}E-200"), Err(DIGITS.to_owned())), + (forty.to_owned(), Err(DIGITS.to_owned())), + ] { + for (path, got) in [ + "PutItem", + "BatchWriteItem", + "TransactWriteItems", + "TransactGetItems", + ] + .into_iter() + .zip(four_paths(&n)) + { + assert_eq!(got, want, "{n} on {path}"); + } + } +} + +fn get(key: Value) -> GetItemInput { + serde_json::from_value(json!({"TableName": "t", "Key": key})).unwrap() +} + +#[test] +fn get_item_key_answers_without_a_prefix() { + assert_eq!( + refused(get_item(&get(json!({"pk": {"N": "abc"}})))), + format!("{NUMERIC}: abc") + ); + // A malformed number after a valid attribute is found too. + assert_eq!( + refused(get_item(&get(json!({"a": {"S": "x"}, "b": {"N": "abc"}})))), + format!("{NUMERIC}: abc") + ); + // The 64-bit exponent rules, and the empty value with its colon. + assert_eq!( + refused(get_item(&get(json!({"pk": {"N": "10E2147483647"}})))), + OVERFLOW + ); + assert_eq!( + refused(get_item(&get(json!({"pk": {"N": "0E2147483648"}})))), + OVERFLOW + ); + assert_eq!( + refused(get_item(&get(json!({"pk": {"N": ""}})))), + format!("{NUMERIC}: ") + ); + get_item(&get(json!({"pk": {"N": "5"}, "sk": {"S": "x"}}))).unwrap(); +} + +#[test] +fn batch_get_keys_answer_without_a_prefix() { + let keys: Vec = serde_json::from_value(json!([ + {"pk": {"N": "1"}}, + {"pk": {"N": "1E126"}}, + ])) + .unwrap(); + assert_eq!(refused(batch_get_keys(&keys)), OVERFLOW); + batch_get_keys(&keys[..1]).unwrap(); +} + +fn delete(body: Value) -> DeleteItemInput { + serde_json::from_value(body).unwrap() +} + +#[test] +fn delete_item_key_and_values_carry_the_prefix() { + let input = delete(json!({ + "TableName": "t", + "Key": {"pk": {"N": "abc"}}, + "ConditionExpression": "n = :v", + "ExpressionAttributeValues": {":v": {"N": "1E126"}}, + })); + assert_eq!( + refused(delete_item_key(&input)), + format!("1 validation error detected: {NUMERIC}: abc") + ); + // No `ExpressionAttributeValues contains invalid value` wrapper. + assert_eq!( + refused(delete_item_values(&input)), + format!("1 validation error detected: {OVERFLOW}") + ); + let input = delete(json!({"TableName": "t", "Key": {"pk": {"N": ""}}})); + assert_eq!( + refused(delete_item_key(&input)), + format!("1 validation error detected: {NUMERIC}: ") + ); + delete_item_values(&input).unwrap(); +} + +#[test] +fn update_item_key_carries_the_prefix() { + let input: UpdateItemInput = serde_json::from_value(json!({ + "TableName": "t", + "Key": {"pk": {"N": "1E126"}}, + "UpdateExpression": "SET a = :s", + "ExpressionAttributeValues": {":s": {"S": "x"}}, + })) + .unwrap(); + assert_eq!( + refused(update_item_key(&input)), + format!("1 validation error detected: {OVERFLOW}") + ); +} + +fn put(body: Value) -> PutItemInput { + serde_json::from_value(body).unwrap() +} + +#[test] +fn put_item_values_carry_the_prefix_and_follow_the_item() { + let input = put(json!({ + "TableName": "t", + "Item": {"pk": {"S": "a"}}, + "ConditionExpression": "attribute_not_exists(n)", + "ExpressionAttributeValues": {":v": {"NS": ["1", "abc"]}}, + })); + assert_eq!( + refused(put_item_values(&input)), + format!("1 validation error detected: {NUMERIC}: abc") + ); + // The item's number is reported first, by the item check, with its own + // text; the values check answers only its own placeholders. + let input = put(json!({ + "TableName": "t", + "Item": {"pk": {"S": "a"}, "n": {"N": "xyz"}}, + "ConditionExpression": "n = :v", + "ExpressionAttributeValues": {":v": {"N": "abc"}}, + })); + assert_eq!(refused(put_item_item(&input)), format!("{NUMERIC}: xyz")); + assert_eq!( + refused(put_item_values(&input)), + format!("1 validation error detected: {NUMERIC}: abc") + ); + // Without values, there is nothing for the values check to refuse. + let input = put(json!({"TableName": "t", "Item": {"pk": {"S": "a"}, "n": {"N": "xyz"}}})); + put_item_values(&input).unwrap(); +} + +#[test] +fn transact_get_item_reads_a_64_bit_exponent() { + // Measured 2026-11-11: `10E2147483647` and `1E2147483648` are a `Number + // overflow`, and an empty `N` answers the message with the colon (it had + // no colon on 2026-10-27). + let item = |n: &str| -> TransactGetItem { + serde_json::from_value(json!({"Get": {"TableName": "t", "Key": {"pk": {"N": n}}}})).unwrap() + }; + assert_eq!(refused(transact_get_item(&item("10E2147483647"))), OVERFLOW); + assert_eq!(refused(transact_get_item(&item("1E2147483648"))), OVERFLOW); + assert_eq!( + refused(transact_get_item(&item(""))), + format!("{NUMERIC}: ") + ); + transact_get_item(&item("5")).unwrap(); +} + +#[test] +fn batch_write_item_checks_put_items_and_delete_keys() { + let batch = |requests: Value| -> BatchWriteItemInput { + serde_json::from_value(json!({"RequestItems": {"t": requests}})).unwrap() + }; + for (request, want) in [ + ( + json!({"PutRequest": {"Item": {"pk": {"S": "a"}, "ns": {"NS": ["1", "abc"]}}}}), + format!("{NUMERIC}: abc"), + ), + ( + json!({"PutRequest": {"Item": {"a": {"S": "x"}, "b": {"N": "abc"}}}}), + format!("{NUMERIC}: abc"), + ), + ( + json!({"PutRequest": {"Item": {"pk": {"S": "a"}, "m": {"M": {"x": {"N": "1E126"}}}}}}), + OVERFLOW.to_owned(), + ), + ( + // Measured 2026-11-11. + json!({"PutRequest": {"Item": {"pk": {"S": "a"}, "n": {"N": "10E2147483647"}}}}), + OVERFLOW.to_owned(), + ), + ( + json!({"DeleteRequest": {"Key": {"pk": {"N": "1E126"}}}}), + OVERFLOW.to_owned(), + ), + // An empty `N` answers the message without the colon in a delete key, + // a put item value and an `NS` member (measured 2026-11-11; the + // delete key also on 2026-10-27). + ( + json!({"DeleteRequest": {"Key": {"pk": {"N": ""}}}}), + NUMERIC.to_owned(), + ), + ( + json!({"PutRequest": {"Item": {"pk": {"S": "a"}, "n": {"N": ""}}}}), + NUMERIC.to_owned(), + ), + ( + json!({"PutRequest": {"Item": {"pk": {"S": "a"}, "n": {"NS": ["1", ""]}}}}), + NUMERIC.to_owned(), + ), + ] { + let input = batch(json!([{"PutRequest": {"Item": {"pk": {"S": "ok"}}}}, request])); + assert_eq!(refused(batch_write_item(&input)), want, "{request}"); + } + batch_write_item(&batch( + json!([{"PutRequest": {"Item": {"n": {"N": "1E2"}}}}]), + )) + .unwrap(); +} + +fn transact(action: Value) -> TransactWriteItem { + serde_json::from_value(action).unwrap() +} + +#[test] +fn transact_write_item_checks_values_then_key_or_item() { + // Values come before the key: the service names the value (measured + // 2026-10-27). A value answers the number message alone, with no prefix + // and no wrapper, a value inside an `L` included, and an empty `N` with + // the colon (measured 2026-11-11 on Update, Put, Delete and + // ConditionCheck values; the empty `N` and the `L` on an Update value). + let update = transact(json!({"Update": { + "TableName": "t", + "Key": {"pk": {"N": "abc"}}, + "UpdateExpression": "SET a = :v", + "ExpressionAttributeValues": {":v": {"N": "xyz"}}, + }})); + assert_eq!( + refused(transact_write_item(&update)), + format!("{NUMERIC}: xyz") + ); + let check = transact(json!({"ConditionCheck": { + "TableName": "t", + "Key": {"pk": {"S": "a"}}, + "ConditionExpression": "n = :v", + "ExpressionAttributeValues": {":v": {"N": ""}}, + }})); + assert_eq!(refused(transact_write_item(&check)), format!("{NUMERIC}: ")); + let put = transact(json!({"Put": { + "TableName": "t", + "Item": {"pk": {"S": "a"}, "n": {"N": "xyz"}}, + "ConditionExpression": "n = :v", + "ExpressionAttributeValues": {":v": {"L": [{"N": "1E126"}]}}, + }})); + assert_eq!(refused(transact_write_item(&put)), OVERFLOW); + // Measured 2026-11-11 on a TransactWriteItems Delete key: `1E2147483647` + // is a `Number overflow`, with no prefix. + let delete = transact(json!({"Delete": { + "TableName": "t", + "Key": {"pk": {"N": "1E2147483647"}}, + }})); + assert_eq!(refused(transact_write_item(&delete)), OVERFLOW); +} + +#[test] +fn transact_write_item_keys_and_items_answer_an_empty_number_with_the_colon() { + // Measured 2026-11-11 on a Delete key and a Put item value, each twice; + // both had no colon on 2026-10-27. + let delete = transact(json!({"Delete": {"TableName": "t", "Key": {"pk": {"N": ""}}}})); + assert_eq!( + refused(transact_write_item(&delete)), + format!("{NUMERIC}: ") + ); + let put = transact(json!({"Put": { + "TableName": "t", + "Item": {"pk": {"S": "a"}, "n": {"N": ""}}, + }})); + assert_eq!(refused(transact_write_item(&put)), format!("{NUMERIC}: ")); +} + +#[test] +fn transact_update_values_answer_the_inner_number_message() { + // Measured 2026-11-11 with `SET n = :v`, each value twice: no prefix, no + // wrapper, and the colon for an empty `N`. + for (value, want) in [ + (json!({"N": "1E126"}), OVERFLOW.to_owned()), + (json!({"N": "abc"}), format!("{NUMERIC}: abc")), + (json!({"N": ""}), format!("{NUMERIC}: ")), + (json!({"M": {"x": {"N": "abc"}}}), format!("{NUMERIC}: abc")), + (json!({"L": [{"N": "1E126"}]}), OVERFLOW.to_owned()), + (json!({"NS": ["1", "abc"]}), format!("{NUMERIC}: abc")), + ] { + let update = transact(json!({"Update": { + "TableName": "t", + "Key": {"pk": {"S": "a"}}, + "UpdateExpression": "SET n = :v", + "ExpressionAttributeValues": {":v": value.clone()}, + }})); + assert_eq!(refused(transact_write_item(&update)), want, "{value}"); + } +} + +#[test] +fn update_item_values_carry_the_prefix_and_no_wrapper() { + // Measured 2026-11-11 with `SET n = :v`, each value twice. + for (value, want) in [ + (json!({"N": "abc"}), format!("{NUMERIC}: abc")), + (json!({"N": "12e"}), format!("{NUMERIC}: 12e")), + (json!({"N": ""}), format!("{NUMERIC}: ")), + (json!({"N": "1E2147483648"}), OVERFLOW.to_owned()), + (json!({"N": "1E126"}), OVERFLOW.to_owned()), + (json!({"M": {"x": {"N": "abc"}}}), format!("{NUMERIC}: abc")), + (json!({"L": [{"N": "1E126"}]}), OVERFLOW.to_owned()), + (json!({"NS": ["1", "abc"]}), format!("{NUMERIC}: abc")), + ] { + let input: UpdateItemInput = serde_json::from_value(json!({ + "TableName": "t", + "Key": {"pk": {"S": "a"}}, + "UpdateExpression": "SET n = :v", + "ExpressionAttributeValues": {":v": value.clone()}, + })) + .unwrap(); + assert_eq!( + refused(update_item_values(&input)), + format!("1 validation error detected: {want}"), + "{value}" + ); + } + let input: UpdateItemInput = serde_json::from_value(json!({ + "TableName": "t", + "Key": {"pk": {"S": "a"}}, + "UpdateExpression": "SET n = :v", + "ExpressionAttributeValues": {":v": {"N": "1E125"}}, + })) + .unwrap(); + update_item_values(&input).unwrap(); +} + +#[test] +fn transact_put_and_delete_values_are_checked_only_with_a_condition() { + for action in ["Put", "Delete"] { + let field = if action == "Put" { "Item" } else { "Key" }; + let mut body = json!({ + "TableName": "t", + field: {"pk": {"S": "a"}}, + "ExpressionAttributeValues": {":v": {"N": "abc"}}, + }); + transact_write_item(&transact(json!({action: body.clone()}))).unwrap(); + // An empty ConditionExpression counts as present. + body["ConditionExpression"] = json!(""); + assert_eq!( + refused(transact_write_item(&transact(json!({action: body})))), + format!("{NUMERIC}: abc"), + "{action}" + ); + } +} + +#[test] +fn several_malformed_values_are_reported_in_placeholder_order() { + // The service named either value on two sends of one PutItem request; + // these checks pin the first placeholder in name order. + let input = put(json!({ + "TableName": "t", + "Item": {"pk": {"S": "a"}}, + "ConditionExpression": "n = :b OR n = :a", + "ExpressionAttributeValues": {":b": {"N": "abc"}, ":a": {"N": "xyz"}}, + })); + assert_eq!( + refused(put_item_values(&input)), + format!("1 validation error detected: {NUMERIC}: xyz") + ); +} + +fn expected(map: Value) -> HashMap { + serde_json::from_value(map).unwrap() +} + +#[test] +fn expected_values_carry_the_prefix() { + // Measured 2026-11-11 on items that do not exist: a Value on PutItem, + // DeleteItem and UpdateItem, and an EQ AttributeValueList on PutItem, + // each with `abc`, `1E2147483648` and an empty N. The last two rows pin + // a choice of these checks: a later list member and a number nested in + // a map are read too. + for (entry, want) in [ + (json!({"Value": {"N": "abc"}}), format!("{NUMERIC}: abc")), + (json!({"Value": {"N": "1E2147483648"}}), OVERFLOW.to_owned()), + (json!({"Value": {"N": ""}}), format!("{NUMERIC}: ")), + ( + json!({"ComparisonOperator": "EQ", "AttributeValueList": [{"N": "abc"}]}), + format!("{NUMERIC}: abc"), + ), + ( + json!({"ComparisonOperator": "EQ", "AttributeValueList": [{"N": "1E2147483648"}]}), + OVERFLOW.to_owned(), + ), + ( + json!({"ComparisonOperator": "EQ", "AttributeValueList": [{"N": ""}]}), + format!("{NUMERIC}: "), + ), + ( + json!({"ComparisonOperator": "BETWEEN", "AttributeValueList": [{"N": "1"}, {"N": "1E126"}]}), + OVERFLOW.to_owned(), + ), + ( + json!({"Value": {"M": {"x": {"N": "abc"}}}}), + format!("{NUMERIC}: abc"), + ), + ] { + let map = expected(json!({"a": entry.clone()})); + assert_eq!( + refused(expected_values(Some(&map))), + format!("1 validation error detected: {want}"), + "{entry}" + ); + } + // `1E125` answered ConditionalCheckFailedException: it passes this check. + expected_values(Some(&expected(json!({"a": {"Value": {"N": "1E125"}}})))).unwrap(); + expected_values(Some(&expected(json!({"a": {"Exists": false}})))).unwrap(); + expected_values(None).unwrap(); +} + +#[test] +fn expected_values_are_read_in_attribute_name_order() { + // Pins a choice of these checks, as for the placeholders above. + let map = expected(json!({ + "b": {"Value": {"N": "abc"}}, + "a": {"Value": {"N": "xyz"}}, + })); + assert_eq!( + refused(expected_values(Some(&map))), + format!("1 validation error detected: {NUMERIC}: xyz") + ); +} + +/// A value nested `levels` maps deep. +fn deep(levels: usize) -> Value { + let mut value = json!({"S": "leaf"}); + for _ in 0..levels { + value = json!({"M": {"a": value}}); + } + value +} + +const NESTING: &str = "Nesting Levels have exceeded supported limits: \ + Attributes in the item have nested levels beyond supported limit"; + +#[test] +fn a_value_nested_too_deep_is_reported_before_a_malformed_number() { + // A BatchWriteItem put item holding both a 33-level value and a number + // the service cannot read answers the nesting message, whichever comes + // first in the item (measured 2026-10-27, the same on both sends). The + // PutItem and DeleteItem condition values answer it too, the order they + // answered before this check; the service answered the nesting on one + // send and the number on the other there. + let batch = |requests: Value| -> BatchWriteItemInput { + serde_json::from_value(json!({"RequestItems": {"t": requests}})).unwrap() + }; + + let values = put(json!({ + "TableName": "t", + "Item": {"pk": {"S": "a"}}, + "ConditionExpression": "n = :d OR n = :v", + "ExpressionAttributeValues": {":d": deep(33), ":v": {"N": "abc"}}, + })); + assert_eq!( + refused(put_item_values(&values)), + format!("1 validation error detected: {NESTING}") + ); + + let batch_input = batch(json!([ + {"PutRequest": {"Item": {"pk": {"S": "a"}, "n": {"N": "abc"}, "d": deep(33)}}} + ])); + assert_eq!(refused(batch_write_item(&batch_input)), NESTING); + + // The earlier request's malformed number still wins over a later + // request's nesting (also measured). + let across = batch(json!([ + {"PutRequest": {"Item": {"pk": {"S": "a"}, "n": {"N": "abc"}}}}, + {"PutRequest": {"Item": {"pk": {"S": "b"}, "d": deep(33)}}} + ])); + assert_eq!( + refused(batch_write_item(&across)), + format!("{NUMERIC}: abc") + ); +} + +#[test] +fn the_put_item_item_reports_a_malformed_number_before_a_value_nested_too_deep() { + // The order `validate_put_item` checks them in, and so what PutItem + // answered before this check. The service follows the request body, + // which `Item` does not keep: a malformed `n` ahead of a 33-level `d` + // answered the number on both sends (measured 2026-10-27). + let item = put(json!({ + "TableName": "t", + "Item": {"pk": {"S": "a"}, "n": {"N": "abc"}, "d": deep(33)}, + })); + assert_eq!(refused(put_item_item(&item)), format!("{NUMERIC}: abc")); + let item = put(json!({ + "TableName": "t", + "Item": {"pk": {"S": "a"}, "d": deep(33)}, + })); + assert_eq!(refused(put_item_item(&item)), NESTING); +} + +#[test] +fn put_item_checks_its_item_without_expression_attribute_values() { + // The item is checked whether or not the request carries values: measured + // 2026-10-27, PutItem to a table that does not exist answers the item's + // malformed number, not ResourceNotFoundException. + let input = put(json!({ + "TableName": "t", + "Item": {"pk": {"S": "a"}, "n": {"N": "abc"}}, + })); + assert_eq!(refused(put_item_item(&input)), format!("{NUMERIC}: abc")); +} + +#[test] +fn batch_write_item_visits_tables_in_name_order() { + // Two tables each holding a malformed number: the answer names the + // first table in name order, so one request cannot answer differently + // from one run to the next. + let input: BatchWriteItemInput = serde_json::from_value(json!({"RequestItems": { + "t2": [{"PutRequest": {"Item": {"n": {"N": "xyz"}}}}], + "t1": [{"DeleteRequest": {"Key": {"pk": {"N": "abc"}}}}], + }})) + .unwrap(); + assert_eq!(refused(batch_write_item(&input)), format!("{NUMERIC}: abc")); +} + +// `ImportTable` rows: measured 2026-11-03 (us-east-1), one import from S3 per +// row. A row is read through `Item`'s deserializer, as the import file +// reader reads it, so a valid number arrives normalized and any other +// keeps its text. + +fn import_item(row: &Value) -> Item { + serde_json::from_value(row.clone()).unwrap() +} + +#[test] +fn import_row_stops_at_a_value_that_is_not_a_number() { + for row in [ + json!({"pk": {"S": "a"}, "n": {"N": "abc"}}), + json!({"pk": {"S": "a"}, "n": {"N": ""}}), + json!({"pk": {"S": "a"}, "n": {"N": " 5"}}), + json!({"pk": {"S": "a"}, "n": {"N": "12e"}}), + // These three were measured again on 2026-11-11, one file each. + json!({"pk": {"S": "a"}, "n": {"N": "1E2147483648"}}), + json!({"pk": {"S": "a"}, "n": {"N": "1E9223372036854775808"}}), + json!({"pk": {"S": "a"}, "n": {"N": "1E-2147483648"}}), + json!({"pk": {"S": "a"}, "n": {"N": "1.5E-2147483647"}}), + json!({"pk": {"S": "a"}, "s": {"NS": ["1", "abc"]}}), + json!({"pk": {"N": "abc"}}), + // A number out of range ahead of it in the row does not change the + // answer, nor does a missing key. + json!({"pk": {"S": "a"}, "a": {"N": "1E126"}, "b": {"N": "abc"}}), + json!({"other": {"S": "x"}, "n": {"N": "abc"}}), + // The service imports a row holding a Unicode decimal digit with the + // digit (`5` here); this check refuses it, as every number check does. + json!({"pk": {"S": "a"}, "n": {"N": "\u{ff15}"}}), + ] { + assert_eq!( + import_row(&import_item(&row)), + Err(ImportRowFault::Malformed), + "{row}" + ); + } +} + +#[test] +fn import_row_refuses_a_number_out_of_range() { + let ones = "1".repeat(39); + for (row, fault) in [ + ( + json!({"pk": {"S": "a"}, "n": {"N": "1E126"}}), + NumberFault::Overflow, + ), + ( + json!({"pk": {"S": "a"}, "n": {"N": "1E999999999"}}), + NumberFault::Overflow, + ), + ( + json!({"pk": {"S": "a"}, "n": {"N": "1E-131"}}), + NumberFault::Underflow, + ), + ( + json!({"pk": {"S": "a"}, "n": {"N": "-1E-131"}}), + NumberFault::Underflow, + ), + // The magnitude passes the 32-bit maximum and wraps to a negative. + // Measured again on 2026-11-11, with `10E2147483647` as well. + ( + json!({"pk": {"S": "a"}, "n": {"N": "1E2147483647"}}), + NumberFault::Underflow, + ), + ( + json!({"pk": {"S": "a"}, "n": {"N": "10E2147483647"}}), + NumberFault::Underflow, + ), + ( + json!({"pk": {"S": "a"}, "n": {"N": ones}}), + NumberFault::TooManyDigits, + ), + ( + json!({"pk": {"S": "a"}, "s": {"NS": ["1", "1E126"]}}), + NumberFault::Overflow, + ), + ( + json!({"pk": {"S": "a"}, "m": {"M": {"x": {"N": "1E126"}}}}), + NumberFault::Overflow, + ), + ( + json!({"pk": {"S": "a"}, "l": {"L": [{"N": "1E-131"}]}}), + NumberFault::Underflow, + ), + (json!({"pk": {"N": "1E126"}}), NumberFault::Overflow), + ( + json!({"other": {"S": "x"}, "n": {"N": "1E126"}}), + NumberFault::Overflow, + ), + ] { + assert_eq!( + import_row(&import_item(&row)), + Err(ImportRowFault::OutOfRange(fault)), + "{row}" + ); + } +} + +#[test] +fn import_row_accepts_what_the_service_imports() { + for row in [ + json!({"pk": {"S": "a"}, "n": {"N": "5"}}), + json!({"pk": {"S": "a"}, "n": {"N": "1.50"}}), + json!({"pk": {"S": "a"}, "n": {"N": "-0.0"}}), + json!({"pk": {"S": "a"}, "n": {"N": "+5"}}), + json!({"pk": {"S": "a"}, "n": {"N": "1.5E2"}}), + json!({"pk": {"S": "a"}, "n": {"N": "0E125"}}), + json!({"pk": {"S": "a"}, "n": {"N": "9.9999999999999999999999999999999999999E125"}}), + json!({"pk": {"S": "a"}, "n": {"N": "1E-130"}}), + json!({"pk": {"S": "a"}, "s": {"NS": ["1.0", "2"]}}), + json!({"pk": {"N": "2.50"}}), + json!({"pk": {"S": "a"}, "b": {"BOOL": true}, "s": {"S": "abc"}}), + ] { + assert_eq!(import_row(&import_item(&row)), Ok(()), "{row}"); + } + // The service refuses `0E126` as `Number overflow`; the row's value is + // `0` until the 64-bit reader range checks every zero (#618). + let zero = json!({"pk": {"S": "a"}, "n": {"N": "0E126"}}); + assert_eq!(import_row(&import_item(&zero)), Ok(())); +} diff --git a/crates/engine/src/batch_get_item.rs b/crates/engine/src/batch_get_item.rs index d392c4a1c..263ce6c0b 100755 --- a/crates/engine/src/batch_get_item.rs +++ b/crates/engine/src/batch_get_item.rs @@ -150,6 +150,21 @@ pub async fn handle_batch_get_item( table_projections.insert(table_name.clone(), projection); } + // A number the service cannot read in a key is checked after every + // table's projection and duplicate checks, and before the table lookups, + // with tables in name order so that a request answers the same way each + // time. Measured 2026-10-27, the service answered a duplicate key in the + // first table of the request body ahead of a malformed number in the + // second, which this pass follows, and a malformed number in the first + // table ahead of a duplicate key in the second, where this pass reports + // the duplicate, as before this check. + let mut table_names: Vec<&String> = input.request_items.keys().collect(); + table_names.sort(); + for table_name in table_names { + let keys = &input.request_items[table_name].keys; + extenddb_core::validation::number::operations::batch_get_keys(keys)?; + } + let mut responses: HashMap> = HashMap::new(); let mut total_rcu: f64 = 0.0; let mut total_pre_proj_bytes: usize = 0; diff --git a/crates/engine/src/batch_write_item.rs b/crates/engine/src/batch_write_item.rs index 71cd354f8..beda44e9e 100755 --- a/crates/engine/src/batch_write_item.rs +++ b/crates/engine/src/batch_write_item.rs @@ -65,7 +65,6 @@ pub async fn handle_batch_write_item( } } } - // Attribute names over the size limit are request-shape validation: every // PutRequest item is checked before any table lookup and before the key // checks, so a request naming a missing table, or an item missing its key, @@ -80,6 +79,14 @@ pub async fn handle_batch_write_item( } } + // Every number in every request is checked before any table is looked up + // or written: the service reports a malformed number ahead of a missing + // table, a key type mismatch, a missing key and a duplicate key, and + // writes none of a batch whose later put holds one (measured 2026-10-27). + // An attribute name over the size limit comes first: for one item holding + // both, the service answered either fault (measured 2026-11-11). + extenddb_core::validation::number::operations::batch_write_item(&input)?; + let empty_maps = ExpressionMaps::default(); let mut all_icm: HashMap> = HashMap::new(); diff --git a/crates/engine/src/delete_item.rs b/crates/engine/src/delete_item.rs index 7a9e712fb..87948198d 100755 --- a/crates/engine/src/delete_item.rs +++ b/crates/engine/src/delete_item.rs @@ -51,6 +51,10 @@ pub async fn handle_delete_item( let input: DeleteItemInput = serde_json::from_value(body).map_err(crate::deserialize_error)?; extenddb_core::validation::validate_table_name(&input.table_name, &ctx.limits)?; + // A number the service cannot read in the key comes next, ahead of the + // parameter checks, the condition, the table lookup and the key schema + // checks (measured 2026-10-27). + extenddb_core::validation::number::operations::delete_item_key(&input)?; // An attribute name over the size limit that arrives through the // condition's ExpressionAttributeNames is input validation, reported @@ -78,6 +82,15 @@ pub async fn handle_delete_item( ], &[("ConditionExpression", has_condition)], )?; + // The numbers in ExpressionAttributeValues follow the mixed-parameter + // check and precede the other expression checks (measured 2026-10-27). A + // value nested too deep comes ahead of a malformed number, as before this + // check; the service answered either on two sends. + // The later nesting check repeats this one and cannot fail after it. + extenddb_core::validation::number::operations::delete_item_values(&input)?; + // A legacy Expected value's number is read ahead of the condition + // (measured 2026-11-11). + extenddb_core::validation::number::operations::expected_values(input.expected.as_ref())?; extenddb_core::expression::validate_expression_param_usage( input.expression_attribute_names.as_ref(), has_condition, diff --git a/crates/engine/src/get_item.rs b/crates/engine/src/get_item.rs index 5108ee2fc..60dbb274c 100755 --- a/crates/engine/src/get_item.rs +++ b/crates/engine/src/get_item.rs @@ -36,6 +36,11 @@ pub async fn handle_get_item( ) -> Result { let input: GetItemInput = serde_json::from_value(body).map_err(crate::deserialize_error)?; + // A number the service cannot read in the key is reported ahead of the + // projection checks, the table lookup and the key schema checks + // (measured 2026-10-27). + extenddb_core::validation::number::operations::get_item(&input)?; + // Validate the projection (and EAN usage) before the existence // check, so a missing table returns ValidationException, not // ResourceNotFoundException. Key validation stays after existence. diff --git a/crates/engine/src/import_export.rs b/crates/engine/src/import_export.rs index bba2318d7..e941f9abd 100755 --- a/crates/engine/src/import_export.rs +++ b/crates/engine/src/import_export.rs @@ -21,6 +21,7 @@ use extenddb_core::types::{ CreateTableInput, ExportFormat, ImportStatus, ImportTableDescription, ImportTableInput, ImportTableOutput, Item, TableCreationParameters, }; +use extenddb_core::validation::number::operations::{ImportRowFault, import_row}; use extenddb_core::validation::{ validate_attribute_name_sizes, validate_item_keys, validate_item_nesting_depth, validate_item_size, validate_key_sizes, @@ -119,23 +120,74 @@ pub async fn handle_import_table( let mut imported_count: i64 = 0; let mut error_count: i64 = 0; - let processed_count = i64::try_from(items.len()).unwrap_or(i64::MAX); + let mut processed_count: i64 = 0; + // Created before the rows are read so that each row's log line names the + // import it belongs to; `item_number` counts items, not file lines. + let import_arn = format!("{}:import/{}", table_arn, uuid::Uuid::new_v4()); + let table_name = tcp.table_name.as_str(); for item in items { + processed_count += 1; + // Numbers before the other checks: a row holding a value that is not + // a number ends the import of the file whatever else the row holds, + // and a row holding a number out of range is skipped (see + // `import_row`). + match import_row(&item) { + Ok(()) => {} + Err(ImportRowFault::OutOfRange(fault)) => { + tracing::warn!( + import_arn = ?import_arn, + table_name = ?table_name, + item_number = processed_count, + error = %fault.text(), + "import: skipping item with a number out of range" + ); + error_count += 1; + continue; + } + Err(ImportRowFault::Malformed) => { + tracing::warn!( + import_arn = ?import_arn, + table_name = ?table_name, + item_number = processed_count, + "import: item holds a value that is not a number; the rest of the file is not imported" + ); + error_count += 1; + break; + } + } if let Err(e) = validate_item_keys(&item, &key_info.key_schema, &key_info.attribute_definitions) { - tracing::warn!(error = %e, "import: skipping item with invalid keys"); + tracing::warn!( + import_arn = ?import_arn, + table_name = ?table_name, + item_number = processed_count, + error = ?e.to_string(), + "import: skipping item with invalid keys" + ); error_count += 1; continue; } if let Err(e) = validate_item_nesting_depth(&item) { - tracing::warn!(error = %e, "import: skipping item with excessive nesting"); + tracing::warn!( + import_arn = ?import_arn, + table_name = ?table_name, + item_number = processed_count, + error = ?e.to_string(), + "import: skipping item with excessive nesting" + ); error_count += 1; continue; } if let Err(e) = validate_item_size(&item, ctx.limits.max_item_size_bytes) { - tracing::warn!(error = %e, "import: skipping oversized item"); + tracing::warn!( + import_arn = ?import_arn, + table_name = ?table_name, + item_number = processed_count, + error = ?e.to_string(), + "import: skipping oversized item" + ); error_count += 1; continue; } @@ -144,12 +196,24 @@ pub async fn handle_import_table( &ctx.limits, extenddb_core::validation::AttributeNameRequestForm::SingleItem, ) { - tracing::warn!(error = %e, "import: skipping item with oversized attribute name"); + tracing::warn!( + import_arn = ?import_arn, + table_name = ?table_name, + item_number = processed_count, + error = ?e.to_string(), + "import: skipping item with oversized attribute name" + ); error_count += 1; continue; } if let Err(e) = validate_key_sizes(&item, &key_info.key_schema, &ctx.limits) { - tracing::warn!(error = %e, "import: skipping item with oversized key"); + tracing::warn!( + import_arn = ?import_arn, + table_name = ?table_name, + item_number = processed_count, + error = ?e.to_string(), + "import: skipping item with oversized key" + ); error_count += 1; continue; } @@ -163,11 +227,29 @@ pub async fn handle_import_table( } let end_time = epoch_seconds(); - let import_arn = format!("{}:import/{}", table_arn, uuid::Uuid::new_v4()); + + // An import that refused any row is FAILED with `ItemValidationError`, + // and the rows it accepted stay in the table (measured 2026-11-03 with + // number rows, and 2026-11-11 with a row whose key has the wrong type). + // The service's message sends the reader to its error log in CloudWatch; + // ExtendDB writes each refused row to the server log instead. + let (import_status, failure_code, failure_message) = if error_count > 0 { + ( + ImportStatus::Failed, + Some("ItemValidationError".to_owned()), + Some( + "Some of the items failed validation checks and were not imported. \ + Please check the server log for more details." + .to_owned(), + ), + ) + } else { + (ImportStatus::Completed, None, None) + }; let description = ImportTableDescription { import_arn, - import_status: ImportStatus::Completed, + import_status, table_arn, table_id: Some(table_id), file_source: input.file_source, @@ -178,8 +260,8 @@ pub async fn handle_import_table( imported_item_count: imported_count, start_time: Some(start_time), end_time: Some(end_time), - failure_code: None, - failure_message: None, + failure_code, + failure_message, }; serialize_output(&ImportTableOutput { diff --git a/crates/engine/src/put_item.rs b/crates/engine/src/put_item.rs index 15efcc9b3..39b708ea0 100755 --- a/crates/engine/src/put_item.rs +++ b/crates/engine/src/put_item.rs @@ -110,6 +110,16 @@ pub async fn handle_put_item( extenddb_core::validation::AttributeNameRequestForm::SingleItem, )?; + // A number the service cannot read or a value nested too deep in the item + // is reported ahead of the mixed-parameter check, the expression checks, + // the table lookup and the key schema checks (measured 2026-10-27). + // Numbers come first, as before this check; the service's order follows + // the request body, which `Item` does not keep. An attribute name over + // the size limit comes before both: the service answered either fault for + // one item holding both (measured 2026-11-11). + // The later `validate_put_item` repeats both and cannot fail after this. + extenddb_core::validation::number::operations::put_item_item(&input)?; + // Reject mixing legacy and expression parameters, then EAN/EAV supplied // without a referencing expression. let has_expression = input @@ -125,6 +135,15 @@ pub async fn handle_put_item( ], &[("ConditionExpression", has_expression)], )?; + // The ExpressionAttributeValues follow the mixed-parameter check and + // precede the other expression checks and the table lookup (measured + // 2026-10-27). A value nested too deep comes ahead of a malformed number, + // as before this check; the service answered either on two sends. + // The later nesting check repeats this one and cannot fail after it. + extenddb_core::validation::number::operations::put_item_values(&input)?; + // A legacy Expected value's number is read ahead of the condition + // (measured 2026-11-11). + extenddb_core::validation::number::operations::expected_values(input.expected.as_ref())?; extenddb_core::expression::validate_expression_param_usage( input.expression_attribute_names.as_ref(), has_expression, diff --git a/crates/engine/src/transact_get_items.rs b/crates/engine/src/transact_get_items.rs index ec6825cff..84c266373 100755 --- a/crates/engine/src/transact_get_items.rs +++ b/crates/engine/src/transact_get_items.rs @@ -101,6 +101,9 @@ pub async fn handle_transact_get_items( // No projection: transactions accept names with no expression. projections.push(None); } + // A number the service cannot read in the key comes after this item's + // projection checks and before the next item's (measured 2026-10-27). + validation::number::operations::transact_get_item(tgi)?; } // Reject oversized primary keys as a per-item cancellation, matching real diff --git a/crates/engine/src/transact_write_items.rs b/crates/engine/src/transact_write_items.rs index b4388e0ce..f008b022e 100755 --- a/crates/engine/src/transact_write_items.rs +++ b/crates/engine/src/transact_write_items.rs @@ -77,6 +77,15 @@ pub async fn handle_transact_write_items( } } + // The client request token's length and pattern are checked ahead of the + // actions (measured 2026-10-27: a 37-character token is reported ahead of + // a missing table, a condition syntax error and a malformed key number; + // measured 2026-11-11: ahead of an attribute name over the size limit and + // a number the service cannot read). + if let Some(ref token) = input.client_request_token { + validate_client_request_token(token)?; + } + // Attribute names over the size limit are request-shape validation of // each member, run for every member in request order before any member's // table lookup: a request whose first member names a missing table @@ -92,6 +101,13 @@ pub async fn handle_transact_write_items( let mut seen_targets: HashSet = HashSet::with_capacity(input.transact_items.len()); for twi in &input.transact_items { let op = prepare_write_op(twi, ctx).await?; + // A number the service cannot read is checked after this action is + // prepared and before the next one is. Where the action also holds + // another fault, that fault is answered, as before this check; the + // service answers the number there (12 rows measured 2026-10-27). An + // earlier action's condition syntax error or unused value is answered + // ahead of a later action's number, as on the service. + extenddb_core::validation::number::operations::transact_write_item(twi)?; let target_key = op.canonical_target(); if !seen_targets.insert(target_key) { return Err(DynamoDbError::ValidationException( @@ -158,10 +174,6 @@ pub async fn handle_transact_write_items( // The token is passed to the storage layer and checked atomically within // the write transaction, guaranteeing that token storage and data writes // commit together. - if let Some(ref token) = input.client_request_token { - validate_client_request_token(token)?; - } - let idempotency = input .client_request_token diff --git a/crates/engine/src/update_item.rs b/crates/engine/src/update_item.rs index c54a45f1b..3014a3d52 100755 --- a/crates/engine/src/update_item.rs +++ b/crates/engine/src/update_item.rs @@ -72,6 +72,10 @@ pub(crate) async fn handle_update_item_with( let input: UpdateItemInput = serde_json::from_value(body).map_err(crate::deserialize_error)?; extenddb_core::validation::validate_table_name(&input.table_name, &ctx.limits)?; + // A number the service cannot read in the key comes next, ahead of the + // parameter and expression checks, the table lookup and the key schema + // checks (measured 2026-10-27). + extenddb_core::validation::number::operations::update_item_key(&input)?; // An attribute name over the size limit is input validation: it is // reported ahead of the table lookup, the expressions, the condition and @@ -112,6 +116,9 @@ pub(crate) async fn handle_update_item_with( ("ConditionExpression", has_condition), ], )?; + // A legacy Expected value's number is read ahead of the condition + // (measured 2026-11-11). + extenddb_core::validation::number::operations::expected_values(input.expected.as_ref())?; // Desugar legacy AttributeUpdates into UpdateExpression if present. let (effective_update_expr, extra_expr_values, extra_expr_names) = diff --git a/crates/storage-mongodb/src/data_engine.rs b/crates/storage-mongodb/src/data_engine.rs index 10aca1d2b..0cb8992d4 100644 --- a/crates/storage-mongodb/src/data_engine.rs +++ b/crates/storage-mongodb/src/data_engine.rs @@ -50,6 +50,85 @@ fn native_attribute_name(path: &[PathElement], maps: &ExpressionMaps) -> Option< update_create::is_plain_path_name(&attr_name).then(|| attr_name.into_owned()) } +/// The most digits each operand of a numeric `ADD` can have on the native +/// `MongoDB` fast path when both operands are integers. The sum then has at +/// most 34 digits, which a BSON `Decimal128` holds exactly. +const NATIVE_ADD_INTEGER_DIGITS: usize = 33; + +/// The most digits before the decimal point that each operand can have on the +/// native fast path when either operand has a fraction. Twenty digits hold +/// every 64-bit integer. +const NATIVE_ADD_DECIMAL_INTEGER_DIGITS: usize = 20; + +/// The most digits after the decimal point that each operand can have on the +/// native fast path. With [`NATIVE_ADD_DECIMAL_INTEGER_DIGITS`], the aligned +/// sum has at most 21 + 13 = 34 digits. +const NATIVE_ADD_DECIMAL_FRACTION_DIGITS: usize = 13; + +/// A numeric `ADD` delta the native fast path can add, with the stored values +/// it can be added to. +struct NativeAddDelta { + /// The `Decimal128` of the delta's normalized text. + value: bson::Decimal128, + /// The pattern the stored `.N` text must match: a number written without + /// an exponent, within the digit limits that keep the sum exact. + stored: String, +} + +/// The delta of a numeric `ADD` as the native fast path adds it. An integer +/// of at most [`NATIVE_ADD_INTEGER_DIGITS`] digits can be added to a stored +/// integer of at most as many digits. A delta within +/// [`NATIVE_ADD_DECIMAL_INTEGER_DIGITS`] and +/// [`NATIVE_ADD_DECIMAL_FRACTION_DIGITS`] can be added to a stored number +/// within the same limits. Either way the sum is exact, and a nonzero sum is +/// at least 1E-13 and below 2E33 in magnitude, so `DynamoDB` can store it. +/// `None` sends the update to the session path, where the update evaluator +/// range checks both operands and the sum. +/// +/// A `Decimal128` keeps 34 digits and holds magnitudes the service refuses. +/// Without these limits, `GetItem` on `MongoDB` 7 returned +/// `99999999999999999999999999999999990000` after +/// `99999999999999999999999999999999999999 + 1` and +/// `1.000000000000000000000000000000000E+126` after `5E125 + 5E125`, where +/// the service answers `100000000000000000000000000000000000000` and +/// `Number overflow` (measured 2026-11-03). +/// +/// The pipeline writes the sum as `Decimal128` text, which reads normalize: +/// `2.25 + -0.25` is written `2.00`, and a sum below 1E-6 in magnitude can be +/// written with an exponent, such as `1E-7` or `0E-13`. The pattern refuses +/// an exponent, so the next `ADD` to such a value takes the session path, +/// which writes the normalized text. +fn native_add_delta(n: &str) -> Option { + let normalized = extenddb_core::validation::number::validate_and_normalize_number(n).ok()?; + let unsigned = normalized.strip_prefix('-').unwrap_or(&normalized); + let (integer, fraction) = unsigned.split_once('.').unwrap_or((unsigned, "")); + let digits_only = integer + .bytes() + .chain(fraction.bytes()) + .all(|b| b.is_ascii_digit()); + if !digits_only { + return None; + } + let integers = format!("[0-9]{{1,{NATIVE_ADD_INTEGER_DIGITS}}}"); + let decimals = format!( + r"[0-9]{{1,{NATIVE_ADD_DECIMAL_INTEGER_DIGITS}}}(\.[0-9]{{1,{NATIVE_ADD_DECIMAL_FRACTION_DIGITS}}})?" + ); + let fits_decimals = integer.len() <= NATIVE_ADD_DECIMAL_INTEGER_DIGITS + && fraction.len() <= NATIVE_ADD_DECIMAL_FRACTION_DIGITS; + let stored = match (fraction.is_empty(), fits_decimals) { + (true, true) => format!(r"\A-?({integers}|{decimals})\z"), + (true, false) if integer.len() <= NATIVE_ADD_INTEGER_DIGITS => { + format!(r"\A-?{integers}\z") + } + (false, true) => format!(r"\A-?{decimals}\z"), + _ => return None, + }; + Some(NativeAddDelta { + value: normalized.parse().ok()?, + stored, + }) +} + #[derive(Clone, Copy)] enum TransactionKind { ReadOnly, @@ -852,7 +931,9 @@ impl MongoEngine { // Compose the key filter with the type guard. If // the doc exists but fails the guard, findAndModify // returns None and we fall through to the slow - // path (which raises ValidationException). + // path, which raises ValidationException for a + // value that is not a number and adds a number + // outside the native digit limits. // Upsert is disabled here because a missing-doc // "no match" is indistinguishable from a // type-mismatch "no match"; the slow path handles @@ -1661,15 +1742,16 @@ impl MongoEngine { /// - `Some(NativeUpdate::Doc(...))` for a plain operator update /// (`$set`/`$unset`/`$inc`), served by /// `find_one_and_update(filter, doc)`. - /// - `Some(NativeUpdate::Pipeline(...))` for numeric `ADD`, which + /// - `Some(NativeUpdate::Pipeline(...))` for a numeric `ADD` of a delta + /// that `native_add_delta` accepts, which /// requires an aggregation-pipeline update (`$set` with computed /// expressions) to convert the string-stored `.N` value to a /// `Decimal128`, add the delta, and convert back — all /// server-side. - /// - `None` on anything else — set-typed `ADD`, `DELETE`, - /// `list_append`, `if_not_exists`, arithmetic, multi-component - /// paths. Those fall through to the session-scoped - /// read-modify-write path. + /// - `None` on anything else: set-typed `ADD`, a numeric `ADD` of a + /// delta that `native_add_delta` refuses, `DELETE`, `list_append`, + /// `if_not_exists`, arithmetic, multi-component paths. Those fall + /// through to the session-scoped read-modify-write path. /// /// The pipeline form is what makes RFC-0003 §4.4 achievable /// without a numeric shadow field: 50+ concurrent @@ -1682,7 +1764,7 @@ impl MongoEngine { ) -> Option { let mut set_doc = Document::new(); let mut unset_doc = Document::new(); - let mut num_adds: Vec<(String, String)> = Vec::new(); + let mut num_adds: Vec<(String, NativeAddDelta)> = Vec::new(); for action in actions { match action { @@ -1697,13 +1779,11 @@ impl MongoEngine { }; match val { AttributeValue::N(n) => { - // Validate the delta parses as Decimal128 - // up-front so a bad number fails fast - // rather than mid-pipeline on mongo. - if n.parse::().is_err() { - return None; - } - num_adds.push((attr_name, n.clone())); + // Only a delta `native_add_delta` accepts takes + // this path, and the filter below admits only + // the stored values that keep the sum exact and + // in range. + num_adds.push((attr_name, native_add_delta(n)?)); } AttributeValue::SS(_) | AttributeValue::NS(_) | AttributeValue::BS(_) => { // Set ADD — $addToSet would work in theory @@ -1762,27 +1842,28 @@ impl MongoEngine { for k in unset_doc.keys() { stage.insert(k, "$$REMOVE"); } - // Guard: the fast path never reads the pre-image, so we - // can't detect an existing non-numeric attribute (e.g. - // ADD to a string). Require every ADD target to be - // absent or already hold `.N` — else return no match and - // let the caller fall back to the slow path, which reads - // the pre-image and returns a proper ValidationException. + // Guard: the fast path never reads the pre-image, so it cannot + // see an attribute that is not a number (ADD to a string), or a + // stored number the sum would round or take out of range. + // Require every ADD target to be absent or to hold a number + // written without an exponent, within the digit limits that + // keep the sum with its delta exact (`NativeAddDelta::stored`). + // Otherwise the filter matches nothing and the caller falls back + // to the slow path, which reads the pre-image and applies the + // update with the update evaluator: the sum, a type mismatch, or + // the number check's ValidationException. let mut guard_clauses: Vec = Vec::with_capacity(num_adds.len()); - for (attr, delta_s) in &num_adds { + for (attr, delta) in &num_adds { let field = format!("item_data.{attr}.N"); let field_ref = format!("${field}"); let attr_path = format!("item_data.{attr}"); - let delta_dec = delta_s - .parse::() - .expect("validated above"); stage.insert( &field, doc! { "$toString": { "$add": [ { "$toDecimal": { "$ifNull": [ &field_ref, "0" ] } }, - { "$toDecimal": bson::Bson::Decimal128(delta_dec) }, + { "$toDecimal": bson::Bson::Decimal128(delta.value) }, ] } }, @@ -1790,7 +1871,7 @@ impl MongoEngine { guard_clauses.push(doc! { "$or": [ { &attr_path: { "$exists": false } }, - { &field: { "$exists": true } }, + { &field: { "$regex": delta.stored.as_str() } }, ] }); } @@ -3648,12 +3729,14 @@ pub(crate) const TRANSIENT_RETRY_ATTEMPTS: u32 = 50; /// converts a string-stored `.N` value to Decimal128, applies the /// delta, and converts back). /// -/// `Pipeline` carries an optional `type_guard` filter — for numeric -/// ADD we require the target attribute to be absent or already an -/// `.N` so we don't clobber a string with a number. When the guard -/// rejects the match, `find_one_and_update` returns `None`, and the -/// caller falls back to the slow (session-scoped) path which reads -/// the pre-image and surfaces a proper `ValidationException`. +/// `Pipeline` carries an optional `type_guard` filter: for numeric +/// ADD the target attribute must be absent or hold a number within +/// the native digit limits (`NativeAddDelta::stored`), so a string is +/// not clobbered with a number and the sum stays exact. When the +/// guard rejects the match, `find_one_and_update` returns `None`, and +/// the caller falls back to the slow (session-scoped) path, which +/// reads the pre-image and either applies the update with the update +/// evaluator or surfaces a `ValidationException`. enum NativeUpdate { Doc(Document), Pipeline { @@ -4614,4 +4697,110 @@ transaction_read_concern = "majority""#, Some(&bson::Bson::Document(want_plain)) ); } + + /// `ADD n :v` with `:v` set to `delta`, built for the native path. + fn native_add(delta: &str) -> Option { + let values = std::collections::HashMap::from([( + "v".to_owned(), + AttributeValue::N(delta.to_owned()), + )]); + let maps = ExpressionMaps::new(std::collections::HashMap::new(), values); + let tokens = expression::tokenize("ADD n :v").unwrap(); + let actions = expression::parse_update(&tokens).unwrap(); + MongoEngine::try_build_native_update(&actions, &maps) + } + + /// A delta takes the native path when its normalized text is an integer + /// of at most 33 digits, or has at most 20 digits before the decimal + /// point and 13 after it. With the filter on the stored value (see the + /// next test), the `Decimal128` sum is then exact and in range. A longer + /// number, a number out of range and a value that is not a number go to + /// the session path, where the update evaluator checks the operands and + /// the sum (see `native_add_delta` for what `GetItem` returned without + /// these limits). + #[test] + fn native_add_takes_only_a_delta_that_can_keep_the_sum_exact() { + let integer = "9".repeat(33); + let widest = format!("{}.{}", "9".repeat(20), "9".repeat(13)); + for delta in [ + "1", + "-1", + "0", + "1E3", + "+7", + "0.5", + "-0.25", + "2.50", + "1E-13", + "1E20", + integer.as_str(), + widest.as_str(), + ] { + assert!( + matches!(native_add(delta), Some(NativeUpdate::Pipeline { .. })), + "{delta}" + ); + } + let long_integer = "1".repeat(34); + let long_whole = format!("{}.5", "1".repeat(21)); + for delta in [ + "5E125", + "1E33", + long_integer.as_str(), + long_whole.as_str(), + "1E-14", + "0.12345678901234", + "1E126", + "abc", + "", + ] { + assert!(native_add(delta).is_none(), "{delta}"); + } + } + + /// The filter of the native pipeline for `delta`. + fn native_add_guard(delta: &str) -> bson::Bson { + let Some(NativeUpdate::Pipeline { type_guard, .. }) = native_add(delta) else { + panic!("{delta} builds a pipeline update"); + }; + let guard = type_guard.expect("an ADD target has a guard"); + guard.get_array("$or").unwrap()[1].clone() + } + + /// The pipeline adds the delta's normalized text. Its filter matches only + /// an absent attribute or a stored number written without an exponent, + /// within the limits that keep the sum with this delta exact: up to 33 + /// integer digits when both operands are integers, and up to 20 digits + /// before the decimal point and 13 after it when either has a fraction. + #[test] + fn native_add_adds_the_normalized_delta_and_guards_the_stored_number() { + let Some(NativeUpdate::Pipeline { pipeline, .. }) = native_add("1E3") else { + panic!("a delta within the limits builds a pipeline update"); + }; + let stage = pipeline[0].get_document("$set").unwrap(); + let add = stage + .get_document("item_data.n.N") + .and_then(|d| d.get_document("$toString")) + .and_then(|d| d.get_array("$add")) + .unwrap(); + let want: bson::Decimal128 = "1000".parse().unwrap(); + assert_eq!( + add[1], + bson::Bson::Document(doc! { "$toDecimal": bson::Bson::Decimal128(want) }) + ); + let integers = r"[0-9]{1,33}"; + let decimals = r"[0-9]{1,20}(\.[0-9]{1,13})?"; + let long_integer = "1".repeat(21); + for (delta, stored) in [ + ("1E3", format!(r"\A-?({integers}|{decimals})\z")), + (long_integer.as_str(), format!(r"\A-?{integers}\z")), + ("0.5", format!(r"\A-?{decimals}\z")), + ] { + assert_eq!( + native_add_guard(delta), + bson::Bson::Document(doc! { "item_data.n.N": { "$regex": stored } }), + "{delta}" + ); + } + } } diff --git a/docs/differences-from-dynamodb.md b/docs/differences-from-dynamodb.md index 91934e489..4ce144747 100755 --- a/docs/differences-from-dynamodb.md +++ b/docs/differences-from-dynamodb.md @@ -45,6 +45,7 @@ from a request sent to that service or backend. | Export formats | DYNAMODB_JSON, ION | DYNAMODB_JSON, ION | | Import execution | Asynchronous (background job) | Synchronous (completes before returning) | | Export execution | Point-in-time snapshot | Current snapshot, synchronous | +| Imported rows holding a number the service cannot store | A row holding a value that is not a number ends the import of the file: it and the rows after it are not imported. A row holding a number out of range or with more than 38 digits is skipped, and the rows after it are imported. Each refused row counts as one error and is logged to CloudWatch, and an import with any error is `FAILED` with `ItemValidationError`, keeping the rows it imported (measured 2026-11-03). Unlike the item operations, it reads a number's exponent and scale as 32-bit integers: `1E2147483648` ends the import of the file and `1E2147483647` is a `Number underflow` (measured again 2026-11-11) | The same, with three differences. A zero whose exponent fits in 32 bits but is out of range, such as `0E126`, is imported as `0`, where the service skips the row (`Number overflow` for `0E126`). A Unicode decimal digit (`\uff15`), which the service imports as the digit (`5`), is not a number to ExtendDB, so the row ends the import of the file. Each refused row is logged to the server log, and the `FailureMessage` names the server log instead of CloudWatch | ## Control Plane diff --git a/tests/rust/src/main.rs b/tests/rust/src/main.rs index 965d44837..ef9c97089 100755 --- a/tests/rust/src/main.rs +++ b/tests/rust/src/main.rs @@ -66,6 +66,10 @@ mod misc_control_plane; #[cfg(test)] mod number_validation; #[cfg(test)] +mod number_validation_all_ops; +#[cfg(test)] +mod number_validation_order; +#[cfg(test)] mod put_item; #[cfg(test)] mod put_item_edge; @@ -106,6 +110,8 @@ mod ttl; #[cfg(test)] mod unicode; #[cfg(test)] +mod update_arithmetic_result; +#[cfg(test)] mod update_expressions; #[cfg(test)] mod update_expressions_more; diff --git a/tests/rust/src/number_validation_all_ops.rs b/tests/rust/src/number_validation_all_ops.rs new file mode 100644 index 000000000..4257a770e --- /dev/null +++ b/tests/rust/src/number_validation_all_ops.rs @@ -0,0 +1,778 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Number (`N`) validation on the operations that did not read a number at +//! all (#618): the keys of `GetItem`, `DeleteItem`, `UpdateItem`, +//! `BatchGetItem` and `TransactGetItems`, the items and keys of +//! `BatchWriteItem` and `TransactWriteItems`, the +//! `ExpressionAttributeValues` of `PutItem`, `DeleteItem` and +//! `TransactWriteItems`, and the legacy `Expected` values of `PutItem`, +//! `DeleteItem` and `UpdateItem`. +//! +//! Every message below was measured against DynamoDB on 2026-10-27 +//! (us-east-1), each request sent twice with the same answer both times, +//! except where a test gives another date. +//! `PutItem`, `UpdateItem` and `DeleteItem` start the message with +//! "1 validation error detected: " and the other operations do not. +//! `TransactWriteItems` answers a value in `ExpressionAttributeValues` with +//! the number message alone (measured 2026-11-11; on 2026-10-27 the service +//! wrapped it as "ExpressionAttributeValues contains invalid value: +//! for key :v"). +//! More tests of the order of these checks against other faults in the same +//! request are in `number_validation_order`, which shares the helpers below. + +use std::collections::HashMap; + +use aws_sdk_dynamodb::error::ProvideErrorMetadata; +use aws_sdk_dynamodb::types::{ + AttributeAction, AttributeValue, AttributeValueUpdate, ComparisonOperator, ConditionCheck, + Delete, DeleteRequest, ExpectedAttributeValue, Get, KeysAndAttributes, Put, PutRequest, + TransactGetItem, TransactWriteItem, Update, WriteRequest, +}; +use aws_smithy_runtime_api::client::result::SdkError; + +use crate::test_base::*; + +pub(crate) type Key = HashMap; + +pub(crate) const NUMERIC: &str = "The parameter cannot be converted to a numeric value: abc"; +const OVERFLOW: &str = + "Number overflow. Attempting to store a number with magnitude larger than supported range"; +/// A number whose magnitude is over the supported range. +const OVER_RANGE: &str = "1E126"; +/// The prefix `PutItem`, `UpdateItem` and `DeleteItem` put on a message. +const PREFIX: &str = "1 validation error detected: "; +const CONDITION: &str = "attribute_not_exists(amount) OR amount = :v"; + +pub(crate) fn n_raw(value: &str) -> AttributeValue { + AttributeValue::N(value.to_string()) +} + +/// A key of `comp_key_number_string`, whose hash key is an `N`. +pub(crate) fn number_key(value: &str) -> Key { + HashMap::from([ + (HASH_KEY_N.to_string(), n_raw(value)), + (RANGE_KEY_S.to_string(), s("x")), + ]) +} + +/// A key of `simple_key_string`, whose hash key is an `S`. +pub(crate) fn string_key(name: &str) -> Key { + HashMap::from([(HASH_KEY_S.to_string(), s(&format!("{name}_{}", ts())))]) +} + +/// An error as "Code:message". +fn describe(err: &SdkError) -> String +where + E: ProvideErrorMetadata, +{ + format!("{}:{}", err_code(err).unwrap_or("None"), err_msg(err)) +} + +/// The error a request failed with; panics when it succeeded. +pub(crate) fn refused(result: Result>) -> String +where + E: ProvideErrorMetadata, +{ + match result { + Ok(_) => panic!("the request must be refused"), + Err(err) => describe(&err), + } +} + +fn assert_refused(err: &str, message: &str) { + assert_eq!(err, format!("ValidationException:{message}")); +} + +pub(crate) fn put_request(item: Key) -> WriteRequest { + let put = PutRequest::builder().set_item(Some(item)).build(); + WriteRequest::builder() + .put_request(put.expect("put")) + .build() +} + +fn delete_request(key: Key) -> WriteRequest { + let delete = DeleteRequest::builder().set_key(Some(key)).build(); + WriteRequest::builder() + .delete_request(delete.expect("delete")) + .build() +} + +pub(crate) async fn batch_write(table: &str, requests: Vec) -> Result<(), String> { + let result = client() + .batch_write_item() + .request_items(table, requests) + .send() + .await; + result.map(|_| ()).map_err(|e| describe(&e)) +} + +pub(crate) async fn transact_write(items: Vec) -> Result<(), String> { + let result = client() + .transact_write_items() + .set_transact_items(Some(items)) + .send() + .await; + result.map(|_| ()).map_err(|e| describe(&e)) +} + +fn txn_put(put: Put) -> TransactWriteItem { + TransactWriteItem::builder().put(put).build() +} + +pub(crate) fn txn_delete(delete: Delete) -> TransactWriteItem { + TransactWriteItem::builder().delete(delete).build() +} + +fn txn_update(update: Update) -> TransactWriteItem { + TransactWriteItem::builder().update(update).build() +} + +pub(crate) fn txn_check(check: ConditionCheck) -> TransactWriteItem { + TransactWriteItem::builder().condition_check(check).build() +} + +fn put_of(table: &str, item: Key) -> Put { + let put = Put::builder().table_name(table).set_item(Some(item)); + put.build().expect("put") +} + +pub(crate) fn delete_of(table: &str, key: Key) -> Delete { + let delete = Delete::builder().table_name(table).set_key(Some(key)); + delete.build().expect("delete") +} + +// ---- keys ---- + +#[tokio::test] +async fn get_item_rejects_a_malformed_number_in_the_key() { + let t = tables().await; + let result = client() + .get_item() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .send() + .await; + assert_refused(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn delete_item_rejects_a_malformed_number_in_the_key() { + let t = tables().await; + let result = client() + .delete_item() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{NUMERIC}")); +} + +#[tokio::test] +async fn update_item_rejects_a_malformed_number_in_the_key() { + let t = tables().await; + let result = client() + .update_item() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .update_expression("SET a = :v") + .expression_attribute_values(":v", s("ok")) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{NUMERIC}")); +} + +#[tokio::test] +async fn batch_get_item_rejects_a_malformed_number_in_a_key() { + let t = tables().await; + let keys = KeysAndAttributes::builder() + .keys(number_key("abc")) + .build() + .expect("keys"); + let result = client() + .batch_get_item() + .request_items(&t.comp_key_number_string, keys) + .send() + .await; + assert_refused(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn transact_get_items_rejects_a_malformed_number_in_a_key() { + let t = tables().await; + let get = Get::builder() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .build() + .expect("get"); + let result = client() + .transact_get_items() + .transact_items(TransactGetItem::builder().get(get).build()) + .send() + .await; + assert_refused(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn batch_write_item_rejects_a_malformed_number_in_a_delete_key() { + let t = tables().await; + let err = batch_write( + &t.comp_key_number_string, + vec![delete_request(number_key("abc"))], + ) + .await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_malformed_number_in_a_delete_key() { + let t = tables().await; + let delete = delete_of(&t.comp_key_number_string, number_key("abc")); + let err = transact_write(vec![txn_delete(delete)]).await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_malformed_number_in_an_update_key() { + let t = tables().await; + let update = Update::builder() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .update_expression("SET a = :v") + .expression_attribute_values(":v", s("ok")) + .build() + .expect("update"); + let err = transact_write(vec![txn_update(update)]).await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_malformed_number_in_a_condition_check_key() { + let t = tables().await; + let check = ConditionCheck::builder() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .condition_expression("attribute_not_exists(a)") + .build() + .expect("check"); + let err = transact_write(vec![txn_check(check)]).await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +// ---- items ---- + +#[tokio::test] +async fn batch_write_item_rejects_a_malformed_number_in_a_put_item() { + let t = tables().await; + let mut item = string_key("b618"); + item.insert("amount".to_string(), n_raw("abc")); + let err = batch_write(&t.simple_key_string, vec![put_request(item)]).await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +#[tokio::test] +async fn batch_write_item_rejects_a_number_set_member_out_of_range() { + let t = tables().await; + let mut item = string_key("b618"); + let members = vec!["1".to_string(), "1E126".to_string()]; + item.insert("amounts".to_string(), AttributeValue::Ns(members)); + let err = batch_write(&t.simple_key_string, vec![put_request(item)]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); +} + +#[tokio::test] +async fn batch_write_item_rejects_a_malformed_number_sort_key() { + // Answered InternalServerError before #618. + let t = tables().await; + let item = HashMap::from([ + (HASH_KEY_S.to_string(), s("b618")), + (RANGE_KEY_N.to_string(), n_raw("abc")), + ]); + let err = batch_write(&t.comp_key_string_number, vec![put_request(item)]).await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_malformed_number_in_a_put_item() { + let t = tables().await; + let mut item = string_key("t618"); + item.insert("amount".to_string(), n_raw("abc")); + let err = transact_write(vec![txn_put(put_of(&t.simple_key_string, item))]).await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_malformed_number_sort_key() { + // Answered InternalServerError before #618. + let t = tables().await; + let item = HashMap::from([ + (HASH_KEY_S.to_string(), s("t618")), + (RANGE_KEY_N.to_string(), n_raw("abc")), + ]); + let err = transact_write(vec![txn_put(put_of(&t.comp_key_string_number, item))]).await; + assert_refused(&err.unwrap_err(), NUMERIC); +} + +// ---- ExpressionAttributeValues ---- + +#[tokio::test] +async fn put_item_rejects_a_condition_value_out_of_range() { + let t = tables().await; + let result = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(string_key("p618"))) + .condition_expression(CONDITION) + .expression_attribute_values(":v", n_raw(OVER_RANGE)) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{OVERFLOW}")); +} + +#[tokio::test] +async fn delete_item_rejects_a_condition_value_out_of_range() { + let t = tables().await; + let result = client() + .delete_item() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("d618"))) + .condition_expression(CONDITION) + .expression_attribute_values(":v", n_raw(OVER_RANGE)) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{OVERFLOW}")); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_condition_check_value_out_of_range() { + // The four TransactWriteItems value tests: measured 2026-11-11 with + // `1E126`, the number message with no prefix and no wrapper. + let t = tables().await; + let check = ConditionCheck::builder() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("t618"))) + .condition_expression(CONDITION) + .expression_attribute_values(":v", n_raw(OVER_RANGE)) + .build() + .expect("check"); + let err = transact_write(vec![txn_check(check)]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_put_condition_value_out_of_range() { + let t = tables().await; + let put = Put::builder() + .table_name(&t.simple_key_string) + .set_item(Some(string_key("t618"))) + .condition_expression(CONDITION) + .expression_attribute_values(":v", n_raw(OVER_RANGE)) + .build() + .expect("put"); + let err = transact_write(vec![txn_put(put)]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); +} + +#[tokio::test] +async fn transact_write_items_rejects_an_update_value_out_of_range() { + let t = tables().await; + let update = Update::builder() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("t618"))) + .update_expression("SET amount = :v") + .expression_attribute_values(":v", n_raw(OVER_RANGE)) + .build() + .expect("update"); + let err = transact_write(vec![txn_update(update)]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); +} + +#[tokio::test] +async fn transact_write_items_rejects_a_delete_condition_value_out_of_range() { + let t = tables().await; + let delete = Delete::builder() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("t618"))) + .condition_expression(CONDITION) + .expression_attribute_values(":v", n_raw(OVER_RANGE)) + .build() + .expect("delete"); + let err = transact_write(vec![txn_delete(delete)]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); +} + +// ---- legacy Expected ---- + +fn expected_value(value: &str) -> ExpectedAttributeValue { + ExpectedAttributeValue::builder() + .value(n_raw(value)) + .build() +} + +/// The three values measured in `Expected` on 2026-11-11, with the message +/// each one answers after the prefix. An empty N answers the message with +/// nothing after the colon. +fn expected_rows() -> [(&'static str, &'static str); 3] { + [ + ("abc", NUMERIC), + ("1E2147483648", OVERFLOW), + ("", "The parameter cannot be converted to a numeric value: "), + ] +} + +#[tokio::test] +async fn put_item_rejects_a_number_in_a_legacy_expected_value_or_list() { + // Measured 2026-11-11 on an item that does not exist: a + // ValidationException with the prefix, not ConditionalCheckFailedException. + let t = tables().await; + for (value, message) in expected_rows() { + let result = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(string_key("e618"))) + .expected("amount", expected_value(value)) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{message}")); + + let eq = ExpectedAttributeValue::builder() + .comparison_operator(ComparisonOperator::Eq) + .attribute_value_list(n_raw(value)) + .build(); + let result = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(string_key("e618"))) + .expected("amount", eq) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{message}")); + } +} + +#[tokio::test] +async fn delete_and_update_item_reject_a_number_in_a_legacy_expected_value() { + // Measured 2026-11-11 on items that do not exist. + let t = tables().await; + for (value, message) in expected_rows() { + let result = client() + .delete_item() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("e618"))) + .expected("amount", expected_value(value)) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{message}")); + + let put_a = AttributeValueUpdate::builder() + .action(AttributeAction::Put) + .value(s("x")) + .build(); + let result = client() + .update_item() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("e618"))) + .attribute_updates("amount", put_a) + .expected("amount", expected_value(value)) + .send() + .await; + assert_refused(&refused(result), &format!("{PREFIX}{message}")); + } +} + +#[tokio::test] +async fn a_legacy_expected_number_in_range_reaches_the_condition() { + // Measured 2026-11-11: `1E125` in the same PutItem request fails the + // condition, since the item does not exist. + let t = tables().await; + let result = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(string_key("e618"))) + .expected("amount", expected_value("1E125")) + .send() + .await; + assert_eq!( + refused(result), + "ConditionalCheckFailedException:The conditional request failed" + ); +} + +// ---- behaviour around the checks ---- + +#[tokio::test] +async fn batch_write_item_writes_nothing_when_a_later_put_is_refused() { + let t = tables().await; + let good = string_key("b618_good"); + let mut bad = string_key("b618_bad"); + bad.insert("amount".to_string(), n_raw("abc")); + let requests = vec![put_request(good.clone()), put_request(bad)]; + let err = batch_write(&t.simple_key_string, requests).await; + assert_refused(&err.unwrap_err(), NUMERIC); + + let stored = client() + .get_item() + .table_name(&t.simple_key_string) + .set_key(Some(good)) + .consistent_read(true) + .send() + .await + .expect("GetItem"); + assert!( + stored.item().is_none(), + "the earlier put must not be written" + ); +} + +#[tokio::test] +async fn transact_write_items_reports_an_earlier_actions_unused_value_first() { + // Measured 2026-10-27: an unused value in the first action is reported + // ahead of a malformed number in the second. + let t = tables().await; + let check = ConditionCheck::builder() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("t618"))) + .condition_expression("attribute_not_exists(amount)") + .expression_attribute_values(":w", s("x")) + .build() + .expect("check"); + let delete = delete_of(&t.comp_key_number_string, number_key("abc")); + let err = transact_write(vec![txn_check(check), txn_delete(delete)]).await; + assert_refused( + &err.unwrap_err(), + "Value provided in ExpressionAttributeValues unused in expressions: keys: {:w}", + ); +} + +#[tokio::test] +async fn transact_write_items_reports_a_long_client_request_token_first() { + // Measured 2026-10-27: a 37-character token is reported ahead of a + // malformed key number. + let t = tables().await; + let delete = delete_of(&t.comp_key_number_string, number_key("abc")); + let result = client() + .transact_write_items() + .transact_items(txn_delete(delete)) + .client_request_token("t".repeat(37)) + .send() + .await; + crate::number_validation_order::assert_token_refused(&refused(result)); +} + +#[tokio::test] +async fn transact_get_items_reports_an_unused_name_ahead_of_the_key_number() { + // Measured 2026-10-27: the item's projection checks come first. + let t = tables().await; + let get = Get::builder() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .projection_expression("a") + .expression_attribute_names("#u", "u") + .build() + .expect("get"); + let result = client() + .transact_get_items() + .transact_items(TransactGetItem::builder().get(get).build()) + .send() + .await; + assert_refused( + &refused(result), + "Value provided in ExpressionAttributeNames unused in expressions: keys: {#u}", + ); +} + +#[tokio::test] +async fn batch_get_item_reports_duplicate_keys_ahead_of_their_number() { + // Measured 2026-10-27. + let t = tables().await; + let keys = KeysAndAttributes::builder() + .keys(number_key("abc")) + .keys(number_key("abc")) + .build() + .expect("keys"); + let result = client() + .batch_get_item() + .request_items(&t.comp_key_number_string, keys) + .send() + .await; + assert_refused( + &refused(result), + "Provided list of item keys contains duplicates", + ); +} + +#[tokio::test] +async fn a_number_over_the_range_is_refused_in_get_and_delete_keys() { + // The GetItem key, a BatchWriteItem delete key and a TransactWriteItems + // delete key. + let t = tables().await; + let table = &t.comp_key_number_string; + let get = client() + .get_item() + .table_name(table) + .set_key(Some(number_key(OVER_RANGE))) + .send() + .await; + assert_refused(&refused(get), OVERFLOW); + + let err = batch_write(table, vec![delete_request(number_key(OVER_RANGE))]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); + + let err = transact_write(vec![txn_delete(delete_of(table, number_key(OVER_RANGE)))]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); +} + +#[tokio::test] +async fn put_batch_and_transact_paths_read_a_64_bit_exponent() { + // Measured 2026-11-11, each value twice on each path: `10E2147483647` and + // `1E2147483648` are a `Number overflow` on PutItem, BatchWriteItem, + // TransactWriteItems and TransactGetItems. DynamoDB puts the prefix on + // the PutItem message and ExtendDB does not, so PutItem is matched on + // the message body. + let t = tables().await; + for value in ["10E2147483647", "1E2147483648"] { + let mut item = string_key("x618"); + item.insert("amount".to_string(), n_raw(value)); + let put = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(item.clone())) + .send() + .await; + let err = refused(put); + assert!( + err.starts_with("ValidationException:") && err.ends_with(OVERFLOW), + "PutItem {value}: {err}" + ); + + let err = batch_write(&t.simple_key_string, vec![put_request(item.clone())]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); + + let err = transact_write(vec![txn_put(put_of(&t.simple_key_string, item))]).await; + assert_refused(&err.unwrap_err(), OVERFLOW); + + let get = Get::builder() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key(value))) + .build() + .expect("get"); + let result = client() + .transact_get_items() + .transact_items(TransactGetItem::builder().get(get).build()) + .send() + .await; + assert_refused(&refused(result), OVERFLOW); + } +} + +#[tokio::test] +async fn an_empty_number_answers_with_the_colon_except_on_batch_write_item() { + // Measured 2026-11-11, each request twice. The message ends with a colon + // and a space on GetItem, BatchGetItem, DeleteItem, UpdateItem, + // TransactGetItems and TransactWriteItems keys and on a TransactWriteItems + // Put item value. BatchWriteItem keys, item values and NS members answer + // without the colon. TransactGetItems and TransactWriteItems keys and + // items had no colon on 2026-10-27. + let t = tables().await; + let table = &t.comp_key_number_string; + let with_colon = "The parameter cannot be converted to a numeric value: "; + let without_colon = "The parameter cannot be converted to a numeric value"; + + let get = client() + .get_item() + .table_name(table) + .set_key(Some(number_key(""))) + .send() + .await; + assert_refused(&refused(get), with_colon); + + let keys = KeysAndAttributes::builder() + .keys(number_key("")) + .build() + .expect("keys"); + let batch_get = client() + .batch_get_item() + .request_items(table, keys) + .send() + .await; + assert_refused(&refused(batch_get), with_colon); + + let delete = client() + .delete_item() + .table_name(table) + .set_key(Some(number_key(""))) + .send() + .await; + assert_refused(&refused(delete), &format!("{PREFIX}{with_colon}")); + + let update = client() + .update_item() + .table_name(table) + .set_key(Some(number_key(""))) + .update_expression("SET a = :a") + .expression_attribute_values(":a", s("x")) + .send() + .await; + assert_refused(&refused(update), &format!("{PREFIX}{with_colon}")); + + let get = Get::builder() + .table_name(table) + .set_key(Some(number_key(""))) + .build() + .expect("get"); + let transact_get = client() + .transact_get_items() + .transact_items(TransactGetItem::builder().get(get).build()) + .send() + .await; + assert_refused(&refused(transact_get), with_colon); + + let err = transact_write(vec![txn_delete(delete_of(table, number_key("")))]).await; + assert_refused(&err.unwrap_err(), with_colon); + + let mut item = string_key("e618"); + item.insert("amount".to_string(), n_raw("")); + let put = put_of(&t.simple_key_string, item.clone()); + let err = transact_write(vec![txn_put(put)]).await; + assert_refused(&err.unwrap_err(), with_colon); + + let err = batch_write(table, vec![delete_request(number_key(""))]).await; + assert_refused(&err.unwrap_err(), without_colon); + + let err = batch_write(&t.simple_key_string, vec![put_request(item)]).await; + assert_refused(&err.unwrap_err(), without_colon); + + let mut item = string_key("e618"); + let members = vec!["1".to_string(), String::new()]; + item.insert("amounts".to_string(), AttributeValue::Ns(members)); + let err = batch_write(&t.simple_key_string, vec![put_request(item)]).await; + assert_refused(&err.unwrap_err(), without_colon); +} + +#[tokio::test] +async fn a_zero_inside_the_range_is_stored_as_zero_on_batch_and_transact_writes() { + // Measured 2026-10-27: `0.0E126` and `-0E-130` are stored as 0. + let t = tables().await; + for (writer, value) in [("batch", "0.0E126"), ("transact", "-0E-130")] { + let key = string_key("z618"); + let mut item = key.clone(); + item.insert("amount".to_string(), n_raw(value)); + let outcome = if writer == "batch" { + batch_write(&t.simple_key_string, vec![put_request(item)]).await + } else { + transact_write(vec![txn_put(put_of(&t.simple_key_string, item))]).await + }; + outcome.expect("a zero inside the range is accepted"); + + let stored = client() + .get_item() + .table_name(&t.simple_key_string) + .set_key(Some(key)) + .consistent_read(true) + .send() + .await + .expect("GetItem"); + let amount = stored.item().and_then(|item| item.get("amount")); + assert_eq!(amount, Some(&n_raw("0")), "{writer} write of {value}"); + } +} diff --git a/tests/rust/src/number_validation_order.rs b/tests/rust/src/number_validation_order.rs new file mode 100644 index 000000000..40d718841 --- /dev/null +++ b/tests/rust/src/number_validation_order.rs @@ -0,0 +1,368 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The order of the #618 number checks against another fault in the same +//! request. Each test sends a request holding a malformed number and one +//! other fault, and expects the fault DynamoDB reported first. Every +//! expectation is a row of the measured service answers (us-east-1, each +//! request sent twice with the same answer both times), named in the test's +//! comment: 2026-10-27, and 2026-11-11 for the rows with an attribute name +//! over the size limit. For an item that holds such a name and a malformed +//! number, the service gave either answer to the same request, and those +//! tests accept either. +//! +//! The tests check that the answer contains the expected message rather than +//! equals it: DynamoDB puts `1 validation error detected: ` on the PutItem +//! item and mixed-parameter messages, and ExtendDB does not, which is a known +//! difference these tests leave alone. + +use aws_sdk_dynamodb::types::{ + ConditionCheck, ExpectedAttributeValue, Get, KeysAndAttributes, Put, TransactGetItem, + TransactWriteItem, +}; + +use crate::number_validation_all_ops::{ + batch_write, delete_of, n_raw, number_key, put_request, refused, string_key, transact_write, + txn_check, txn_delete, Key, NUMERIC, +}; +use crate::test_base::*; + +const MIXED: &str = "Can not use both expression and non-expression parameters in the same request"; + +/// Assert a `ValidationException` whose message contains `text`. +fn assert_refused_with(err: &str, text: &str) { + assert!( + err.starts_with("ValidationException:") && err.contains(text), + "expected a ValidationException containing {text:?}, got {err}" + ); +} + +fn missing_table() -> String { + format!("Absent618{}", ts()) +} + +fn not_exists() -> ExpectedAttributeValue { + ExpectedAttributeValue::builder().exists(false).build() +} + +/// An item of `simple_key_string` holding a malformed number. +fn item_with_a_malformed_number(name: &str) -> Key { + let mut item = string_key(name); + item.insert("n".to_string(), n_raw("abc")); + item +} + +#[tokio::test] +async fn get_item_reports_the_key_number_ahead_of_an_unused_name() { + // "GetItem EAN without projection + key pk abc": the number. + let t = tables().await; + let result = client() + .get_item() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .expression_attribute_names("#u", "u") + .send() + .await; + assert_refused_with(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn delete_item_reports_the_key_number_ahead_of_the_mixed_parameter_error() { + // "DeleteItem Expected + ConditionExpression + key pk abc": the number. + let t = tables().await; + let result = client() + .delete_item() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .expected("a", not_exists()) + .condition_expression("attribute_not_exists(b)") + .send() + .await; + assert_refused_with(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn delete_item_reports_the_mixed_parameter_error_ahead_of_value_numbers() { + // "DeleteItem Expected + CE + EAV abc": the mixed-parameter error. + let t = tables().await; + let result = client() + .delete_item() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("o618"))) + .expected("a", not_exists()) + .condition_expression("attribute_not_exists(b) OR b = :v") + .expression_attribute_values(":v", n_raw("abc")) + .send() + .await; + assert_refused_with(&refused(result), MIXED); +} + +#[tokio::test] +async fn update_item_reports_the_key_number_ahead_of_the_mixed_parameter_error() { + // "UpdateExpression + Expected + key pk abc" and "ConditionExpression + + // Expected + key pk abc": the number on both. + let t = tables().await; + let result = client() + .update_item() + .table_name(&t.comp_key_number_string) + .set_key(Some(number_key("abc"))) + .update_expression("SET a = :s") + .expression_attribute_values(":s", s("x")) + .expected("x", not_exists()) + .send() + .await; + assert_refused_with(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn put_item_reports_the_item_number_ahead_of_an_absent_table() { + // "PutItem absent table + item n abc": the number. + let result = client() + .put_item() + .table_name(missing_table()) + .set_item(Some(item_with_a_malformed_number("o618"))) + .send() + .await; + assert_refused_with(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn put_item_reports_the_item_number_ahead_of_the_mixed_parameter_error() { + // "PutItem Expected + CE + item n abc": the number. + let t = tables().await; + let result = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(item_with_a_malformed_number("o618"))) + .expected("a", not_exists()) + .condition_expression("attribute_not_exists(b)") + .send() + .await; + assert_refused_with(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn put_item_reports_the_mixed_parameter_error_ahead_of_value_numbers() { + // "PutItem Expected + ConditionExpression + EAV abc": the mixed-parameter + // error. + let t = tables().await; + let result = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(string_key("o618"))) + .expected("a", not_exists()) + .condition_expression("attribute_not_exists(b) OR b = :v") + .expression_attribute_values(":v", n_raw("abc")) + .send() + .await; + assert_refused_with(&refused(result), MIXED); +} + +#[tokio::test] +async fn batch_get_item_reports_a_key_number_ahead_of_an_absent_table() { + // "BatchGetItem absent-table", one table with the key pk abc: the number. + // One table, so the answer does not depend on the order of RequestItems. + let keys = KeysAndAttributes::builder() + .keys(number_key("abc")) + .build() + .expect("keys"); + let result = client() + .batch_get_item() + .request_items(missing_table(), keys) + .send() + .await; + assert_refused_with(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn batch_write_item_reports_a_put_number_ahead_of_an_absent_table() { + // "BatchWriteItem put absent-table", one table with a put holding n abc: + // the number. + let put = put_request(item_with_a_malformed_number("o618")); + let err = batch_write(&missing_table(), vec![put]).await; + assert_refused_with(&err.unwrap_err(), NUMERIC); +} + +#[tokio::test] +async fn transact_get_items_reports_an_earlier_key_number_ahead_of_a_later_projection() { + // "TransactGetItems get1 key pk abc + get2 projection syntax": the number. + let t = tables().await; + let get = |key: Key, projection: Option<&str>| { + let get = Get::builder() + .table_name(&t.comp_key_number_string) + .set_key(Some(key)) + .set_projection_expression(projection.map(str::to_string)); + TransactGetItem::builder() + .get(get.build().expect("get")) + .build() + }; + let result = client() + .transact_get_items() + .transact_items(get(number_key("abc"), None)) + .transact_items(get(number_key("2"), Some(",,b"))) + .send() + .await; + assert_refused_with(&refused(result), NUMERIC); +} + +#[tokio::test] +async fn transact_write_items_reports_an_earlier_key_number_ahead_of_a_later_condition_syntax() { + // "TransactWriteItems item1 key pk abc + item2 condition syntax": the + // number. + let t = tables().await; + let first = delete_of(&t.comp_key_number_string, number_key("abc")); + let second = ConditionCheck::builder() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("o618"))) + .condition_expression("a = = :w") + .expression_attribute_values(":w", s("x")) + .build() + .expect("check"); + let err = transact_write(vec![txn_delete(first), txn_check(second)]).await; + assert_refused_with(&err.unwrap_err(), NUMERIC); +} + +// ---- an attribute name over the size limit (measured 2026-11-11) ---- + +/// The size refusal for a 65,536-byte attribute name in an item. +const LONG_NAME: &str = "Attribute name is too large, must be less than 65536 bytes"; +/// The answer for `1E2147483648`. +const OVERFLOW: &str = + "Number overflow. Attempting to store a number with magnitude larger than supported range"; +/// A 37-character `ClientRequestToken`, one over the limit. +const TOKEN_TEXT: &str = + "failed to satisfy constraint: Member must have length less than or equal to 36"; + +/// An attribute name of 65,536 bytes, one over the limit. +fn long_name() -> String { + "a".repeat(65_536) +} + +/// An item of `simple_key_string` holding a 65,536-byte attribute name and, +/// when `number` is set, the number `1E2147483648`. +fn item_with_a_long_name(name: &str, number: bool) -> Key { + let mut item = string_key(name); + item.insert(long_name(), s("x")); + if number { + item.insert("n".to_string(), n_raw("1E2147483648")); + } + item +} + +/// An item of `simple_key_string` holding the number `1E2147483648`. +fn item_with_an_overflow(name: &str) -> Key { + let mut item = string_key(name); + item.insert("n".to_string(), n_raw("1E2147483648")); + item +} + +/// Assert a `ValidationException` whose message ends with the name refusal +/// or with the overflow, the two answers the service gave. +fn assert_name_or_overflow(err: &str) { + assert!( + err.starts_with("ValidationException:") + && (err.ends_with(LONG_NAME) || err.ends_with(OVERFLOW)), + "expected the name refusal or the overflow, got {}", + &err[..err.len().min(300)] + ); +} + +/// Assert the refusal of a 37-character `ClientRequestToken`. The service's +/// message names the member `ClientRequestToken`, measured twice on +/// 2026-11-11 (06:34 and 08:25 UTC), where it named it `clientRequestToken` +/// on 2026-10-27. ExtendDB still writes `clientRequestToken`, which issue +/// #959 tracks, so the check ignores the case of the member name. +pub(crate) fn assert_token_refused(err: &str) { + let lower = err.to_ascii_lowercase(); + assert!( + err.starts_with("ValidationException:") + && lower.contains("value at 'clientrequesttoken'") + && err.contains(TOKEN_TEXT), + "expected the token's constraint message, got {}", + &err[..err.len().min(300)] + ); +} + +#[tokio::test] +async fn put_batch_and_transact_items_answer_a_long_name_or_the_number() { + // "PutItem item {pk, LONG, n}": the name on 6 of 8 sends and the + // overflow on 2; "{pk, n, LONG}": the overflow on 2 of 2. The same for + // BatchWriteItem (6 and 2 of 8; 2 of 2) and for a TransactWriteItems Put + // (8 and 4 of 12; the name on 2 of 8 and the overflow on 6). The SDK does + // not keep the attribute order, so each answer is accepted. ExtendDB + // reports the name: its size check runs first. + let t = tables().await; + let result = client() + .put_item() + .table_name(&t.simple_key_string) + .set_item(Some(item_with_a_long_name("o618", true))) + .send() + .await; + assert_name_or_overflow(&refused(result)); + + let put = put_request(item_with_a_long_name("o618", true)); + let err = batch_write(&t.simple_key_string, vec![put]).await; + assert_name_or_overflow(&err.unwrap_err()); + + let put = Put::builder() + .table_name(&t.simple_key_string) + .set_item(Some(item_with_a_long_name("o618", true))) + .build() + .expect("put"); + let err = transact_write(vec![TransactWriteItem::builder().put(put).build()]).await; + assert_name_or_overflow(&err.unwrap_err()); +} + +#[tokio::test] +async fn delete_and_update_item_report_a_long_name_ahead_of_value_numbers() { + // "DeleteItem, #n = LONG in ExpressionAttributeNames, :v in + // ExpressionAttributeValues" and "UpdateItem SET #n = :v, same maps": the + // name for key #n, 4 of 4 each, with either map first in the body. + let t = tables().await; + let want = format!("ValidationException:1 validation error detected: {LONG_NAME} for key #n"); + let result = client() + .delete_item() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("o618"))) + .condition_expression("#n = :v") + .expression_attribute_names("#n", long_name()) + .expression_attribute_values(":v", n_raw("1E2147483648")) + .send() + .await; + assert_eq!(refused(result), want); + + let result = client() + .update_item() + .table_name(&t.simple_key_string) + .set_key(Some(string_key("o618"))) + .update_expression("SET #n = :v") + .expression_attribute_names("#n", long_name()) + .expression_attribute_values(":v", n_raw("1E2147483648")) + .send() + .await; + assert_eq!(refused(result), want); +} + +#[tokio::test] +async fn transact_write_items_reports_a_long_token_ahead_of_a_long_name_and_a_number() { + // "TransactWriteItems ClientRequestToken of 37 characters, Put item {pk, + // LONG}" and "..., Put item {pk, n}": the token, 2 of 2 each. + let t = tables().await; + for item in [ + item_with_a_long_name("o618", false), + item_with_an_overflow("o618"), + ] { + let put = Put::builder() + .table_name(&t.simple_key_string) + .set_item(Some(item)) + .build() + .expect("put"); + let result = client() + .transact_write_items() + .transact_items(TransactWriteItem::builder().put(put).build()) + .client_request_token("x".repeat(37)) + .send() + .await; + assert_token_refused(&refused(result)); + } +} diff --git a/tests/rust/src/update_arithmetic_result.rs b/tests/rust/src/update_arithmetic_result.rs new file mode 100644 index 000000000..2f42d462f --- /dev/null +++ b/tests/rust/src/update_arithmetic_result.rs @@ -0,0 +1,224 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +//! The result of `+`, `-` and a number `ADD` in an `UpdateExpression`, on +//! `UpdateItem` and a `TransactWriteItems` `Update`: how a result in range is +//! spelled, and the answer for one that is not. +//! +//! Every row was measured against DynamoDB on 2026-11-03 (us-east-1) on both +//! operations, each request sent twice with the same answer both times. The +//! two `1.5 + 1.5` rows were measured on `UpdateItem` only. + +use std::collections::HashMap; + +use aws_sdk_dynamodb::operation::transact_write_items::TransactWriteItemsError; +use aws_sdk_dynamodb::types::{AttributeValue, ReturnValue, TransactWriteItem, Update}; + +use crate::test_base::*; + +const OVERFLOW: &str = + "Number overflow. Attempting to store a number with magnitude larger than supported range"; + +fn number(value: &str) -> AttributeValue { + AttributeValue::N(value.to_string()) +} + +/// Store an item whose `n` holds `stored` and return its key. +async fn seed(table: &str, stored: &str) -> HashMap { + let mut item = create_item(table); + item.insert("n".into(), number(stored)); + client() + .put_item() + .table_name(table) + .set_item(Some(item.clone())) + .send() + .await + .unwrap(); + get_key(table, &item) +} + +async fn stored_n(table: &str, key: HashMap) -> Option { + client() + .get_item() + .table_name(table) + .set_key(Some(key)) + .consistent_read(true) + .send() + .await + .unwrap() + .item() + .and_then(|item| item.get("n").cloned()) +} + +fn transact_update( + table: &str, + key: HashMap, + expression: &str, + value: &str, +) -> TransactWriteItem { + TransactWriteItem::builder() + .update( + Update::builder() + .table_name(table) + .set_key(Some(key)) + .update_expression(expression) + .expression_attribute_values(":v", number(value)) + .build() + .unwrap(), + ) + .build() +} + +#[tokio::test] +async fn arithmetic_result_is_stored_as_the_service_spells_it() { + let c = client(); + let t = tables().await; + let table = &t.simple_key_string; + let min = format!("0.{}1", "0".repeat(129)); + let twice_max_magnitude = format!("2{}", "0".repeat(125)); + let rows = [ + ("2.25", "SET n = n - :v", "0.25", "2"), + ("2.25", "ADD n :v", "-0.25", "2"), + ("1.5", "SET n = n - :v", "1.5", "0"), + ("2E-130", "SET n = n - :v", "1E-130", min.as_str()), + ( + "1E125", + "SET n = n + :v", + "1E125", + twice_max_magnitude.as_str(), + ), + ]; + for (stored, expression, value, expected) in rows { + let key = seed(table, stored).await; + let resp = c + .update_item() + .table_name(table) + .set_key(Some(key.clone())) + .update_expression(expression) + .expression_attribute_values(":v", number(value)) + .return_values(ReturnValue::UpdatedNew) + .send() + .await + .unwrap_or_else(|e| panic!("UpdateItem `{expression}`, n = {stored}: {}", err_msg(&e))); + let returned = resp.attributes().and_then(|a| a.get("n")).cloned(); + assert_eq!( + returned, + Some(number(expected)), + "UpdateItem `{expression}`, n = {stored}" + ); + assert_eq!( + stored_n(table, key).await, + Some(number(expected)), + "UpdateItem `{expression}`" + ); + + let key = seed(table, stored).await; + c.transact_write_items() + .transact_items(transact_update(table, key.clone(), expression, value)) + .send() + .await + .unwrap_or_else(|e| { + panic!( + "TransactWriteItems `{expression}`, n = {stored}: {}", + err_msg(&e) + ) + }); + assert_eq!( + stored_n(table, key).await, + Some(number(expected)), + "TransactWriteItems `{expression}`" + ); + } + // Measured on UpdateItem only. + for expression in ["SET n = n + :v", "ADD n :v"] { + let key = seed(table, "1.5").await; + c.update_item() + .table_name(table) + .set_key(Some(key.clone())) + .update_expression(expression) + .expression_attribute_values(":v", number("1.5")) + .send() + .await + .unwrap(); + assert_eq!( + stored_n(table, key).await, + Some(number("3")), + "`{expression}`" + ); + } +} + +/// A result outside the range or with more than 38 digits is a +/// `Number overflow` with no prefix, a result below the range included. On +/// `TransactWriteItems` it is the cancellation reason. +#[tokio::test] +async fn arithmetic_result_out_of_range_is_an_overflow() { + let c = client(); + let t = tables().await; + let table = &t.simple_key_string; + let rows = [ + ("5E125", "SET n = n + :v", "5E125"), + ("5E125", "ADD n :v", "5E125"), + ("1E125", "SET n = n + :v", "1"), + ("1.1E-130", "SET n = n - :v", "1E-130"), + ("1.1E-130", "ADD n :v", "-1E-130"), + ("-1.1E-130", "SET n = n + :v", "1E-130"), + ]; + for (stored, expression, value) in rows { + let key = seed(table, stored).await; + let before = stored_n(table, key.clone()).await; + let err = c + .update_item() + .table_name(table) + .set_key(Some(key.clone())) + .update_expression(expression) + .expression_attribute_values(":v", number(value)) + .send() + .await + .expect_err("an out-of-range result must be refused"); + assert_eq!( + err_code(&err), + Some("ValidationException"), + "`{expression}`, n = {stored}" + ); + assert_eq!( + err_msg(&err), + OVERFLOW, + "UpdateItem `{expression}`, n = {stored}" + ); + assert_eq!( + stored_n(table, key.clone()).await, + before, + "the item keeps its value" + ); + + let err = c + .transact_write_items() + .transact_items(transact_update(table, key.clone(), expression, value)) + .send() + .await + .expect_err("an out-of-range result must cancel the transaction"); + let message = err_msg(&err); + let TransactWriteItemsError::TransactionCanceledException(tce) = err.into_service_error() + else { + panic!( + "expected TransactionCanceledException for `{expression}`, n = {stored}: {message}" + ); + }; + let reasons: Vec<(Option<&str>, Option<&str>)> = tce + .cancellation_reasons() + .iter() + .map(|r| (r.code(), r.message())) + .collect(); + assert_eq!( + reasons, + [(Some("ValidationError"), Some(OVERFLOW))], + "`{expression}`, n = {stored}" + ); + assert_eq!( + stored_n(table, key).await, + before, + "the item keeps its value" + ); + } +} diff --git a/tests/rust/src/update_item_number_validation.rs b/tests/rust/src/update_item_number_validation.rs index 6d9475460..30ba783be 100644 --- a/tests/rust/src/update_item_number_validation.rs +++ b/tests/rust/src/update_item_number_validation.rs @@ -39,11 +39,11 @@ async fn update_item_rejects_malformed_number_in_expression() { .await .expect_err("malformed number must be rejected"); assert_eq!(err_code(&e), Some("ValidationException"), "{}", err_msg(&e)); - assert!( - err_msg(&e).contains("ExpressionAttributeValues contains invalid value") - && err_msg(&e).contains("cannot be converted to a numeric value: 12e"), - "got: {}", - err_msg(&e) + // Measured 2026-11-11: the prefix and the number message, with no + // ExpressionAttributeValues wrapper. + assert_eq!( + err_msg(&e), + "1 validation error detected: The parameter cannot be converted to a numeric value: 12e" ); } diff --git a/tests/test_backup_job.py b/tests/test_backup_job.py index c83f1bef4..e167a7bc6 100644 --- a/tests/test_backup_job.py +++ b/tests/test_backup_job.py @@ -12,13 +12,13 @@ waits for the slot and is answered only once the first job has ended (the first backup AVAILABLE), and a third is accepted at once afterwards; a wait that outlives the 30 s bound is LimitExceededException naming the bound; -- a table holding an `N` value the wire let in (#618: `1E126` or `abc` in a - non-key attribute, `1E126` in the range key of a key-ordered table) written - through each of the four paths that store it unvalidated (BatchWriteItem, - TransactWriteItems, UpdateItem on an absent item in the key it creates, - ImportTable) gives a FAILED row, no manifest under the root, three log - lines naming each item's key and attribute, and a FAILED reason giving the - count and the first item and naming #618; +- a table holding items with an `N` value the service cannot store (#618: + `1E126` or `abc` in a non-key attribute, `1E126` or more in the range key + of a key-ordered table), in the shapes the write paths stored before they + checked numbers and written straight into the table's storage on each + backend, gives a FAILED row, no manifest under the root, three log lines + naming each item's key and attribute, and a FAILED reason giving the count + and the first item and naming #618; - an item over `max_item_size_bytes` (stored under the default limit, the server restarted with a lower one) gives a reason that does not name #618; - a refused item with a UUID hash key and a ten-digit epoch `N` range key is @@ -159,12 +159,9 @@ import time import uuid from concurrent.futures import ThreadPoolExecutor +from decimal import Decimal -import boto3 import pytest -import requests -from botocore.auth import SigV4Auth -from botocore.awsrequest import AWSRequest from botocore.exceptions import ClientError import lifecycle_helpers as lh @@ -343,24 +340,6 @@ def _started(d: _Deployment, log_path: str) -> tuple[_Server, object, dict]: return server, _client(d.port, creds), creds -def _signed_post(d: _Deployment, creds: dict, operation: str, body: dict) -> requests.Response: - """A hand-signed request for an operation botocore's model would refuse - (ImportTable's `FileSource`).""" - body_bytes = json.dumps(body).encode("utf-8") - headers = { - "X-Amz-Target": f"DynamoDB_20120810.{operation}", - "Content-Type": "application/x-amz-json-1.0", - } - endpoint = f"https://127.0.0.1:{d.port}" - frozen = boto3.Session( - aws_access_key_id=creds["AWS_ACCESS_KEY_ID"], - aws_secret_access_key=creds["AWS_SECRET_ACCESS_KEY"], - ).get_credentials().get_frozen_credentials() - aws_req = AWSRequest(method="POST", url=endpoint, data=body_bytes, headers=headers) - SigV4Auth(frozen, "dynamodb", "us-east-1").add_auth(aws_req) - return requests.post(endpoint, data=body_bytes, headers=dict(aws_req.headers), verify=False) - - def _wait_active(ddb, name: str, timeout: float = 60.0) -> None: deadline = time.monotonic() + timeout while time.monotonic() < deadline: @@ -593,111 +572,197 @@ def write(start: int) -> None: list(pool.map(write, range(0, n, 25))) -# The three items each path stores: `1E126` and `abc` in a non-key attribute -# and `1E126` in the range key (an `abc` range key is refused by the number -# column on PostgreSQL and SQLite, so no path stores one). UpdateItem -# validates the values its expression sets, so its three items carry the bad -# value in the key it creates. +# Items holding an `N` value the service cannot store (#618), in the shapes +# the write paths stored before they checked numbers. `values`: `1E126` and +# `abc` in non-key attributes and `1E126` in the range key, as BatchWriteItem, +# TransactWriteItems and ImportTable stored them. `keys`: three range keys out +# of range, as UpdateItem stored them on an absent item, in the key it +# created. Each path refuses them now, so the cells write them into the +# table's storage (`_seed_stored_items`): the backup's check of stored items +# guards the items a table still holds from before. An `abc` range key has no +# stored form, since every backend's range key column holds a number or an +# order-preserving text of one. BAD_ITEMS = [ {"pk": {"S": "a"}, "sk": {"N": "1"}, "amount": {"N": "1E126"}}, {"pk": {"S": "a"}, "sk": {"N": "2"}, "note": {"N": "abc"}}, {"pk": {"S": "b"}, "sk": {"N": "1E126"}}, ] BAD_KEYS = [ - {"pk": {"S": "a"}, "sk": {"N": "1E126"}}, - {"pk": {"S": "b"}, "sk": {"N": "1E127"}}, - {"pk": {"S": "c"}, "sk": {"N": "1E128"}}, + {"pk": {"S": "a"}, "sk": {"N": "1E126"}, "note": {"S": "created from the key"}}, + {"pk": {"S": "b"}, "sk": {"N": "1E127"}, "note": {"S": "created from the key"}}, + {"pk": {"S": "c"}, "sk": {"N": "1E128"}, "note": {"S": "created from the key"}}, ] +STORED_ITEMS = {"values": BAD_ITEMS, "keys": BAD_KEYS} GOOD_ITEMS = [{"pk": {"S": "z"}, "sk": {"N": str(i)}, "amount": {"N": str(i)}} for i in range(5)] -def _write_batch(d, ddb, creds, name): - _create_composite_table(ddb, name) - ddb.batch_write_item( - RequestItems={name: [{"PutRequest": {"Item": i}} for i in BAD_ITEMS + GOOD_ITEMS]} - ) - return BAD_ITEMS - - -def _write_transact(d, ddb, creds, name): - _create_composite_table(ddb, name) - ddb.transact_write_items( - TransactItems=[{"Put": {"TableName": name, "Item": i}} for i in BAD_ITEMS + GOOD_ITEMS] - ) - return BAD_ITEMS - - -def _write_update(d, ddb, creds, name): - _create_composite_table(ddb, name) - for key in BAD_KEYS: - ddb.update_item( - TableName=name, - Key=key, - UpdateExpression="SET note = :v", - ExpressionAttributeValues={":v": {"S": "created from the key"}}, +def _seed_cell(d: _Deployment) -> None: + """The cells that seed stored items write a MongoDB deployment's + documents with `mongosh` in the replica set's container (`_mongosh`), + which `EXTENDDB_TEST_MONGODB_CONTAINER` names.""" + if d.backend == "mongodb" and not _mongodb_container(): + pytest.skip("EXTENDDB_TEST_MONGODB_CONTAINER is unset: the MongoDB items are seeded with mongosh in it") + + +def _key_text(value: dict) -> str: + """A key attribute's text in storage: the `S` or `N` as written.""" + (text,) = value.values() + return text + + +def _sqlite_number_key(n: str) -> str: + """The SQLite backend's text for an `N` range key, as + `encode_orderable_number` (crates/storage-sqlite/src/number_key.rs) + writes it: `1` for zero; otherwise a class digit (`2` for a positive + value, `0` for a negative one), the exponent `E` of the form + `0.d1d2... x 10^E` in six digits (100000 + E, or 100000 - E for a + negative value), and the digits without trailing zeros (for a negative + value each digit complemented to 9 - d, then `:`).""" + value = Decimal(n) + if value == 0: + return "1" + sign, digits, exponent = value.as_tuple() + digits = list(digits) + while digits[-1] == 0: + digits.pop() + exponent += 1 + e = len(digits) + exponent + if sign: + return f"0{100000 - e:06d}" + "".join(str(9 - x) for x in digits) + ":" + return f"2{100000 + e:06d}" + "".join(str(x) for x in digits) + + +def _mongodb_id(item: dict) -> str: + """A MongoDB base-table document's `_id`: the hash and range key texts + as a netstring, `:,` each (`composite_id` in + crates/storage-mongodb/src/data/mod.rs).""" + parts = [_key_text(item["pk"]), _key_text(item["sk"])] + return "".join(f"{len(p.encode())}:{p}," for p in parts) + + +def _pg_data_connection_string(catalog: str) -> str: + """The data database's connection string: `init` names the data + database after the catalog, less the `_catalog` suffix.""" + base, _, database = catalog.rpartition("/") + assert database.endswith("_catalog"), catalog + return f"{base}/{database[: -len('_catalog')]}" + + +def _seed_stored_items(d: _Deployment, name: str, items: list[dict]) -> None: + """Write `items` into the storage of the key-ordered table `name` (hash + key `pk` S, range key `sk` N) past the wire, which refuses them, in the + form the server writes an item: + + - SQLite: a row of `"_ddb_"` in the database file, with `pk` + the hash key's text, `sk_n` the range key's text from + `_sqlite_number_key` and `item_data` the item as DynamoDB JSON; + - PostgreSQL: the same row in the data database, with `sk_n` a + `NUMERIC` and `item_data` a `JSONB`; + - MongoDB: a document of `extenddb_data._ddb_
` holding `_id` + (`_mongodb_id`), `pk`, `sk_n` as a Decimal128 and `item_data`, written + with `mongosh` in the replica set's container. + + The table id is read from the catalog. The caller writes `GOOD_ITEMS` + through the wire first, and the form is checked against the one the + server stored for `GOOD_ITEMS[0]`: a change to the stored form fails + here, before any row the server might not read is written.""" + good = GOOD_ITEMS[0] + if d.backend == "mongodb": + table_id = _mongosh( + f"const t = db.tables.findOne({{'_id.account_id': {json.dumps(ACCOUNT)}, " + f"'_id.table_name': {json.dumps(name)}}});\nprint(JSON.stringify(t.table_id))" ) - for item in GOOD_ITEMS: - ddb.put_item(TableName=name, Item=item) - return BAD_KEYS - - -def _write_import(d, ddb, creds, name): - path = os.path.join(d.cwd, ACCOUNT, f"{name}.json") - with open(path, "w") as f: - for item in BAD_ITEMS + GOOD_ITEMS: - f.write(json.dumps({"Item": item}) + "\n") - resp = _signed_post( - d, - creds, - "ImportTable", - { - "FileSource": {"Path": path}, - "InputFormat": "DYNAMODB_JSON", - "TableCreationParameters": { - "TableName": name, - "AttributeDefinitions": [ - {"AttributeName": "pk", "AttributeType": "S"}, - {"AttributeName": "sk", "AttributeType": "N"}, - ], - "KeySchema": [ - {"AttributeName": "pk", "KeyType": "HASH"}, - {"AttributeName": "sk", "KeyType": "RANGE"}, - ], - "BillingMode": "PAY_PER_REQUEST", - }, - }, - ) - assert resp.status_code == 200, resp.text - assert resp.json()["ImportTableDescription"]["ImportStatus"] == "COMPLETED", resp.text - _wait_active(ddb, name) - return BAD_ITEMS - + coll = f'db.getSiblingDB("extenddb_data").getCollection({json.dumps("_ddb_" + table_id)})' + row = _mongosh( + f"const g = {coll}.findOne({{_id: {json.dumps(_mongodb_id(good))}}});\n" + "print(JSON.stringify(g && {pk: g.pk, sk_n: g.sk_n.toString(), item_data: g.item_data}))" + ) + assert row and row["pk"] == _key_text(good["pk"]), row + assert Decimal(row["sk_n"]) == Decimal(good["sk"]["N"]) and row["item_data"] == good, row + docs = ", ".join( + f"{{_id: {json.dumps(_mongodb_id(i))}, pk: {json.dumps(_key_text(i['pk']))}, " + f"sk_n: NumberDecimal({json.dumps(i['sk']['N'])}), item_data: {json.dumps(i)}}}" + for i in items + ) + n = _mongosh(f"print(Object.keys({coll}.insertMany([{docs}]).insertedIds).length)") + assert n == len(items), n + return + rows = [(_key_text(i["pk"]), i["sk"]["N"], json.dumps(i, separators=(",", ":"))) for i in items] + if d.backend == "sqlite": + try: + import pysqlite3 as sqlite3 + except ImportError: + import sqlite3 + conn = sqlite3.connect(d.catalog, timeout=10) + try: + found = conn.execute( + "SELECT table_id FROM tables WHERE account_id = ? AND table_name = ?", (ACCOUNT, name) + ).fetchone() + assert found, f"no catalog row for {name}" + data = f'"_ddb_{found[0]}"' + row = conn.execute( + f"SELECT item_data FROM {data} WHERE pk = ? AND sk_n = ?", + (_key_text(good["pk"]), _sqlite_number_key(good["sk"]["N"])), + ).fetchone() + assert row and json.loads(row[0]) == good, row + conn.executemany( + f"INSERT INTO {data} (pk, sk_n, item_data) VALUES (?, ?, ?)", + [(pk, _sqlite_number_key(sk), item) for pk, sk, item in rows], + ) + conn.commit() + finally: + conn.close() + return + import psycopg2 -WRITE_PATHS = { - "batch_write_item": _write_batch, - "transact_write_items": _write_transact, - "update_item": _write_update, - "import_table": _write_import, -} + conn = psycopg2.connect(d.catalog) + try: + with conn.cursor() as cur: + cur.execute("SELECT table_id FROM tables WHERE account_id = %s AND table_name = %s", (ACCOUNT, name)) + found = cur.fetchone() + finally: + conn.close() + assert found, f"no catalog row for {name}" + data = f'"_ddb_{found[0]}"' + conn = psycopg2.connect(_pg_data_connection_string(d.catalog)) + try: + with conn.cursor() as cur: + cur.execute( + f"SELECT item_data FROM {data} WHERE pk = %s AND sk_n = %s::numeric", + (_key_text(good["pk"]), good["sk"]["N"]), + ) + row = cur.fetchone() + assert row and row[0] == good, row + cur.executemany(f"INSERT INTO {data} (pk, sk_n, item_data) VALUES (%s, %s::numeric, %s::jsonb)", rows) + conn.commit() + finally: + conn.close() def _key_json(item: dict) -> str: return json.dumps({"pk": item["pk"], "sk": item["sk"]}, separators=(",", ":")) -@pytest.mark.parametrize("path", sorted(WRITE_PATHS)) -def test_an_item_the_wire_let_in_fails_the_backup(deployment, tmp_path, path): - """Three items with an `N` value #618 let in, stored through one write - path, and five that restore: CreateBackup answers CREATING, the job - validates every item (three log lines, one per item, with its key and - attribute path), the row is FAILED with a reason giving the count and - the first item and naming #618, DescribeBackup is BackupNotFound, - ListBackups omits it, and no manifest is under the root.""" +@pytest.mark.parametrize("kind", sorted(STORED_ITEMS)) +def test_a_stored_item_holding_an_invalid_number_fails_the_backup(deployment, tmp_path, kind): + """Three stored items holding an `N` value the service cannot store + (#618), written into the table's storage in the shapes the write paths + stored before they checked numbers (`_seed_stored_items`; `kind` names + the set in `STORED_ITEMS`), and five that restore: CreateBackup answers + CREATING, the job validates every item (three log lines, one per item, + with its key and attribute path), the row is FAILED with a reason giving + the count and the first item and naming #618, DescribeBackup is + BackupNotFound, ListBackups omits it, and no manifest is under the + root.""" d = deployment - server, ddb, creds = _started(d, str(tmp_path / "serve.log")) + _seed_cell(d) + server, ddb, _ = _started(d, str(tmp_path / "serve.log")) try: - name = f"n618-{path.replace('_', '')}-{uuid.uuid4().hex[:6]}" - bad = WRITE_PATHS[path](d, ddb, creds, name) + name = f"n618-{kind}-{uuid.uuid4().hex[:6]}" + _create_composite_table(ddb, name) + ddb.batch_write_item(RequestItems={name: [{"PutRequest": {"Item": i}} for i in GOOD_ITEMS]}) + bad = STORED_ITEMS[kind] + _seed_stored_items(d, name, bad) stored = ddb.scan(TableName=name, ConsistentRead=True)["Items"] assert len(stored) == len(bad) + len(GOOD_ITEMS), stored details = ddb.create_backup(TableName=name, BackupName="nightly")["BackupDetails"] @@ -727,18 +792,21 @@ def test_an_item_the_wire_let_in_fails_the_backup(deployment, tmp_path, path): def test_a_refused_item_with_a_uuid_and_epoch_key_is_named_whole(deployment, tmp_path): - """One refused item with a realistic key, a UUID hash key and a ten-digit - epoch range key, and `amount: abc`: the log line and the FAILED reason - carry the whole key as DynamoDB JSON (`{"pk":{"S":""},"sk":{"N": - "1761955200"}}`), not a shortened form, and the wire's reason for the - value, so an operator can find the item from either.""" + """One stored item the backup refuses, written into storage as + `_seed_stored_items` does, with a realistic key, a UUID hash key and a + ten-digit epoch range key, and `amount: abc`: the log line and the FAILED + reason carry the whole key as DynamoDB JSON (`{"pk":{"S":""},"sk": + {"N":"1761955200"}}`), not a shortened form, and the wire's reason for + the value, so an operator can find the item from either.""" d = deployment + _seed_cell(d) server, ddb, _ = _started(d, str(tmp_path / "serve.log")) try: name = f"n618-key-{uuid.uuid4().hex[:6]}" _create_composite_table(ddb, name) + ddb.batch_write_item(RequestItems={name: [{"PutRequest": {"Item": i}} for i in GOOD_ITEMS]}) item = {"pk": {"S": str(uuid.uuid4())}, "sk": {"N": "1761955200"}, "amount": {"N": "abc"}} - ddb.batch_write_item(RequestItems={name: [{"PutRequest": {"Item": i}} for i in [item] + GOOD_ITEMS]}) + _seed_stored_items(d, name, [item]) arn = ddb.create_backup(TableName=name, BackupName="nightly")["BackupDetails"]["BackupArn"] _wait_backup_gone(ddb, arn) key = _key_json(item) diff --git a/tests/test_import_export.py b/tests/test_import_export.py index ac601f1ef..aa4d170e9 100755 --- a/tests/test_import_export.py +++ b/tests/test_import_export.py @@ -500,3 +500,216 @@ def test_export_refuses_to_overwrite_an_existing_file( }) finally: discard(target) + + +# --------------------------------------------------------------------------- +# ImportTable rows holding numbers the service cannot store +# +# Measured against DynamoDB ImportTable from S3 on 2026-11-03 (us-east-1), one +# import per file, and again on 2026-11-11 for the rows marked so. Unless a +# test says otherwise, each file held a row with a valid number, the row under +# test, and another valid row. A row holding a value that is not a number +# ends the import of the file there; a row holding a number out of range is +# skipped and the rows after it are imported. Either way the row counts as one +# error, and an import with any error is FAILED with ItemValidationError, +# keeping the rows it imported. ImportTable reads a number's exponent and +# scale as 32-bit integers: on 2026-11-11 it still did, when BatchWriteItem +# already read a 64-bit exponent. +# --------------------------------------------------------------------------- + +OUT_OF_RANGE = [ + pytest.param({"n": {"N": "1E126"}}, id="overflow"), + pytest.param({"n": {"N": "1E-131"}}, id="underflow"), + pytest.param({"n": {"N": "1" * 39}}, id="39-digits"), + pytest.param({"n": {"N": "1E999999999"}}, id="large-exponent"), + # Number underflow: the 32-bit magnitude wraps. Both measured 2026-11-11. + pytest.param({"n": {"N": "1E2147483647"}}, id="magnitude-wraps"), + pytest.param({"n": {"N": "10E2147483647"}}, id="magnitude-wraps-two-digits"), + pytest.param({"s": {"NS": ["1", "1E126"]}}, id="number-set"), + pytest.param({"m": {"M": {"x": {"N": "1E126"}}}}, id="in-a-map"), + pytest.param({"l": {"L": [{"N": "1E-131"}]}}, id="in-a-list"), +] + +NOT_A_NUMBER = [ + pytest.param({"n": {"N": "abc"}}, id="letters"), + pytest.param({"n": {"N": ""}}, id="empty"), + pytest.param({"n": {"N": " 5"}}, id="leading-space"), + pytest.param({"n": {"N": "12e"}}, id="no-exponent-digits"), + # Not numbers to a 32-bit reader. Measured again on 2026-11-11, with the + # exponent past the 64-bit range as well. + pytest.param({"n": {"N": "1E2147483648"}}, id="exponent-over-32-bits"), + pytest.param({"n": {"N": "1E-2147483648"}}, id="scale-over-32-bits"), + pytest.param({"n": {"N": "1E9223372036854775808"}}, id="exponent-over-64-bits"), + pytest.param({"s": {"NS": ["1", "abc"]}}, id="number-set"), + pytest.param({"a": {"N": "1E126"}, "b": {"N": "abc"}}, id="after-a-number-out-of-range"), + # The service imports this row with `5`. ExtendDB does not read a Unicode + # decimal digit as the digit in any number (docs/differences-from-dynamodb.md). + pytest.param({"n": {"N": "\uff15"}}, id="fullwidth-digit"), +] + + +class TestImportTableNumbers: + """ImportTable refuses rows holding numbers the service cannot store.""" + + @staticmethod + def _import(table: str, rows: list[dict], key_type: str = "S") -> dict: + path = import_source("".join(json.dumps({"Item": row}) + "\n" for row in rows)) + try: + return extenddb_request("ImportTable", { + "FileSource": {"Path": path}, + "InputFormat": "DYNAMODB_JSON", + "TableCreationParameters": { + "TableName": table, + "AttributeDefinitions": [ + {"AttributeName": "pk", "AttributeType": key_type}, + ], + "KeySchema": [{"AttributeName": "pk", "KeyType": "HASH"}], + "BillingMode": "PAY_PER_REQUEST", + }, + })["ImportTableDescription"] + finally: + discard(path) + + @staticmethod + def _counts(desc: dict) -> tuple[int, int, int]: + return desc["ProcessedItemCount"], desc["ImportedItemCount"], desc["ErrorCount"] + + @staticmethod + def _assert_failed(desc: dict) -> None: + assert (desc["ImportStatus"], desc.get("FailureCode")) == ("FAILED", "ItemValidationError") + assert desc["FailureMessage"].startswith( + "Some of the items failed validation checks and were not imported." + ) + + @staticmethod + def _items(dynamodb_client, table: str) -> dict: + items = dynamodb_client.scan(TableName=table, ConsistentRead=True)["Items"] + return {next(iter(item["pk"].values())): item for item in items} + + @pytest.mark.parametrize("attrs", OUT_OF_RANGE) + def test_a_row_holding_a_number_out_of_range_is_skipped( + self, dynamodb_client, unique_table_name, cleanup_table, attrs + ): + cleanup_table(unique_table_name) + desc = self._import(unique_table_name, [ + {"pk": {"S": "before"}, "n": {"N": "1"}}, + {"pk": {"S": "under-test"}, **attrs}, + {"pk": {"S": "after"}, "n": {"N": "2"}}, + ]) + assert self._counts(desc) == (3, 2, 1) + self._assert_failed(desc) + assert sorted(self._items(dynamodb_client, unique_table_name)) == ["after", "before"] + + @pytest.mark.parametrize("attrs", NOT_A_NUMBER) + def test_a_row_holding_a_value_that_is_not_a_number_ends_the_import( + self, dynamodb_client, unique_table_name, cleanup_table, attrs + ): + cleanup_table(unique_table_name) + desc = self._import(unique_table_name, [ + {"pk": {"S": "before"}, "n": {"N": "1"}}, + {"pk": {"S": "under-test"}, **attrs}, + {"pk": {"S": "after"}, "n": {"N": "2"}}, + ]) + assert self._counts(desc) == (2, 1, 1) + self._assert_failed(desc) + assert sorted(self._items(dynamodb_client, unique_table_name)) == ["before"] + + def test_a_row_with_no_key_and_a_value_that_is_not_a_number_ends_the_import( + self, dynamodb_client, unique_table_name, cleanup_table + ): + cleanup_table(unique_table_name) + desc = self._import(unique_table_name, [ + {"pk": {"S": "before"}, "n": {"N": "1"}}, + {"other": {"S": "x"}, "n": {"N": "abc"}}, + {"pk": {"S": "after"}, "n": {"N": "2"}}, + ]) + assert self._counts(desc) == (2, 1, 1) + self._assert_failed(desc) + assert sorted(self._items(dynamodb_client, unique_table_name)) == ["before"] + + @pytest.mark.parametrize("key,counts,imported", [ + pytest.param("1E126", (3, 2, 1), ["1", "3"], id="out-of-range"), + pytest.param("abc", (2, 1, 1), ["1"], id="not-a-number"), + ]) + def test_a_number_key_is_checked_as_any_number( + self, dynamodb_client, unique_table_name, cleanup_table, key, counts, imported + ): + cleanup_table(unique_table_name) + desc = self._import( + unique_table_name, + [{"pk": {"N": "1"}}, {"pk": {"N": key}}, {"pk": {"N": "3"}}], + key_type="N", + ) + assert self._counts(desc) == counts + self._assert_failed(desc) + assert sorted(self._items(dynamodb_client, unique_table_name)) == imported + + def test_rows_after_a_skipped_row_are_imported_until_one_ends_the_import( + self, dynamodb_client, unique_table_name, cleanup_table + ): + """The measured five-row file had these values in this order.""" + cleanup_table(unique_table_name) + desc = self._import(unique_table_name, [ + {"pk": {"S": "before"}, "n": {"N": "1"}}, + {"pk": {"S": "out-of-range"}, "n": {"N": "1E126"}}, + {"pk": {"S": "middle"}, "n": {"N": "3"}}, + {"pk": {"S": "not-a-number"}, "n": {"N": "abc"}}, + {"pk": {"S": "after"}, "n": {"N": "2"}}, + ]) + assert self._counts(desc) == (4, 2, 2) + self._assert_failed(desc) + assert sorted(self._items(dynamodb_client, unique_table_name)) == ["before", "middle"] + + def test_a_row_whose_key_has_the_wrong_type_fails_the_import( + self, dynamodb_client, unique_table_name, cleanup_table + ): + """A file whose only refused row has a number key on a string-key table. + + Measured on 2026-11-11 (us-east-1) with the rows a-k, pk 2 (N) and + c-k, every number valid: the import is FAILED with + ItemValidationError, ProcessedItemCount 3, ImportedItemCount 2 and + ErrorCount 1, and a-k and c-k are in the table. The service logged + the row as "One or more parameter values were invalid: Type mismatch + for key pk expected: S actual: N". The handler gives this status for + any refused row, whichever check refused it. + """ + cleanup_table(unique_table_name) + desc = self._import(unique_table_name, [ + {"pk": {"S": "before"}, "n": {"N": "1"}}, + {"pk": {"N": "2"}, "n": {"N": "3"}}, + {"pk": {"S": "after"}, "n": {"N": "4"}}, + ]) + assert self._counts(desc) == (3, 2, 1) + self._assert_failed(desc) + assert sorted(self._items(dynamodb_client, unique_table_name)) == ["after", "before"] + + def test_numbers_are_stored_as_the_service_spells_them( + self, dynamodb_client, unique_table_name, cleanup_table + ): + """Every row valid; each case was a row of one measured file.""" + cleanup_table(unique_table_name) + cases = { + "frac": ({"N": "1.50"}, {"N": "1.5"}), + "negative-zero": ({"N": "-0.0"}, {"N": "0"}), + "plus": ({"N": "+5"}, {"N": "5"}), + "exponent": ({"N": "1.5E2"}, {"N": "150"}), + "zero-exponent": ({"N": "0E125"}, {"N": "0"}), + "max": ( + {"N": "9.9999999999999999999999999999999999999E125"}, + {"N": "9" * 38 + "0" * 88}, + ), + "min": ({"N": "1E-130"}, {"N": "0." + "0" * 129 + "1"}), + "number-set": ({"NS": ["1.0", "2"]}, {"NS": ["1", "2"]}), + } + desc = self._import( + unique_table_name, + [{"pk": {"S": name}, "n": given} for name, (given, _) in cases.items()], + ) + assert self._counts(desc) == (len(cases), len(cases), 0) + assert (desc["ImportStatus"], desc.get("FailureCode")) == ("COMPLETED", None) + items = self._items(dynamodb_client, unique_table_name) + for name, (_, stored) in cases.items(): + got = items[name]["n"] + if "NS" in got: + got = {"NS": sorted(got["NS"])} + assert got == stored, name diff --git a/tests/test_number_validation_all_ops.py b/tests/test_number_validation_all_ops.py new file mode 100644 index 000000000..73c366ad9 --- /dev/null +++ b/tests/test_number_validation_all_ops.py @@ -0,0 +1,81 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Number validation on the batch, transaction and condition value paths (#618). + +One request for each form of the message, and one showing that a refused +BatchWriteItem writes nothing. Every expectation is DynamoDB's answer, measured +in us-east-1 with each request sent twice and the same answer both times. +BatchWriteItem gives the number message with no prefix and PutItem puts +"1 validation error detected: " ahead of it (2026-10-27). TransactWriteItems +gives the number message for a value in ExpressionAttributeValues with no +prefix and no wrapper (2026-11-11). The Rust wire tests in +tests/rust/src/number_validation_all_ops.rs hold the other paths. +""" + +from __future__ import annotations + +import pytest +from botocore.exceptions import ClientError + +NOT_A_NUMBER = "The parameter cannot be converted to a numeric value: abc" +OVERFLOW = ( + "Number overflow. Attempting to store a number with magnitude larger than " + "supported range" +) + + +@pytest.fixture() +def table(create_and_cleanup_table) -> str: + return create_and_cleanup_table()["TableDescription"]["TableName"] + + +def _refusal(call) -> tuple[str, str]: + with pytest.raises(ClientError) as ei: + call() + err = ei.value.response["Error"] + return err["Code"], err["Message"] + + +def test_batch_write_item_gives_the_number_message_without_a_prefix(dynamodb_client, table): + answer = _refusal(lambda: dynamodb_client.batch_write_item(RequestItems={table: [ + {"PutRequest": {"Item": {"pk": {"S": "b618"}, "n": {"N": "abc"}}}}, + ]})) + assert answer == ("ValidationException", NOT_A_NUMBER) + + +def test_put_item_puts_the_prefix_on_a_condition_value(dynamodb_client, table): + answer = _refusal(lambda: dynamodb_client.put_item( + TableName=table, + Item={"pk": {"S": "p618"}}, + ConditionExpression="attribute_not_exists(amount) OR amount = :v", + ExpressionAttributeValues={":v": {"N": "1E126"}}, + )) + assert answer == ("ValidationException", f"1 validation error detected: {OVERFLOW}") + + +def test_transact_write_items_gives_a_value_number_message_without_a_wrapper( + dynamodb_client, table +): + answer = _refusal(lambda: dynamodb_client.transact_write_items(TransactItems=[ + {"Update": { + "TableName": table, + "Key": {"pk": {"S": "t618"}}, + "UpdateExpression": "SET amount = :v", + "ExpressionAttributeValues": {":v": {"N": "1E126"}}, + }}, + ])) + assert answer == ("ValidationException", OVERFLOW) + + +def test_a_refused_batch_write_item_writes_nothing(dynamodb_client, table): + """A valid put ahead of a put holding `abc`: neither is written.""" + answer = _refusal(lambda: dynamodb_client.batch_write_item(RequestItems={table: [ + {"PutRequest": {"Item": {"pk": {"S": "good"}}}}, + {"PutRequest": {"Item": {"pk": {"S": "bad"}, "n": {"N": "abc"}}}}, + ]})) + assert answer == ("ValidationException", NOT_A_NUMBER) + stored = dynamodb_client.get_item( + TableName=table, Key={"pk": {"S": "good"}}, ConsistentRead=True + ) + assert "Item" not in stored diff --git a/tests/test_update_item_number_validation.py b/tests/test_update_item_number_validation.py index e30ce11e6..9da4b0e25 100644 --- a/tests/test_update_item_number_validation.py +++ b/tests/test_update_item_number_validation.py @@ -42,8 +42,12 @@ def test_update_item_rejects_malformed_number_in_expression( _update_set(dynamodb_client, table_with_item, {"N": "12e"}) err = ei.value.response["Error"] assert err["Code"] == "ValidationException" - assert "ExpressionAttributeValues contains invalid value" in err["Message"] - assert "cannot be converted to a numeric value: 12e" in err["Message"] + # Measured 2026-11-11: the prefix and the number message, with no + # ExpressionAttributeValues wrapper. + assert err["Message"] == ( + "1 validation error detected: " + "The parameter cannot be converted to a numeric value: 12e" + ) def test_update_item_rejects_number_overflow_in_expression(