From c24d1d54a780a59524b53eb43192ed4c5a8ddcf6 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Tue, 27 Oct 2026 15:27:59 +0000 Subject: [PATCH 1/2] fix(engine): validate numbers on every key and item path Only the PutItem item and UpdateItem's update values were checked. AttributeValue keeps a number it cannot read as written, so GetItem, DeleteItem and UpdateItem keys, BatchGetItem and TransactGetItems keys, BatchWriteItem writes, every TransactWriteItems action, and PutItem and DeleteItem condition values used values such as abc and 1E126 as written, where DynamoDB refuses them with a ValidationException. Each of those paths now checks its N and NS values with the text that operation answers. PutItem, UpdateItem and DeleteItem start the message with "1 validation error detected: ", the other operations do not, and a TransactWriteItems value names its placeholder. BatchWriteItem, TransactGetItems and TransactWriteItems read a number with a 32-bit exponent and scale, as DynamoDB does on those operations, so 10E2147483647 is a Number underflow there and a Number overflow on the others. Each check is placed where DynamoDB reports it, unless that position would change the answer to a request with a second fault that main already answers as DynamoDB does. The ClientRequestToken check now runs before the actions, as on DynamoDB. A zero whose exponent fits in an i32, such as 0E126, stays accepted as 0 until Query, Scan and ImportTable validate numbers. A zero whose exponent does not fit is range checked on its magnitude, which makes 0E9223372036854775807 a Number underflow, as on DynamoDB. Part of #618 --- crates/core/src/validation/number.rs | 89 ++- .../core/src/validation/number/operations.rs | 364 +++++++++++ .../src/validation/number/operations/tests.rs | 589 ++++++++++++++++++ crates/engine/src/batch_get_item.rs | 15 + crates/engine/src/batch_write_item.rs | 5 + crates/engine/src/delete_item.rs | 10 + crates/engine/src/get_item.rs | 5 + crates/engine/src/put_item.rs | 14 + crates/engine/src/transact_get_items.rs | 3 + crates/engine/src/transact_write_items.rs | 18 +- crates/engine/src/update_item.rs | 4 + 11 files changed, 1095 insertions(+), 21 deletions(-) create mode 100644 crates/core/src/validation/number/operations.rs create mode 100644 crates/core/src/validation/number/operations/tests.rs diff --git a/crates/core/src/validation/number.rs b/crates/core/src/validation/number.rs index 80f0ec00c..c85a080a8 100644 --- a/crates/core/src/validation/number.rs +++ b/crates/core/src/validation/number.rs @@ -3,10 +3,18 @@ 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; + /// Validate and normalize a `DynamoDB` number string. pub fn validate_and_normalize_number(s: &str) -> Result { // DynamoDB does not trim: leading or trailing whitespace makes the number @@ -70,13 +78,13 @@ 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, Scan (#544, #562) and ImportTable do not + // validate numbers yet: refusing `0E126` here would make them look it + // up and store it as a key of its own instead of `0` (#618). if i32::try_from(explicit_exp).is_err() { - check_magnitude(point_offset)?; + check_zero_magnitude(explicit_exp, frac_part)?; } return Ok("0".to_owned()); } @@ -136,6 +144,36 @@ fn check_magnitude(magnitude_exp: i128) -> Result<(), DynamoDbError> { Ok(()) } +/// Range check a zero on its magnitude, its precision (1 for a zero) less its +/// scale, where the scale is the number of fraction digits less the exponent. +/// The scale saturates at the 64-bit limits and the magnitude wraps, which +/// gives the service's answer for every such zero measured on 2026-10-27 +/// (us-east-1, each value twice): +/// +/// - `0E9223372036854775807` is an underflow: without fraction digits, its +/// magnitude wraps past the 64-bit maximum. Measured on `GetItem`, +/// `DeleteItem`, `UpdateItem`, `BatchGetItem`, `PutItem` and the `PutItem` +/// and `DeleteItem` condition values. `0E9223372036854775806`, one less, +/// does not wrap and is an overflow (measured on `PutItem`). +/// `0.0E9223372036854775807`, with one fraction digit, does not wrap and is +/// an overflow (`GetItem`). +/// - `0.00E-9223372036854775808` and `0.000E-9223372036854775808` are +/// underflows, as every zero whose exponent is below the 32-bit range is: +/// their scale saturates instead of wrapping (`GetItem`). +fn check_zero_magnitude(explicit_exp: i64, frac_part: &str) -> Result<(), DynamoDbError> { + let scale = i64::try_from(frac_part.len()) + .unwrap_or(i64::MAX) + .saturating_sub(explicit_exp); + let magnitude = 1_i64.wrapping_sub(scale); + if magnitude > MAX_MAGNITUDE { + return Err(overflow_err()); + } + if magnitude < MIN_MAGNITUDE { + return Err(underflow_err()); + } + Ok(()) +} + /// Format digits * 10^exp as a plain decimal string (no scientific notation). fn format_plain(negative: bool, digits: &str, exp: i32) -> String { let mut result = String::new(); @@ -456,24 +494,41 @@ mod tests { ] { assert_eq!(validate_and_normalize_number(n).unwrap(), "0", "{n:?}"); } - // A zero whose exponent does not fit in an i32. DynamoDB also refuses a - // zero such as `0E126` whose exponent fits; see the comment in - // `validate_and_normalize_number` for why those are not checked. - for n in ["0E2147483648", "0E9223372036854775806"] { + // A zero whose exponent does not fit in an i32. DynamoDB refuses each; + // the rows with fraction digits and the 64-bit minimum or maximum + // exponent were sent to its GetItem, twice each. + for n in [ + "0E2147483648", + "0E9223372036854775806", + "0.0E9223372036854775807", + ] { assert_refused(n, OVERFLOW); } - for n in ["0E-9223372036854775807", "0E-9223372036854775808"] { + for n in [ + "0E-9223372036854775807", + "0E-9223372036854775808", + // The scale saturates at the 64-bit minimum exponent instead of + // wrapping, so these are underflows, as on the service. + "0.00E-9223372036854775808", + "0.000E-9223372036854775808", + // The magnitude, 1 less the scale, wraps past the 64-bit maximum, + // as it does on the service: `0E9223372036854775806` above is an + // overflow and this an underflow. + "0E9223372036854775807", + ] { 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) and ImportTable + // validate numbers, which #618 still tracks: a zero refused by this + // function keeps its raw spelling, and those paths would then look up and + // store `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_scan_and_import_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..95f2334be --- /dev/null +++ b/crates/core/src/validation/number/operations.rs @@ -0,0 +1,364 @@ +// 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. +//! +//! Measured 2026-10-27 (us-east-1, every request twice, the same answer both +//! times), the service reads a number one of two ways: +//! +//! - `GetItem`, `BatchGetItem`, `PutItem`, `UpdateItem` and `DeleteItem` read +//! it as [`validate_and_normalize_number`] does, with a 64-bit exponent. An +//! empty value answers `The parameter cannot be converted to a numeric +//! value: ` with nothing after the colon. +//! - `BatchWriteItem`, `TransactGetItems` and `TransactWriteItems` read it with +//! a 32-bit exponent and scale; see [`validate_number_i32_scale`]. An empty +//! value answers the message without the colon. +//! +//! `PutItem`, `UpdateItem` and `DeleteItem` start the message with +//! `1 validation error detected: `, and the other operations do not. +//! `TransactWriteItems` reports a value in `ExpressionAttributeValues` as +//! `ExpressionAttributeValues contains invalid value: for key +//! `. It 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. + +use std::collections::HashMap; + +use super::{ + MAX_MAGNITUDE, MAX_SIGNIFICANT_DIGITS, MIN_MAGNITUDE, overflow_err, too_many_digits_err, + underflow_err, validate_and_normalize_number, +}; +use crate::error::DynamoDbError; +use crate::types::{ + AttributeValue, BatchWriteItemInput, DeleteItemInput, GetItemInput, Item, PutItemInput, + TransactGetItem, TransactWriteItem, UpdateItemInput, +}; + +/// The service's text for a value that is not a number. +const NUMERIC_VALUE: &str = "The parameter cannot be converted to a numeric value"; + +/// The prefix `PutItem`, `UpdateItem` and `DeleteItem` put on the message. +const SINGLE_WRITE_PREFIX: &str = "1 validation error detected: "; + +/// How an operation reads a number. +#[derive(Clone, Copy)] +enum Rules { + /// `GetItem`, `BatchGetItem`, `PutItem`, `UpdateItem` and `DeleteItem`. + I64Exponent, + /// `BatchWriteItem`, `TransactGetItems` and `TransactWriteItems`. + I32Scale, +} + +/// 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(Rules::I64Exponent, &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(Rules::I64Exponent, 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(Rules::I64Exponent, &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(Rules::I64Exponent, &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 `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. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn transact_get_item(item: &TransactGetItem) -> Result<(), DynamoDbError> { + item_numbers(Rules::I32Scale, &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(Rules::I32Scale, &put.item).map_err(bare)?; + } + if let Some(delete) = &request.delete_request { + item_numbers(Rules::I32Scale, &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. +/// +/// # 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.condition_expression.as_ref(); + ( + values.and(put.expression_attribute_values.as_ref()), + &put.item, + ) + } else if let Some(delete) = &item.delete { + let values = delete.condition_expression.as_ref(); + ( + values.and(delete.expression_attribute_values.as_ref()), + &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(Rules::I32Scale, values).map_err(|(message, key)| { + DynamoDbError::ValidationException(format!( + "ExpressionAttributeValues contains invalid value: {message} for key {key}" + )) + })?; + item_numbers(Rules::I32Scale, key_or_item).map_err(bare) +} + +/// 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(Rules::I64Exponent, Some(values)).map_err(|(message, _)| message) +} + +/// The first number in `item` the service cannot read, as its message. +fn item_numbers(rules: Rules, item: &Item) -> Result<(), String> { + item.values() + .try_for_each(|value| attribute_numbers(rules, value)) +} + +/// The first number in `values` the service cannot read, as its message and +/// placeholder. Placeholders are visited in name order. +fn value_numbers( + rules: Rules, + values: Option<&HashMap>, +) -> Result<(), (String, &str)> { + let Some(values) = values else { + return Ok(()); + }; + let mut keys: Vec<&String> = values.keys().collect(); + keys.sort(); + for key in keys { + attribute_numbers(rules, &values[key]).map_err(|message| (message, key.as_str()))?; + } + Ok(()) +} + +/// 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(rules: Rules, value: &AttributeValue) -> Result<(), String> { + match value { + AttributeValue::N(n) => number_text(rules, n), + AttributeValue::NS(set) => set.iter().try_for_each(|n| number_text(rules, n)), + AttributeValue::L(list) => list.iter().try_for_each(|v| attribute_numbers(rules, v)), + AttributeValue::M(map) => map.values().try_for_each(|v| attribute_numbers(rules, v)), + _ => Ok(()), + } +} + +/// One number's text under `rules`, as the message the service gives when it +/// cannot read it. +fn number_text(rules: Rules, n: &str) -> Result<(), String> { + match rules { + Rules::I64Exponent if n.is_empty() => Err(format!("{NUMERIC_VALUE}: ")), + Rules::I64Exponent => validate_and_normalize_number(n).map(drop).map_err(message), + Rules::I32Scale => validate_number_i32_scale(n), + } +} + +/// Check `s` as the service reads a number on `BatchWriteItem`, +/// `TransactGetItems` and `TransactWriteItems`, and return the message for the +/// first rule it breaks (measured 2026-10-27, us-east-1, every value twice): +/// +/// 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. +fn validate_number_i32_scale(s: &str) -> Result<(), String> { + if s.is_empty() { + return Err(NUMERIC_VALUE.to_owned()); + } + let malformed = || format!("{NUMERIC_VALUE}: {s}"); + 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(|_| 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(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_else(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(|_| malformed())?; + let magnitude = precision.wrapping_sub(scale); + if i64::from(magnitude) > MAX_MAGNITUDE { + return Err(message(overflow_err())); + } + if i64::from(magnitude) < MIN_MAGNITUDE { + return Err(message(underflow_err())); + } + if unscaled.trim_end_matches('0').len() > MAX_SIGNIFICANT_DIGITS { + return Err(message(too_many_digits_err())); + } + 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..1d53375f7 --- /dev/null +++ b/crates/core/src/validation/number/operations/tests.rs @@ -0,0 +1,589 @@ +// Copyright 2026 ExtendDB contributors +// SPDX-License-Identifier: Apache-2.0 + +use serde_json::{Value, json}; + +use super::*; +use crate::types::{BatchWriteItemInput, TransactGetItem, TransactWriteItem}; + +// 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 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:?}"), + } +} + +fn i32_scale(n: &str) -> Result<(), String> { + validate_number_i32_scale(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())); +} + +/// A deterministic xorshift generator for the reader comparison below. +struct Generator(u64); + +impl Generator { + fn below(&mut self, bound: u64) -> u64 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 7; + self.0 ^= self.0 << 17; + self.0 % bound + } + + fn pick(&mut self, from: &[T]) -> T { + from[usize::try_from(self.below(from.len() as u64)).unwrap()] + } + + /// `count` digits, about half of them zeros. + fn digits(&mut self, count: usize) -> String { + (0..count) + .map(|_| self.pick(&['0', '0', '0', '1', '2', '5', '9'])) + .collect() + } + + /// A string shaped like a number, malformed about one time in twenty. + fn number_like(&mut self) -> String { + let mut s = self.pick(&["", "", "+", "-"]).to_owned(); + let integer_digits = self.pick(&[0, 1, 2, 3, 38, 39, 45]); + s.push_str(&self.digits(integer_digits)); + if self.below(2) == 0 { + s.push('.'); + let fraction_digits = self.pick(&[0, 1, 2, 5, 39, 45]); + s.push_str(&self.digits(fraction_digits)); + } + if self.below(2) == 0 { + s.push_str(self.pick(&["e", "E", "E+", "e-", "E-"])); + let exponent = match self.below(5) { + 0 => self.below(300), + 1 => 120 + self.below(15), + 2 => 2_147_483_640 + self.below(16), + 3 => 9_223_372_036_854_775_800 + self.below(9), + _ => self.below(10_000_000_000), + }; + s.push_str(&exponent.to_string()); + } + if self.below(20) == 0 { + // The string is ASCII so far, so every index is a char boundary. + let at = usize::try_from(self.below(s.len() as u64 + 1)).unwrap(); + let junk = self.pick(&[' ', 'a', '.', '+', 'e', '\u{b2}', '\u{ff15}']); + s.insert(at, junk); + } + s + } +} + +#[test] +fn the_two_readers_agree_outside_the_documented_differences() { + // `validate_number_i32_scale` and `validate_and_normalize_number` read + // one grammar. They must accept the same values and refuse the others + // with the same message, except in four cases the docs describe: + // 1. a zero whose exponent fits in 32 bits, which the 64-bit reader + // accepts as 0 (#618) and the 32-bit reader range checks; + // 2. an exponent, a scale (fraction digits less the exponent) or a + // magnitude (digits less the scale) that does not fit in 32 bits, + // where the 32-bit reader cannot convert the value or wraps its + // magnitude, and both refuse it; + // 3. more than 38 digits and a magnitude out of range, where the 32-bit + // reader names the magnitude and the 64-bit reader the digit count; + // 4. a zero whose exponent fits in 32 bits and whose scale does not + // (`0.0E-2147483648`), which the 64-bit reader accepts as 0 and the + // 32-bit reader cannot convert. + // The empty value's text differs too (module doc), and is tested above. + let mut generator = Generator(0x9E37_79B9_7F4A_7C15); + let mut agreed = 0; + for _ in 0..4_000 { + let s = generator.number_like(); + if s.is_empty() { + continue; + } + let narrow = i32_scale(&s); + let wide = validate_and_normalize_number(&s); + let wide_answer = wide.clone().map(drop).map_err(message); + if narrow == wide_answer { + agreed += 1; + continue; + } + // The exponent as written, when it is a plain signed integer, and the + // scale it gives with the fraction digits. + let (mantissa, exponent) = s + .split_once(['e', 'E']) + .map_or((s.as_str(), None), |(m, e)| (m, e.parse::().ok())); + let fraction_digits = mantissa.split_once('.').map_or(0, |(_, f)| f.len()); + let digits: String = mantissa.chars().filter(char::is_ascii_digit).collect(); + let precision = digits.trim_start_matches('0').len().max(1); + let exponent_wide = exponent.is_some_and(|e| i32::try_from(e).is_err()); + let scale = exponent.map(|e| i128::try_from(fraction_digits).unwrap() - i128::from(e)); + let scale_wide = scale.is_some_and(|scale| i32::try_from(scale).is_err()); + let magnitude_wide = scale.is_some_and(|scale| { + i32::try_from(i128::try_from(precision).unwrap() - scale).is_err() + }); + let magnitude = |m: &str| m == OVERFLOW || m == UNDERFLOW; + let malformed = |m: &str| m.starts_with(NUMERIC); + let documented = match (&narrow, &wide) { + (Err(m), Ok(zero)) => { + zero == "0" && (magnitude(m) || (malformed(m) && !exponent_wide && scale_wide)) + } + (Err(_), Err(_)) if exponent_wide || scale_wide || magnitude_wide => true, + (Err(m), Err(_)) => magnitude(m) && wide_answer == Err(DIGITS.to_owned()), + (Ok(()), _) => false, + }; + assert!( + documented, + "{s:?}: 32-bit reader {narrow:?}, 64-bit reader {wide_answer:?}" + ); + } + assert!(agreed > 2_000, "only {agreed} of the values agreed"); +} + +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: an overflow where the transaction paths + // answer an underflow, 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_uses_the_32_bit_rules() { + 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"))), + UNDERFLOW + ); + assert_eq!( + refused(transact_get_item(&item("1E2147483648"))), + format!("{NUMERIC}: 1E2147483648") + ); + assert_eq!(refused(transact_get_item(&item(""))), 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(), + ), + ( + json!({"PutRequest": {"Item": {"pk": {"S": "a"}, "n": {"N": "10E2147483647"}}}}), + UNDERFLOW.to_owned(), + ), + ( + json!({"DeleteRequest": {"Key": {"pk": {"N": "1E126"}}}}), + OVERFLOW.to_owned(), + ), + ( + json!({"DeleteRequest": {"Key": {"pk": {"N": ""}}}}), + 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() +} + +fn wrapped(message: &str, key: &str) -> String { + format!("ExpressionAttributeValues contains invalid value: {message} for key {key}") +} + +#[test] +fn transact_write_item_checks_values_then_key_or_item() { + // Values come before the key: the service names the 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)), + wrapped(&format!("{NUMERIC}: xyz"), ":v") + ); + let check = transact(json!({"ConditionCheck": { + "TableName": "t", + "Key": {"pk": {"S": "a"}}, + "ConditionExpression": "n = :v", + "ExpressionAttributeValues": {":v": {"N": ""}}, + }})); + assert_eq!(refused(transact_write_item(&check)), wrapped(NUMERIC, ":v")); + 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)), wrapped(OVERFLOW, ":v")); + let delete = transact(json!({"Delete": { + "TableName": "t", + "Key": {"pk": {"N": "1E2147483647"}}, + }})); + assert_eq!(refused(transact_write_item(&delete)), UNDERFLOW); +} + +#[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})))), + wrapped(&format!("{NUMERIC}: abc"), ":v"), + "{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") + ); +} + +/// 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")); +} 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 b0381c7a7..66e011669 100755 --- a/crates/engine/src/batch_write_item.rs +++ b/crates/engine/src/batch_write_item.rs @@ -65,6 +65,11 @@ 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). + extenddb_core::validation::number::operations::batch_write_item(&input)?; let empty_maps = ExpressionMaps::default(); let mut all_icm: HashMap> = diff --git a/crates/engine/src/delete_item.rs b/crates/engine/src/delete_item.rs index 049c368f6..e006d9455 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)?; // Validate expressions/`Expected` before the existence check, // so a missing table returns ValidationException, not ResourceNotFoundException. @@ -68,6 +72,12 @@ 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)?; 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/put_item.rs b/crates/engine/src/put_item.rs index ffd6f70b4..186030509 100755 --- a/crates/engine/src/put_item.rs +++ b/crates/engine/src/put_item.rs @@ -94,6 +94,14 @@ pub async fn handle_put_item( } })?; + // 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. + // 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 @@ -109,6 +117,12 @@ 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)?; 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 43fb78a1c..fa8ab545b 100755 --- a/crates/engine/src/transact_write_items.rs +++ b/crates/engine/src/transact_write_items.rs @@ -75,6 +75,13 @@ 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). + if let Some(ref token) = input.client_request_token { + validate_client_request_token(token)?; + } + // Resolve table key info, validate inputs, parse expressions, and check // for duplicate targets — all in a single pass to avoid redundant // `table_key_info` lookups. @@ -82,6 +89,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( @@ -148,10 +162,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 7f74c205c..3b329d866 100755 --- a/crates/engine/src/update_item.rs +++ b/crates/engine/src/update_item.rs @@ -61,6 +61,10 @@ pub async fn handle_update_item( 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)?; // Validate Update/Condition expressions (and AttributeUpdates // desugaring) before the existence check; key, item-content nesting, and From 909482b2d757be0c6fe7a8243b3abcfc2026e1e5 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Tue, 27 Oct 2026 15:27:59 +0000 Subject: [PATCH 2/2] test(wire): number validation and its order on every operation number_validation_all_ops covers each validated path with DynamoDB's message for abc, 1E126 and the 32-bit edge values, a refused batch or transaction writing nothing, and valid zeros stored as 0. number_validation_order pins the position of each check against a second fault in the same request: an absent table, a mixed-parameter error, an unused name, a projection or condition syntax error in another item, a duplicate key and a long ClientRequestToken. Each expectation is a DynamoDB answer measured twice on 2026-10-27. Part of #618 --- tests/rust/src/main.rs | 4 + tests/rust/src/number_validation_all_ops.rs | 557 ++++++++++++++++++++ tests/rust/src/number_validation_order.rs | 219 ++++++++ 3 files changed, 780 insertions(+) create mode 100644 tests/rust/src/number_validation_all_ops.rs create mode 100644 tests/rust/src/number_validation_order.rs diff --git a/tests/rust/src/main.rs b/tests/rust/src/main.rs index 965d44837..4df92cc65 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; 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..e500ea2b0 --- /dev/null +++ b/tests/rust/src/number_validation_all_ops.rs @@ -0,0 +1,557 @@ +// 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`, and the +//! `ExpressionAttributeValues` of `PutItem`, `DeleteItem` and +//! `TransactWriteItems`. +//! +//! Every message below was measured against DynamoDB on 2026-10-27 +//! (us-east-1), each request sent twice with the same answer both times. +//! `PutItem`, `UpdateItem` and `DeleteItem` start the message with +//! "1 validation error detected: " and the other operations do not. +//! `TransactWriteItems` reports a value in `ExpressionAttributeValues` 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::{ + AttributeValue, ConditionCheck, Delete, DeleteRequest, 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"; + +/// What `TransactWriteItems` answers for a value in `ExpressionAttributeValues`. +fn in_values(message: &str) -> String { + format!("ExpressionAttributeValues contains invalid value: {message} for key :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() { + 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(), &in_values(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(), &in_values(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(), &in_values(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(), &in_values(OVERFLOW)); +} + +// ---- 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; + let err = refused(result); + assert!( + err.starts_with("ValidationException:") + && err.contains("at 'clientRequestToken' failed to satisfy constraint"), + "expected the token's constraint message, got: {err}" + ); +} + +#[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 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..722305961 --- /dev/null +++ b/tests/rust/src/number_validation_order.rs @@ -0,0 +1,219 @@ +// 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, +//! 2026-10-27, each request sent twice with the same answer both times), +//! named in the test's comment. +//! +//! 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, TransactGetItem, +}; + +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); +}