From eb370a39aa5f4ddb0266ff4923d58d48ec95c1fe Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sat, 24 Oct 2026 23:36:41 +0000 Subject: [PATCH 1/7] fix(query): check Query values against the key schema and the number rules A Query with a KeyConditionExpression did not check a key condition value's type against the key attribute's AttributeDefinitions type, and did not check that an N value in ExpressionAttributeValues is a number. The storage layer compared the value's inner text with the stored key and ignored the type tag, so {"N": "bob"} on an S partition key read the bob partition while authorization bound no dynamodb:LeadingKeys value to it, and a sort key value of another type answered InternalServerError on SQLite. Add the two checks the service applies before it reads a row, on the expression path (crates/engine/src/query_key_values.rs): - an N value in ExpressionAttributeValues that is not a number answers "The parameter cannot be converted to a numeric value: " (the overflow and 38-digit texts for those cases; an empty value answers the message with the colon). The check covers the whole map, including NS members and values nested in L and M, whether the key condition, the filter or nothing reads them, with the walk PutItem and UpdateItem already use. It runs at request time after the Limit and expression parameter mix checks and before the table lookup, which moves those two checks ahead of the table lookup as well. The server runs the same request-time checks ahead of the IAM decision, so a principal under any LeadingKeys fence form gets ValidationException and no items; - a key condition value whose type is not the key attribute's type answers "One or more parameter values were invalid: Condition parameter type does not match schema type", for the partition key, the sort key under every comparison operator, both BETWEEN bounds, the begins_with prefix and multi-attribute index keys, after the key schema, filter, projection, unused value and Select checks and before the ExclusiveStartKey check. When a compared placeholder is undefined the check returns without a verdict and the undefined placeholder is reported. Measured on the service on 2026-10-24 (113 requests, each sent twice). The legacy KeyConditions path is unchanged; its checks land with #507. Scan is unchanged. Unit tests at the checks and a dual-target wire test file, tests/test_query_key_value_types.py, which fails on main. --- crates/core/src/validation/mod.rs | 10 +- crates/engine/src/lib.rs | 1 + crates/engine/src/query.rs | 59 +-- crates/engine/src/query_key_values.rs | 224 +++++++++ crates/engine/src/query_key_values/tests.rs | 395 ++++++++++++++++ crates/server/src/request_helpers.rs | 14 + tests/test_query_key_value_types.py | 489 ++++++++++++++++++++ 7 files changed, 1156 insertions(+), 36 deletions(-) create mode 100644 crates/engine/src/query_key_values.rs create mode 100644 crates/engine/src/query_key_values/tests.rs create mode 100644 tests/test_query_key_value_types.py diff --git a/crates/core/src/validation/mod.rs b/crates/core/src/validation/mod.rs index c144b46b6..02e24e618 100755 --- a/crates/core/src/validation/mod.rs +++ b/crates/core/src/validation/mod.rs @@ -1903,7 +1903,15 @@ fn validate_lsi_requires_range_key(input: &CreateTableInput) -> Result<(), Dynam Ok(()) } -fn validate_attribute_number(value: &AttributeValue) -> Result<(), DynamoDbError> { +/// Validate every number inside one attribute value (`N`, `NS` members, and +/// the values nested in `L` and `M`) against the `DynamoDB` number rules, with +/// the bare numeric message. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` for the first number that is +/// not a number, has more than 38 significant digits, or is out of range. +pub fn validate_attribute_number(value: &AttributeValue) -> Result<(), DynamoDbError> { match value { AttributeValue::N(n) => { number::validate_and_normalize_number(n)?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 74083fbfa..d24b47286 100755 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -27,6 +27,7 @@ mod legacy_filter; mod list_tables; mod put_item; mod query; +pub mod query_key_values; mod read_helpers; mod request_limits; mod scan; diff --git a/crates/engine/src/query.rs b/crates/engine/src/query.rs index f45526af8..97e07ead2 100755 --- a/crates/engine/src/query.rs +++ b/crates/engine/src/query.rs @@ -71,6 +71,14 @@ pub async fn handle_query( )?; let input: QueryInput = serde_json::from_value(body).map_err(crate::deserialize_error)?; + // Request-time checks in the service's order: Limit, the expression and + // non-expression parameter mix, then every number in + // ExpressionAttributeValues (measured 2026-10-24: a Limit of 0 and a + // parameter mix are reported ahead of an N value that is not a number, and + // that value is reported ahead of a missing table or index). The server + // runs the same function ahead of the IAM decision; see `query_key_values`. + crate::query_key_values::validate_query_request_values(&input)?; + // P118: Fetch key_info first so we can use table_id for index lookup. let key_info = ctx .table_key_info(&input.table_name) @@ -137,14 +145,7 @@ pub async fn handle_query( )?; } - // Validate Limit >= 1 (REQ-QUERY-001) - if let Some(limit) = input.limit - && limit < 1 - { - return Err(DynamoDbError::ValidationException( - "1 validation error detected: Value at 'Limit' failed to satisfy constraint: Member must have value greater than or equal to 1".to_owned(), - )); - } + // Limit >= 1 is checked at request time in validate_query_request_values. // For index queries, build a key_info that reflects the index's key schema // so the storage layer uses the correct SK column for the index table. @@ -166,35 +167,10 @@ pub async fn handle_query( key_info.clone() }; - // --- Legacy vs expression mutual exclusivity checks --- - let kce_present = input.key_condition_expression.is_some(); - let has_kc = input.key_conditions.as_ref().is_some_and(|m| !m.is_empty()); - let has_fe = input.filter_expression.is_some(); - let has_qf = input.query_filter.as_ref().is_some_and(|m| !m.is_empty()); - let has_pe = input.projection_expression.is_some(); - let has_atg = input - .attributes_to_get - .as_ref() - .is_some_and(|a| !a.is_empty()); + // The legacy versus expression mutual exclusivity check runs at request + // time in validate_query_request_values. let has_cond_op = input.conditional_operator.is_some(); - // KeyConditions legitimately combines with FilterExpression and - // ProjectionExpression, so it joins the mixing check only when - // KeyConditionExpression is also present. - extenddb_core::validation::validate_no_expression_param_mixing( - &[ - ("AttributesToGet", has_atg), - ("QueryFilter", has_qf), - ("ConditionalOperator", has_cond_op), - ("KeyConditions", has_kc && kce_present), - ], - &[ - ("ProjectionExpression", has_pe), - ("FilterExpression", has_fe), - ("KeyConditionExpression", kce_present), - ], - )?; - // An explicitly empty KeyConditionExpression is rejected up front, before // the expression-parameter-usage checks (matching real DynamoDB). reject_empty_key_condition_expression(input.key_condition_expression.as_deref())?; @@ -471,6 +447,19 @@ pub async fn handle_query( )?; } + // A key condition value whose type is not the key attribute's type is + // refused after the key schema, filter, projection, unused value and + // Select checks and before the ExclusiveStartKey check (measured + // 2026-10-24). The legacy KeyConditions path makes its own checks when it + // desugars. + if legacy_kc_maps.is_none() { + crate::query_key_values::validate_key_condition_types( + &key_condition, + effective_maps, + &query_key_info.attribute_definitions, + )?; + } + // Validate ExclusiveStartKey matches the key schema if let Some(ref start_key) = input.exclusive_start_key { validate_query_exclusive_start_key(start_key, &key_info, index_info.as_ref())?; diff --git a/crates/engine/src/query_key_values.rs b/crates/engine/src/query_key_values.rs new file mode 100644 index 000000000..e49968648 --- /dev/null +++ b/crates/engine/src/query_key_values.rs @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Checks on the values a `Query` carries, as the service applies them before +//! any row is read. +//! +//! Measured on 2026-10-24 (tables with S/S, S/N and B/B keys, a GSI and an +//! LSI): +//! +//! 1. an `N` value in `ExpressionAttributeValues` that is not a number answers +//! `The parameter cannot be converted to a numeric value: ` (or the +//! overflow, underflow and 38-digit messages). Every value in the map is +//! checked, whether the key condition, the filter or nothing reads it. The +//! check runs at request time, after the `Limit` check and the expression +//! and non-expression parameter mix check, and before the table is looked +//! up; see [`validate_query_request_values`]; +//! 2. a key condition value whose type is not the key attribute's +//! `AttributeDefinitions` type answers `One or more parameter values were +//! invalid: Condition parameter type does not match schema type`, on the +//! partition and the sort key, on a table, a GSI and an LSI, for `=`, the +//! ordering operators, `BETWEEN` and `begins_with`. The check runs after the +//! key schema, filter, projection, unused value and `Select` checks and +//! before the `ExclusiveStartKey` check; see +//! [`validate_key_condition_types`]. +//! +//! Without these checks a backend compares the value's inner text with the +//! stored key and ignores the type tag, so `{"N": "bob"}` on an S key reads the +//! `bob` partition, while authorization binds no `dynamodb:LeadingKeys` value +//! to a number it cannot parse. + +use std::collections::HashMap; + +use extenddb_core::error::DynamoDbError; +use extenddb_core::expression::{ + Expr, ExpressionMaps, KeyCondition, PathElement, SortKeyCondition, +}; +use extenddb_core::types::{AttributeDefinition, AttributeValue, QueryInput, ScalarAttributeType}; + +/// The service's text for a key condition value of the wrong type. +pub const KEY_CONDITION_TYPE_MISMATCH: &str = "One or more parameter values were invalid: Condition parameter type does not match schema type"; + +/// The service's text for a `Limit` below 1 on `Query`. +pub const LIMIT_BELOW_ONE: &str = "1 validation error detected: Value at 'Limit' failed to satisfy constraint: Member must have value greater than or equal to 1"; + +/// The bare numeric message the number validator uses for an empty `N`. +const EMPTY_NUMBER: &str = "The parameter cannot be converted to a numeric value"; + +/// The `Query` checks that run on the request alone, before the table is +/// looked up, in the service's order: `Limit`, the expression and +/// non-expression parameter mix, then every number in +/// `ExpressionAttributeValues`. +/// +/// The server runs the same function ahead of the IAM decision, so a principal +/// whose `dynamodb:LeadingKeys` fence cannot bind a number that does not parse +/// gets the request's `ValidationException` rather than +/// `AccessDeniedException`. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn validate_query_request_values(input: &QueryInput) -> Result<(), DynamoDbError> { + if let Some(limit) = input.limit + && limit < 1 + { + return Err(DynamoDbError::ValidationException( + LIMIT_BELOW_ONE.to_owned(), + )); + } + + let kce_present = input.key_condition_expression.is_some(); + let has_kc = input.key_conditions.as_ref().is_some_and(|m| !m.is_empty()); + let has_qf = input.query_filter.as_ref().is_some_and(|m| !m.is_empty()); + let has_atg = input + .attributes_to_get + .as_ref() + .is_some_and(|a| !a.is_empty()); + // KeyConditions combines with FilterExpression and ProjectionExpression, so + // it joins the mix check only when KeyConditionExpression is also present. + extenddb_core::validation::validate_no_expression_param_mixing( + &[ + ("AttributesToGet", has_atg), + ("QueryFilter", has_qf), + ("ConditionalOperator", input.conditional_operator.is_some()), + ("KeyConditions", has_kc && kce_present), + ], + &[ + ( + "ProjectionExpression", + input.projection_expression.is_some(), + ), + ("FilterExpression", input.filter_expression.is_some()), + ("KeyConditionExpression", kce_present), + ], + )?; + + if let Some(values) = &input.expression_attribute_values { + validate_expression_attribute_value_numbers(values)?; + } + Ok(()) +} + +/// Refuse the first `N` value in `ExpressionAttributeValues` (including `NS` +/// members and values nested in `L` and `M`) that is not a number, with the +/// bare numeric message: no `ExpressionAttributeValues contains invalid value` +/// wrapper, which the service uses on `UpdateItem` and not on `Query`. The +/// placeholders are visited in sorted order so the reported value is +/// deterministic when several are invalid. An empty `N` answers the message +/// with the colon and nothing after it, as the service does on `Query`. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with the service's text. +pub fn validate_expression_attribute_value_numbers( + values: &HashMap, +) -> Result<(), DynamoDbError> { + let mut keys: Vec<&String> = values.keys().collect(); + keys.sort(); + for key in keys { + match extenddb_core::validation::validate_attribute_number(&values[key]) { + Ok(()) => {} + Err(DynamoDbError::ValidationException(msg)) if msg == EMPTY_NUMBER => { + return Err(DynamoDbError::ValidationException(format!( + "{EMPTY_NUMBER}: " + ))); + } + Err(other) => return Err(other), + } + } + Ok(()) +} + +/// Refuse a key condition value whose type is not the key's +/// `AttributeDefinitions` type. +/// +/// Every value the condition compares is checked: the partition key value(s), +/// the sort key value (or both `BETWEEN` bounds, or the `begins_with` prefix) +/// and the extra equality conditions of a multi-attribute key schema. A path +/// that does not resolve to an attribute name, a value that is not a +/// placeholder, and a key attribute with no definition are left to the checks +/// that already refuse them. When any compared placeholder is undefined the +/// whole check is skipped: the service reports the undefined placeholder +/// first, and the storage layer does so here. +/// +/// # Errors +/// +/// Returns `DynamoDbError::ValidationException` with +/// [`KEY_CONDITION_TYPE_MISMATCH`]. +pub fn validate_key_condition_types( + key_condition: &KeyCondition, + maps: &ExpressionMaps, + attribute_definitions: &[AttributeDefinition], +) -> Result<(), DynamoDbError> { + let mut pairs: Vec<(&[PathElement], &Expr)> = + vec![(&key_condition.pk_path, &key_condition.pk_value)]; + for (path, value) in &key_condition.extra_pk_conditions { + pairs.push((path, value)); + } + match &key_condition.sk_condition { + Some(SortKeyCondition::Compare { path, value, .. }) => pairs.push((path, value)), + Some(SortKeyCondition::Between { path, low, high }) => { + pairs.push((path, low)); + pairs.push((path, high)); + } + Some(SortKeyCondition::BeginsWith { path, prefix }) => pairs.push((path, prefix)), + None => {} + } + for (path, value) in &key_condition.extra_sk_conditions { + pairs.push((path, value)); + } + + let any_undefined = pairs.iter().any( + |(_, value)| matches!(value, Expr::Placeholder(name) if !maps.values.contains_key(name)), + ); + if any_undefined { + return Ok(()); + } + + for (path, value) in pairs { + let Some(attr_name) = resolve_attr_name(path, &maps.names) else { + continue; + }; + let Some(expected) = attribute_definitions + .iter() + .find(|ad| ad.attribute_name == attr_name) + .map(|ad| ad.attribute_type) + else { + continue; + }; + let Expr::Placeholder(name) = value else { + continue; + }; + let Some(av) = maps.values.get(name) else { + continue; + }; + if !type_matches(expected, av) { + return Err(DynamoDbError::ValidationException( + KEY_CONDITION_TYPE_MISMATCH.to_owned(), + )); + } + } + Ok(()) +} + +fn type_matches(expected: ScalarAttributeType, value: &AttributeValue) -> bool { + matches!( + (expected, value), + (ScalarAttributeType::S, AttributeValue::S(_)) + | (ScalarAttributeType::N, AttributeValue::N(_)) + | (ScalarAttributeType::B, AttributeValue::B(_)) + ) +} + +/// The top-level attribute name of a key path, through `#name` references. +fn resolve_attr_name(path: &[PathElement], names: &HashMap) -> Option { + match path.first()? { + PathElement::Attribute(name) => match name.strip_prefix('#') { + Some(reference) => names.get(reference).cloned(), + None => Some(name.clone()), + }, + PathElement::Index(_) => None, + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/engine/src/query_key_values/tests.rs b/crates/engine/src/query_key_values/tests.rs new file mode 100644 index 000000000..6130dd875 --- /dev/null +++ b/crates/engine/src/query_key_values/tests.rs @@ -0,0 +1,395 @@ +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; + +use extenddb_core::error::DynamoDbError; +use extenddb_core::expression::{ + ExpressionMaps, KeyCondition, parse_key_condition, tokenize_with_limit, +}; +use extenddb_core::types::{AttributeDefinition, AttributeValue, QueryInput, ScalarAttributeType}; + +use super::{ + KEY_CONDITION_TYPE_MISMATCH, LIMIT_BELOW_ONE, validate_expression_attribute_value_numbers, + validate_key_condition_types, validate_query_request_values, +}; + +fn defs(pairs: &[(&str, ScalarAttributeType)]) -> Vec { + pairs + .iter() + .map(|(name, ty)| AttributeDefinition { + attribute_name: (*name).to_owned(), + attribute_type: *ty, + }) + .collect() +} + +fn parse(kce: &str) -> KeyCondition { + let tokens = tokenize_with_limit(kce, 1000).expect("tokenize"); + parse_key_condition(&tokens).expect("parse") +} + +fn maps(names: &[(&str, &str)], values: &[(&str, AttributeValue)]) -> ExpressionMaps { + ExpressionMaps::new( + names + .iter() + .map(|(k, v)| ((*k).to_owned(), (*v).to_owned())) + .collect(), + values + .iter() + .map(|(k, v)| ((*k).to_owned(), v.clone())) + .collect(), + ) +} + +fn s(v: &str) -> AttributeValue { + AttributeValue::S(v.to_owned()) +} + +fn n(v: &str) -> AttributeValue { + AttributeValue::N(v.to_owned()) +} + +fn message(result: Result<(), DynamoDbError>) -> Option { + match result { + Ok(()) => None, + Err(DynamoDbError::ValidationException(msg)) => Some(msg), + Err(other) => panic!("unexpected error {other:?}"), + } +} + +const SS: &[(&str, ScalarAttributeType)] = &[ + ("pk", ScalarAttributeType::S), + ("sk", ScalarAttributeType::S), +]; + +#[test] +fn partition_value_of_another_type_is_refused() { + let kc = parse("pk = :u"); + for wrong in [ + n("5"), + AttributeValue::B(vec![1]), + AttributeValue::Bool(true), + AttributeValue::Null, + ] { + let m = maps(&[], &[("u", wrong)]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &defs(SS))).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH) + ); + } +} + +#[test] +fn matching_types_pass() { + let kc = parse("pk = :u AND sk BETWEEN :a AND :b"); + let m = maps(&[], &[("u", s("x")), ("a", s("a")), ("b", s("b"))]); + assert!(validate_key_condition_types(&kc, &m, &defs(SS)).is_ok()); + + let kc = parse("pk = :u AND begins_with(sk, :p)"); + let m = maps(&[], &[("u", s("x")), ("p", s("a"))]); + assert!(validate_key_condition_types(&kc, &m, &defs(SS)).is_ok()); + + let n_sk = defs(&[ + ("pk", ScalarAttributeType::S), + ("sk", ScalarAttributeType::N), + ]); + let kc = parse("pk = :u AND sk > :v"); + let m = maps(&[], &[("u", s("x")), ("v", n("5"))]); + assert!(validate_key_condition_types(&kc, &m, &n_sk).is_ok()); +} + +#[test] +fn sort_key_value_of_another_type_is_refused_for_every_operator() { + for kce in [ + "pk = :u AND sk = :v", + "pk = :u AND sk < :v", + "pk = :u AND sk <= :v", + "pk = :u AND sk > :v", + "pk = :u AND sk >= :v", + "pk = :u AND :v <= sk", + "pk = :u AND begins_with(sk, :v)", + ] { + let kc = parse(kce); + let m = maps(&[], &[("u", s("x")), ("v", n("5"))]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &defs(SS))).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH), + "{kce}" + ); + } +} + +#[test] +fn either_between_bound_of_another_type_is_refused() { + let kc = parse("pk = :u AND sk BETWEEN :a AND :b"); + for (a, b) in [(n("1"), s("b")), (s("a"), n("2")), (n("1"), n("2"))] { + let m = maps(&[], &[("u", s("x")), ("a", a), ("b", b)]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &defs(SS))).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH) + ); + } +} + +#[test] +fn string_on_a_number_or_binary_key_is_refused() { + let n_sk = defs(&[ + ("pk", ScalarAttributeType::S), + ("sk", ScalarAttributeType::N), + ]); + let kc = parse("pk = :u AND sk = :v"); + let m = maps(&[], &[("u", s("x")), ("v", s("5"))]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &n_sk)).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH) + ); + + let b_pk = defs(&[ + ("bpk", ScalarAttributeType::B), + ("bsk", ScalarAttributeType::B), + ]); + let kc = parse("bpk = :u"); + for wrong in [s("x"), n("5")] { + let m = maps(&[], &[("u", wrong)]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &b_pk)).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH) + ); + } +} + +#[test] +fn name_reference_resolves_to_the_defined_attribute() { + let kc = parse("#p = :u"); + let m = maps(&[("p", "pk")], &[("u", n("5"))]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &defs(SS))).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH) + ); + let m = maps(&[("p", "pk")], &[("u", s("x"))]); + assert!(validate_key_condition_types(&kc, &m, &defs(SS)).is_ok()); +} + +#[test] +fn index_key_schema_uses_the_index_attribute_definition() { + // A GSI on gpk (S) with a range key gsk (S) on a table whose base sort key is N. + let d = defs(&[ + ("pk", ScalarAttributeType::S), + ("sk", ScalarAttributeType::N), + ("gpk", ScalarAttributeType::S), + ("gsk", ScalarAttributeType::S), + ]); + let kc = parse("gpk = :g AND gsk = :s"); + let m = maps(&[], &[("g", s("x")), ("s", n("5"))]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &d)).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH) + ); + let m = maps(&[], &[("g", s("x")), ("s", s("g"))]); + assert!(validate_key_condition_types(&kc, &m, &d).is_ok()); +} + +#[test] +fn multipart_hash_conditions_are_checked() { + let d = defs(&[ + ("g1", ScalarAttributeType::S), + ("g2", ScalarAttributeType::N), + ("r", ScalarAttributeType::S), + ]); + let mut kc = parse("g1 = :a AND g2 = :b AND r > :c"); + let names = HashMap::new(); + kc.resolve_multipart(&["g1", "g2"], &names) + .expect("resolve"); + assert_eq!(kc.extra_pk_conditions.len(), 1); + let m = maps(&[], &[("a", s("x")), ("b", s("1")), ("c", s("r"))]); + assert_eq!( + message(validate_key_condition_types(&kc, &m, &d)).as_deref(), + Some(KEY_CONDITION_TYPE_MISMATCH) + ); + let m = maps(&[], &[("a", s("x")), ("b", n("1")), ("c", s("r"))]); + assert!(validate_key_condition_types(&kc, &m, &d).is_ok()); +} + +#[test] +fn undefined_placeholder_and_undefined_attribute_are_left_to_other_checks() { + let kc = parse("pk = :u AND sk = :zz"); + let m = maps(&[], &[("u", n("5"))]); + // The partition value is wrong, and :zz is undefined: the service reports + // the undefined placeholder first, so the type check stands aside. + assert!(validate_key_condition_types(&kc, &m, &defs(SS)).is_ok()); + let m = maps(&[], &[("u", s("x"))]); + assert!(validate_key_condition_types(&kc, &m, &defs(SS)).is_ok()); + + // No AttributeDefinition for the attribute: not this check's refusal. + let kc = parse("other = :u"); + let m = maps(&[], &[("u", n("5"))]); + assert!(validate_key_condition_types(&kc, &m, &defs(SS)).is_ok()); +} + +fn query(body: serde_json::Value) -> QueryInput { + serde_json::from_value(body).expect("QueryInput") +} + +#[test] +fn number_values_that_are_not_numbers_are_refused_with_the_bare_message() { + let cases = [ + ( + "bob", + "The parameter cannot be converted to a numeric value: bob", + ), + ( + "1e", + "The parameter cannot be converted to a numeric value: 1e", + ), + ( + "12abc", + "The parameter cannot be converted to a numeric value: 12abc", + ), + ( + " 5", + "The parameter cannot be converted to a numeric value: 5", + ), + ("", "The parameter cannot be converted to a numeric value: "), + ( + "1e200", + "Number overflow. Attempting to store a number with magnitude larger than supported range", + ), + ]; + for (raw, expected) in cases { + let values = HashMap::from([(":u".to_owned(), n(raw))]); + assert_eq!( + message(validate_expression_attribute_value_numbers(&values)).as_deref(), + Some(expected), + "{raw:?}" + ); + } + let values = HashMap::from([(":u".to_owned(), n(&"1".repeat(39)))]); + assert_eq!( + message(validate_expression_attribute_value_numbers(&values)).as_deref(), + Some("Attempting to store more than 38 significant digits in a Number") + ); +} + +#[test] +fn number_values_that_are_numbers_and_other_types_pass() { + let values = HashMap::from([ + (":u".to_owned(), n("5")), + (":v".to_owned(), n("-1.5e3")), + (":w".to_owned(), s("bob")), + (":x".to_owned(), AttributeValue::Bool(true)), + (":y".to_owned(), AttributeValue::L(vec![n("1"), s("bob")])), + ]); + assert!(validate_expression_attribute_value_numbers(&values).is_ok()); +} + +#[test] +fn every_value_in_the_map_is_checked_whatever_reads_it() { + // :n is read by nothing in the key condition: checked all the same. + let values = HashMap::from([(":p".to_owned(), s("x")), (":n".to_owned(), n("bob"))]); + assert_eq!( + message(validate_expression_attribute_value_numbers(&values)).as_deref(), + Some("The parameter cannot be converted to a numeric value: bob") + ); +} + +#[test] +fn numbers_nested_in_sets_lists_and_maps_are_checked() { + let nested = [ + AttributeValue::NS(["1".to_owned(), "bob".to_owned()].into_iter().collect()), + AttributeValue::L(vec![s("a"), n("bob")]), + AttributeValue::M([("a".to_owned(), n("bob"))].into_iter().collect()), + AttributeValue::M( + [( + "a".to_owned(), + AttributeValue::L(vec![AttributeValue::M( + [("b".to_owned(), n("bob"))].into_iter().collect(), + )]), + )] + .into_iter() + .collect(), + ), + ]; + for value in nested { + let values = HashMap::from([(":p".to_owned(), s("x")), (":n".to_owned(), value.clone())]); + assert_eq!( + message(validate_expression_attribute_value_numbers(&values)).as_deref(), + Some("The parameter cannot be converted to a numeric value: bob"), + "{value:?}" + ); + } +} + +#[test] +fn first_invalid_number_in_placeholder_order_is_reported() { + let values = HashMap::from([(":b".to_owned(), n("two")), (":a".to_owned(), n("one"))]); + assert_eq!( + message(validate_expression_attribute_value_numbers(&values)).as_deref(), + Some("The parameter cannot be converted to a numeric value: one") + ); +} + +#[test] +fn request_time_checks_run_limit_then_mix_then_numbers() { + let bad_number = serde_json::json!({ + "TableName": "t", + "KeyConditionExpression": "pk = :u", + "ExpressionAttributeValues": {":u": {"N": "bob"}}, + }); + + let mut with_limit = bad_number.clone(); + with_limit["Limit"] = serde_json::json!(0); + assert_eq!( + message(validate_query_request_values(&query(with_limit))).as_deref(), + Some(LIMIT_BELOW_ONE) + ); + + let mut with_mix = bad_number.clone(); + with_mix["KeyConditions"] = serde_json::json!({ + "pk": {"ComparisonOperator": "EQ", "AttributeValueList": [{"S": "x"}]} + }); + assert_eq!( + message(validate_query_request_values(&query(with_mix))).as_deref(), + Some( + "Can not use both expression and non-expression parameters in the same request: \ + Non-expression parameters: {KeyConditions} Expression parameters: {KeyConditionExpression}" + ) + ); + + assert_eq!( + message(validate_query_request_values(&query(bad_number))).as_deref(), + Some("The parameter cannot be converted to a numeric value: bob") + ); + + // A number only a FilterExpression reads, and one nothing reads. + for body in [ + serde_json::json!({ + "TableName": "t", + "KeyConditionExpression": "pk = :p", + "FilterExpression": "a = :n", + "ExpressionAttributeValues": {":p": {"S": "x"}, ":n": {"N": "bob"}}, + }), + serde_json::json!({ + "TableName": "t", + "KeyConditionExpression": "pk = :p", + "ExpressionAttributeValues": {":p": {"S": "x"}, ":n": {"N": "bob"}}, + }), + ] { + assert_eq!( + message(validate_query_request_values(&query(body))).as_deref(), + Some("The parameter cannot be converted to a numeric value: bob") + ); + } + + // Legacy KeyConditions alone does not join the mix check; a well-formed + // request passes. + let legacy = serde_json::json!({ + "TableName": "t", + "KeyConditions": { + "pk": {"ComparisonOperator": "EQ", "AttributeValueList": [{"S": "x"}]} + }, + "FilterExpression": "a = :n", + "ExpressionAttributeValues": {":n": {"N": "5"}}, + "Limit": 1, + }); + assert!(validate_query_request_values(&query(legacy)).is_ok()); +} diff --git a/crates/server/src/request_helpers.rs b/crates/server/src/request_helpers.rs index e7d12abf9..9f34b2632 100755 --- a/crates/server/src/request_helpers.rs +++ b/crates/server/src/request_helpers.rs @@ -184,6 +184,20 @@ pub(crate) async fn authorize_request( authz_resource::validate_tag_resource_arn_shape(arn)?; } + // A Query's request-time checks (Limit, the expression and non-expression + // parameter mix, every number in ExpressionAttributeValues) run before the + // IAM decision, for every principal. The engine makes the same checks at + // request time; they run here as well because authorization binds no + // dynamodb:LeadingKeys value to a number it cannot parse, and a fence that + // needs the key present (ForAnyValue, a Null guard, the plain operators) + // would otherwise answer AccessDeniedException where ValidationException is + // the request's error. A body the engine cannot deserialize is left to it. + if operation == "Query" + && let Ok(query) = serde_json::from_value::(input.clone()) + { + extenddb_engine::query_key_values::validate_query_request_values(&query)?; + } + // Every other ARN-bearing field (StreamArn, BackupArn, the export TableArn, // a restore SourceTableArn) must parse to the resource kind the operation // takes, or the request is refused with ValidationException before the IAM diff --git a/tests/test_query_key_value_types.py b/tests/test_query_key_value_types.py new file mode 100644 index 000000000..02ee3b317 --- /dev/null +++ b/tests/test_query_key_value_types.py @@ -0,0 +1,489 @@ +# Copyright 2026 ExtendDB contributors +# SPDX-License-Identifier: Apache-2.0 + +"""Query key condition values checked against the key schema. + +A ``KeyConditionExpression`` value is refused before any row is read when an +``N`` value is not a number (``The parameter cannot be converted to a numeric +value: ``) or when a key condition value's type is not the key +attribute's ``AttributeDefinitions`` type (``One or more parameter values were +invalid: Condition parameter type does not match schema type``). Every expected +answer below was measured against the service on 2026-10-24 (15:10 UTC and +21:00 UTC) on tables with S/S, S/N and B/B keys, a GSI and an LSI. The rows +also pin where the two checks fall against the other Query checks: the numeric +check after Limit and the expression parameter mix check and before the table +lookup; the type check after the key schema, filter, projection, unused value +and Select checks and before the ExclusiveStartKey check. + +Dual-target: runs against the service and against extenddb with the same +assertions. The LeadingKeys class needs the extenddb management API and is +skipped against the service. + +Run: + EXTENDDB_TEST_ENDPOINT=https://localhost:18443 \\ + EXTENDDB_ADMIN_USER=admin EXTENDDB_ADMIN_PASSWORD= \\ + pytest tests/test_query_key_value_types.py -v +""" + +from __future__ import annotations + +import os +import uuid +from typing import Any + +import boto3 +import pytest +from botocore.config import Config as BotoConfig +from botocore.exceptions import ClientError + +from conftest import scoped_table, wait_for_active + +NUMERIC = "The parameter cannot be converted to a numeric value: " +TYPE_MISMATCH = ( + "One or more parameter values were invalid: " + "Condition parameter type does not match schema type" +) + +S = {"AttributeType": "S"} + + +def _err(exc_info) -> tuple[str, str]: + err = exc_info.value.response["Error"] + return err["Code"], err["Message"] + + +@pytest.fixture(scope="module") +def ss_table(dynamodb_client): + """pk S, sk S; LSI lsi1 on lsk S; GSI gsi1 on gpk S (hash only).""" + with scoped_table( + dynamodb_client, + attribute_definitions=[ + {"AttributeName": "pk", **S}, + {"AttributeName": "sk", **S}, + {"AttributeName": "lsk", **S}, + {"AttributeName": "gpk", **S}, + ], + key_schema=[ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "sk", "KeyType": "RANGE"}, + ], + LocalSecondaryIndexes=[{ + "IndexName": "lsi1", + "KeySchema": [ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "lsk", "KeyType": "RANGE"}, + ], + "Projection": {"ProjectionType": "ALL"}, + }], + GlobalSecondaryIndexes=[{ + "IndexName": "gsi1", + "KeySchema": [{"AttributeName": "gpk", "KeyType": "HASH"}], + "Projection": {"ProjectionType": "ALL"}, + }], + ) as name: + for pk in ("alice", "bob"): + dynamodb_client.put_item(TableName=name, Item={ + "pk": {"S": pk}, "sk": {"S": "1"}, "lsk": {"S": "l"}, "gpk": {"S": pk}}) + yield name + + +@pytest.fixture(scope="module") +def ns_table(dynamodb_client): + """pk S, sk N; GSI gsi1 on gpk S / gsk S.""" + with scoped_table( + dynamodb_client, + attribute_definitions=[ + {"AttributeName": "pk", **S}, + {"AttributeName": "sk", "AttributeType": "N"}, + {"AttributeName": "gpk", **S}, + {"AttributeName": "gsk", **S}, + ], + key_schema=[ + {"AttributeName": "pk", "KeyType": "HASH"}, + {"AttributeName": "sk", "KeyType": "RANGE"}, + ], + GlobalSecondaryIndexes=[{ + "IndexName": "gsi1", + "KeySchema": [ + {"AttributeName": "gpk", "KeyType": "HASH"}, + {"AttributeName": "gsk", "KeyType": "RANGE"}, + ], + "Projection": {"ProjectionType": "ALL"}, + }], + ) as name: + for pk in ("alice", "bob"): + dynamodb_client.put_item(TableName=name, Item={ + "pk": {"S": pk}, "sk": {"N": "5"}, "gpk": {"S": pk}, "gsk": {"S": "g"}}) + yield name + + +@pytest.fixture(scope="module") +def bb_table(dynamodb_client): + """bpk B, bsk B.""" + with scoped_table( + dynamodb_client, + attribute_definitions=[ + {"AttributeName": "bpk", "AttributeType": "B"}, + {"AttributeName": "bsk", "AttributeType": "B"}, + ], + key_schema=[ + {"AttributeName": "bpk", "KeyType": "HASH"}, + {"AttributeName": "bsk", "KeyType": "RANGE"}, + ], + ) as name: + dynamodb_client.put_item(TableName=name, Item={"bpk": {"B": b"bob"}, "bsk": {"B": b"x"}}) + yield name + + +NB = {"N": "bob"} +N5 = {"N": "5"} +SX = {"S": "x"} +B_BOB = {"B": b"bob"} +MISSING = "no-such-table-kv-types" +LEGACY_PK = {"pk": {"ComparisonOperator": "EQ", "AttributeValueList": [SX]}} +L_NB = {"L": [{"S": "a"}, NB]} +M_NB = {"M": {"a": NB}} +NS_NB = {"NS": ["1", "bob"]} + +LIMIT_MSG = ( + "1 validation error detected: Value at 'Limit' failed to satisfy constraint: " + "Member must have value greater than or equal to 1" +) +MIX_MSG = ( + "Can not use both expression and non-expression parameters in the same request: " + "Non-expression parameters: {KeyConditions} Expression parameters: {KeyConditionExpression}" +) +OVERFLOW_MSG = ( + "Number overflow. Attempting to store a number with magnitude larger than supported range" +) +DIGITS_MSG = "Attempting to store more than 38 significant digits in a Number" +MISSED_PK = "Query condition missed key schema element: pk" +MISSED_GPK = "Query condition missed key schema element: gpk" +RESERVED_KCE = ( + "Invalid KeyConditionExpression: Attribute name is a reserved keyword; reserved keyword: other" +) +NO_INDEX = "The table does not have the specified index: nope" +UNUSED_X = "Value provided in ExpressionAttributeValues unused in expressions: keys: {:x}" +GSI_CONSISTENT = "Consistent reads are not supported on global secondary indexes" +FILTER_BEGINS_WITH_N = ( + "Invalid FilterExpression: Incorrect operand type for operator or function; " + "operator or function: begins_with, operand type: N" +) +PROJECTION_OVERLAP = ( + "Invalid ProjectionExpression: Two document paths overlap with each other; " + "must remove or rewrite one of these paths; path one: [a], path two: [a, b]" +) +FILTER_RESERVED = ( + "Invalid FilterExpression: Attribute name is a reserved keyword; reserved keyword: name" +) +SELECT_PROJECTION = ( + "1 validation error detected: Cannot specify the ProjectionExpression when choosing to " + "get ALL_ATTRIBUTES" +) +V = "ValidationException" +RNF = "ResourceNotFoundException" + + +def kce(expr, values, **extra): + return {"KeyConditionExpression": expr, "ExpressionAttributeValues": values, **extra} + + +# (row, table, Query arguments, expected error code, expected message). +# Tables: ss (pk S, sk S, LSI lsi1 on lsk S, GSI gsi1 on gpk S), ns (pk S, sk N, +# GSI gsi1 on gpk S / gsk S), bb (bpk B, bsk B), missing (no such table). +ROWS = [ + # The numeric check against the other Query checks. + ("a1 missing table", "missing", kce("pk = :u", {":u": NB}), V, NUMERIC + "bob"), + ("a2 unknown IndexName", "ss", kce("pk = :u", {":u": NB}, IndexName="nope"), V, NUMERIC + "bob"), + ("a3 undefined placeholder", "ss", kce("pk = :u AND sk = :zz", {":u": NB}), V, NUMERIC + "bob"), + ("a4 unused value", "ss", kce("pk = :u", {":u": NB, ":x": SX}), V, NUMERIC + "bob"), + ("a5 syntax error", "ss", kce("pk = :u AND", {":u": NB}), V, NUMERIC + "bob"), + ("a6 Limit 0", "ss", kce("pk = :u", {":u": NB}, Limit=0), V, LIMIT_MSG), + ("a7 N bob only in FilterExpression", "ss", + kce("pk = :p", {":p": SX, ":n": NB}, FilterExpression="a = :n"), V, NUMERIC + "bob"), + ("a8 N bob unused", "ss", kce("pk = :p", {":p": SX, ":n": NB}), V, NUMERIC + "bob"), + ("a9 N bob in filter beside pk = N 5", "ss", + kce("pk = :p", {":p": N5, ":n": NB}, FilterExpression="a = :n"), V, NUMERIC + "bob"), + ("a10 ConsistentRead on GSI", "ss", + kce("gpk = :u", {":u": NB}, IndexName="gsi1", ConsistentRead=True), V, NUMERIC + "bob"), + ("a11 N empty", "ss", kce("pk = :u", {":u": {"N": ""}}), V, NUMERIC), + ("a12 N 1e", "ss", kce("pk = :u", {":u": {"N": "1e"}}), V, NUMERIC + "1e"), + ("a13 N 12abc", "ss", kce("pk = :u", {":u": {"N": "12abc"}}), V, NUMERIC + "12abc"), + ("a14 sk only", "ss", kce("sk = :u", {":u": NB}), V, NUMERIC + "bob"), + ("a15 KeyConditions and KeyConditionExpression", "ss", + kce("pk = :u", {":u": NB}, KeyConditions=LEGACY_PK), V, MIX_MSG), + ("a16 BETWEEN bounds reversed", "ss", + kce("pk = :u AND sk BETWEEN :a AND :b", {":u": NB, ":a": {"S": "b"}, ":b": {"S": "a"}}), + V, NUMERIC + "bob"), + ("a17 nested path", "ss", kce("pk.x = :u", {":u": NB}), V, NUMERIC + "bob"), + ("a18 pk >", "ss", kce("pk > :u", {":u": NB}), V, NUMERIC + "bob"), + ("a19 ExclusiveStartKey bad", "ss", + kce("pk = :u", {":u": NB}, ExclusiveStartKey={"zz": SX}), V, NUMERIC + "bob"), + ("a20 pk = B AND sk = N bob", "ss", + kce("pk = :u AND sk = :s", {":u": B_BOB, ":s": NB}), V, NUMERIC + "bob"), + ("a21 pk = S AND sk = N bob", "ss", + kce("pk = :u AND sk = :s", {":u": SX, ":s": NB}), V, NUMERIC + "bob"), + ("a22 N 1e200", "ss", kce("pk = :u", {":u": {"N": "1e200"}}), V, OVERFLOW_MSG), + ("a23 N 39 digits", "ss", kce("pk = :u", {":u": {"N": "1" * 39}}), V, DIGITS_MSG), + ("a24 Select ALL_ATTRIBUTES with ProjectionExpression", "ss", + kce("pk = :u", {":u": NB}, Select="ALL_ATTRIBUTES", ProjectionExpression="a"), + V, NUMERIC + "bob"), + ("a25 reserved word in filter", "ss", + kce("pk = :u", {":u": NB, ":n": SX}, FilterExpression="name = :n"), V, NUMERIC + "bob"), + ("a26 undefined name in filter", "ss", + kce("pk = :u", {":u": NB, ":n": SX}, FilterExpression="#n = :n"), V, NUMERIC + "bob"), + ("a27 two sort key conditions", "ss", + kce("pk = :u AND sk = :s AND sk > :t", {":u": NB, ":s": N5, ":t": {"N": "6"}}), + V, NUMERIC + "bob"), + ("a28 sk = N bob with unused value", "ss", + kce("pk = :u AND sk = :s", {":u": SX, ":s": NB, ":x": SX}), V, NUMERIC + "bob"), + ("a29 ProjectionExpression overlap", "ss", + kce("pk = :u", {":u": NB}, ProjectionExpression="a, a.b"), V, NUMERIC + "bob"), + ("a30 N space 5", "ss", kce("pk = :u", {":u": {"N": " 5"}}), V, NUMERIC + " 5"), + # The type check against the other key condition checks. + ("b1 sk = N 5 only", "ss", kce("sk = :s", {":s": N5}), V, MISSED_PK), + ("b2 reserved word in key condition", "ss", + kce("pk = :u AND other = :s", {":u": SX, ":s": N5}), V, RESERVED_KCE), + ("b4 BETWEEN N bounds on S sk", "ss", + kce("pk = :u AND sk BETWEEN :a AND :b", {":u": SX, ":a": {"N": "1"}, ":b": {"N": "2"}}), + V, TYPE_MISMATCH), + ("b6 sk > N 5", "ss", kce("pk = :u AND sk > :s", {":u": SX, ":s": N5}), V, TYPE_MISMATCH), + ("b7 sk < B", "ss", kce("pk = :u AND sk < :s", {":u": SX, ":s": B_BOB}), V, TYPE_MISMATCH), + ("b9 unknown IndexName", "ss", kce("pk = :u", {":u": N5}, IndexName="nope"), V, NO_INDEX), + ("b10 Limit 0", "ss", kce("pk = :u", {":u": N5}, Limit=0), V, LIMIT_MSG), + ("b11 unused value", "ss", kce("pk = :u", {":u": N5, ":x": SX}), V, UNUSED_X), + ("b13 missing table", "missing", kce("pk = :u", {":u": N5}), RNF, "Requested resource not found"), + ("b14 GSI gpk = N 5", "ss", kce("gpk = :g", {":g": N5}, IndexName="gsi1"), V, TYPE_MISMATCH), + ("b15 LSI lsk = N 5", "ss", + kce("pk = :u AND lsk = :l", {":u": SX, ":l": N5}, IndexName="lsi1"), V, TYPE_MISMATCH), + ("b16 GSI with base pk = N 5", "ss", kce("pk = :u", {":u": N5}, IndexName="gsi1"), V, MISSED_GPK), + ("b17 GSI with base pk = S", "ss", kce("pk = :u", {":u": SX}, IndexName="gsi1"), V, MISSED_GPK), + ("b18 N sk = S 5", "ns", kce("pk = :u AND sk = :s", {":u": SX, ":s": {"S": "5"}}), V, TYPE_MISMATCH), + ("b19 N sk BETWEEN S", "ns", + kce("pk = :u AND sk BETWEEN :a AND :b", {":u": SX, ":a": {"S": "1"}, ":b": {"S": "2"}}), + V, TYPE_MISMATCH), + ("b20 N sk begins_with S", "ns", + kce("pk = :u AND begins_with(sk, :b)", {":u": SX, ":b": {"S": "1"}}), V, TYPE_MISMATCH), + ("b22 pk = N 5 AND N sk = S 5", "ns", + kce("pk = :u AND sk = :s", {":u": N5, ":s": {"S": "5"}}), V, TYPE_MISMATCH), + ("b23 N sk = N bob", "ns", kce("pk = :u AND sk = :s", {":u": SX, ":s": NB}), V, NUMERIC + "bob"), + ("b24 B pk = S", "bb", kce("bpk = :u", {":u": SX}), V, TYPE_MISMATCH), + ("b25 B pk = N 5", "bb", kce("bpk = :u", {":u": N5}), V, TYPE_MISMATCH), + ("b26 B pk = N bob", "bb", kce("bpk = :u", {":u": NB}), V, NUMERIC + "bob"), + ("b27 B sk begins_with S", "bb", + kce("bpk = :u AND begins_with(bsk, :b)", {":u": B_BOB, ":b": SX}), V, TYPE_MISMATCH), + ("b28 ConsistentRead on GSI", "ss", + kce("gpk = :g", {":g": N5}, IndexName="gsi1", ConsistentRead=True), V, GSI_CONSISTENT), + ("b29 filter begins_with N", "ss", + kce("pk = :u", {":u": N5, ":n": {"N": "1"}}, FilterExpression="begins_with(a, :n)"), + V, FILTER_BEGINS_WITH_N), + ("b30 ProjectionExpression overlap", "ss", + kce("pk = :u", {":u": N5}, ProjectionExpression="a, a.b"), V, PROJECTION_OVERLAP), + ("b31 ExclusiveStartKey bad", "ss", + kce("pk = :u", {":u": N5}, ExclusiveStartKey={"zz": SX}), V, TYPE_MISMATCH), + ("b32 pk = N 5 AND sk = N bob", "ss", + kce("pk = :u AND sk = :s", {":u": N5, ":s": NB}), V, NUMERIC + "bob"), + ("b33 pk = B AND sk = N 5", "ss", + kce("pk = :u AND sk = :s", {":u": B_BOB, ":s": N5}), V, TYPE_MISMATCH), + ("b35 pk = NULL", "ss", kce("pk = :u", {":u": {"NULL": True}}), V, TYPE_MISMATCH), + ("b36 pk = SS", "ss", kce("pk = :u", {":u": {"SS": ["a"]}}), V, TYPE_MISMATCH), + ("b37 pk = L", "ss", kce("pk = :u", {":u": {"L": [SX]}}), V, TYPE_MISMATCH), + ("b38 pk = M", "ss", kce("pk = :u", {":u": {"M": {"a": SX}}}), V, TYPE_MISMATCH), + ("b41 reserved word in filter", "ss", + kce("pk = :u", {":u": N5, ":n": SX}, FilterExpression="name = :n"), V, FILTER_RESERVED), + ("b42 Select ALL_ATTRIBUTES with ProjectionExpression", "ss", + kce("pk = :u", {":u": N5}, Select="ALL_ATTRIBUTES", ProjectionExpression="a"), + V, SELECT_PROJECTION), + ("b43 non-key second clause", "ss", + kce("pk = :u AND other = :s", {":u": N5, ":s": SX}), V, RESERVED_KCE), + ("b45 pk = N 5 through #p", "ss", + kce("#p = :u", {":u": N5}, ExpressionAttributeNames={"#p": "pk"}), V, TYPE_MISMATCH), + ("b46 reversed operands", "ss", + kce("pk = :u AND :s <= sk", {":u": SX, ":s": N5}), V, TYPE_MISMATCH), + ("b47 GSI gsk = N 5", "ns", + kce("gpk = :g AND gsk = :s", {":g": SX, ":s": N5}, IndexName="gsi1"), V, TYPE_MISMATCH), + # Nested N values, values no expression reads, and the Limit and mix checks + # against the table and index lookups (measured 2026-10-24 21:06 UTC). + ("c1 L with N bob in filter", "ss", + kce("pk = :p", {":p": SX, ":l": L_NB}, FilterExpression="a = :l"), V, NUMERIC + "bob"), + ("c2 L with N bob unused", "ss", kce("pk = :p", {":p": SX, ":l": L_NB}), V, NUMERIC + "bob"), + ("c3 M with N bob in filter", "ss", + kce("pk = :p", {":p": SX, ":m": M_NB}, FilterExpression="a = :m"), V, NUMERIC + "bob"), + ("c4 M with N bob unused", "ss", kce("pk = :p", {":p": SX, ":m": M_NB}), V, NUMERIC + "bob"), + ("c5 NS with bob in filter", "ss", + kce("pk = :p", {":p": SX, ":n": NS_NB}, FilterExpression="a = :n"), V, NUMERIC + "bob"), + ("c6 NS with bob unused", "ss", kce("pk = :p", {":p": SX, ":n": NS_NB}), V, NUMERIC + "bob"), + ("c7 L with N empty in filter", "ss", + kce("pk = :p", {":p": SX, ":l": {"L": [{"N": ""}]}}, FilterExpression="a = :l"), V, NUMERIC), + ("c8 M in L in M with N bob", "ss", + kce("pk = :p", {":p": SX, ":m": {"M": {"a": {"L": [M_NB]}}}}), V, NUMERIC + "bob"), + ("c9 N bob unused on a missing table", "missing", + kce("pk = :p", {":p": SX, ":n": NB}), V, NUMERIC + "bob"), + ("c12 N bob with no key condition", "ss", + {"ExpressionAttributeValues": {":n": NB}}, V, NUMERIC + "bob"), + ("c13 N bob beside legacy KeyConditions", "ss", + {"KeyConditions": LEGACY_PK, "ExpressionAttributeValues": {":n": NB}}, V, NUMERIC + "bob"), + ("d1 Limit 0 on a missing table", "missing", kce("pk = :p", {":p": SX}, Limit=0), V, LIMIT_MSG), + ("d2 Limit 0 with unknown IndexName", "ss", + kce("pk = :p", {":p": SX}, IndexName="nope", Limit=0), V, LIMIT_MSG), + ("d3 Limit 0 with ConsistentRead on GSI", "ss", + kce("gpk = :p", {":p": SX}, IndexName="gsi1", ConsistentRead=True, Limit=0), V, LIMIT_MSG), + ("d4 KeyConditions and KeyConditionExpression on a missing table", "missing", + kce("pk = :p", {":p": SX}, KeyConditions=LEGACY_PK), V, MIX_MSG), + ("d5 Limit 0 with KeyConditions and KeyConditionExpression", "ss", + kce("pk = :p", {":p": SX}, Limit=0, KeyConditions=LEGACY_PK), V, LIMIT_MSG), + ("d6 Limit 0 with N bob and an undefined placeholder", "ss", + kce("pk = :p AND sk = :zz", {":p": SX, ":n": NB}, Limit=0), V, LIMIT_MSG), + # The other value types the earlier record covers. + ("pk = BOOL", "ss", kce("pk = :u", {":u": {"BOOL": True}}), V, TYPE_MISMATCH), + ("pk = N bob AND pk > S alice", "ss", + kce("pk = :u AND pk > :v", {":u": NB, ":v": {"S": "alice"}}), V, NUMERIC + "bob"), +] + + +class TestKeyConditionValueTypes: + """Each row: a Query and the service's answer.""" + + @pytest.fixture + def tables(self, ss_table, ns_table, bb_table): + return {"ss": ss_table, "ns": ns_table, "bb": bb_table, "missing": MISSING} + + @pytest.mark.parametrize("row,table,kwargs,code,expected", ROWS, ids=[r[0] for r in ROWS]) + def test_rows(self, dynamodb_client_no_validation, tables, row, table, kwargs, code, expected): + with pytest.raises(ClientError) as exc_info: + dynamodb_client_no_validation.query(TableName=tables[table], **kwargs) + assert _err(exc_info) == (code, expected) + + def test_matching_types_read_the_named_partition(self, dynamodb_client, ss_table): + resp = dynamodb_client.query( + TableName=ss_table, KeyConditionExpression="pk = :u", + ExpressionAttributeValues={":u": {"S": "bob"}}) + assert [it["pk"]["S"] for it in resp["Items"]] == ["bob"] + + def test_control_matching_types_on_every_table(self, dynamodb_client, ss_table, ns_table, bb_table): + assert dynamodb_client.query( + TableName=ss_table, KeyConditionExpression="pk = :u AND sk = :s", + ExpressionAttributeValues={":u": {"S": "nope"}, ":s": SX})["Count"] == 0 + assert dynamodb_client.query( + TableName=ns_table, KeyConditionExpression="pk = :u AND sk > :s", + ExpressionAttributeValues={":u": {"S": "bob"}, ":s": {"N": "1"}})["Count"] == 1 + assert dynamodb_client.query( + TableName=bb_table, KeyConditionExpression="bpk = :u", + ExpressionAttributeValues={":u": B_BOB})["Count"] == 1 + + +class TestLeadingKeysWithAnInvalidKeyValue: + """A principal under each LeadingKeys fence form gets ValidationException and + no items for a Query whose ExpressionAttributeValues carries an N value that + is not a number, whether the key condition, the filter or nothing reads it. + + extenddb only: the fences need users created through the management API.""" + + @pytest.fixture(autouse=True) + def setup(self): + endpoint = os.environ.get("EXTENDDB_TEST_ENDPOINT", "").strip() + admin_user = os.environ.get("EXTENDDB_ADMIN_USER", "").strip() + admin_pass = os.environ.get("EXTENDDB_ADMIN_PASSWORD", "").strip() + if not endpoint: + pytest.skip("LeadingKeys fences need the extenddb management API") + if not admin_user or not admin_pass: + pytest.fail("MISCONFIGURED: EXTENDDB_ADMIN_USER and EXTENDDB_ADMIN_PASSWORD are required") + from management_helpers import ManagementClient + + self.endpoint = endpoint + self.region = os.environ.get("AWS_DEFAULT_REGION", "us-east-1") + self.mgmt = ManagementClient(endpoint, admin_user, admin_pass) + self.account_id = f"{uuid.uuid4().int % 10**12:012d}" + resp = self.mgmt.create_account(self.account_id, f"kv-test-{self.account_id}") + assert resp.status_code == 201, resp.text + self._users: list[str] = [] + yield + for u in self._users: + try: + self.mgmt.delete_user(self.account_id, u) + except Exception: + pass + self.mgmt.delete_account(self.account_id) + + def _client(self, policy: dict | None) -> Any: + name = f"kv-{uuid.uuid4().hex[:8]}" + resp = self.mgmt.create_user(self.account_id, name, "TestPass123!") + assert resp.status_code == 201, resp.text + self._users.append(name) + resp = self.mgmt.create_access_key(self.account_id, name) + assert resp.status_code == 201, resp.text + creds = resp.json() + if policy is not None: + resp = self.mgmt.put_user_policy(self.account_id, name, "p", policy) + assert resp.status_code == 204, resp.text + return boto3.client( + "dynamodb", endpoint_url=self.endpoint, region_name=self.region, + aws_access_key_id=creds["access_key_id"], + aws_secret_access_key=creds["secret_access_key"], + verify=False, + config=BotoConfig(retries={"max_attempts": 0}, parameter_validation=False)) + + def _fence(self, condition: dict, table: str) -> dict: + arn = f"arn:aws:dynamodb:{self.region}:{self.account_id}:table/{table}" + return { + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": "dynamodb:*", + "Resource": [arn, arn + "/index/*"], + "Condition": condition, + }], + } + + def test_every_fence_form_gets_validation_exception_and_no_items(self): + owner = self._client({ + "Version": "2012-10-17", + "Statement": [{"Effect": "Allow", "Action": "dynamodb:*", "Resource": "*"}], + }) + table = f"kv-{uuid.uuid4().hex[:8]}" + owner.create_table( + TableName=table, + AttributeDefinitions=[{"AttributeName": "pk", **S}], + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + BillingMode="PAY_PER_REQUEST") + wait_for_active(owner, table) + for pk in ("alice", "bob"): + owner.put_item(TableName=table, Item={"pk": {"S": pk}}) + + fences = { + "ForAllValues": {"ForAllValues:StringEquals": {"dynamodb:LeadingKeys": ["alice"]}}, + "ForAnyValue": {"ForAnyValue:StringEquals": {"dynamodb:LeadingKeys": ["alice"]}}, + "Null false": {"Null": {"dynamodb:LeadingKeys": "false"}, + "ForAllValues:StringEquals": {"dynamodb:LeadingKeys": ["alice"]}}, + "StringEquals": {"StringEquals": {"dynamodb:LeadingKeys": "alice"}}, + "StringLike": {"StringLike": {"dynamodb:LeadingKeys": "ali*"}}, + "StringNotEquals": {"StringNotEquals": {"dynamodb:LeadingKeys": "bob"}}, + } + # The reported shape, and an N value that only the filter reads or + # nothing reads beside a key value the fence would bind. + requests = { + "pk = N bob": {"TableName": table, "KeyConditionExpression": "pk = :u", + "ExpressionAttributeValues": {":u": NB}}, + "pk = S alice, N bob in filter": { + "TableName": table, "KeyConditionExpression": "pk = :u", + "FilterExpression": "a = :n", + "ExpressionAttributeValues": {":u": {"S": "alice"}, ":n": NB}}, + "pk = S bob, N bob unused": { + "TableName": table, "KeyConditionExpression": "pk = :u", + "ExpressionAttributeValues": {":u": {"S": "bob"}, ":n": NB}}, + } + failures = [] + clients = {label: self._client(self._fence(condition, table)) + for label, condition in fences.items()} + clients["owner"] = owner + for shape, request in requests.items(): + for label, client in clients.items(): + try: + resp = client.query(**request) + failures.append(f"{shape} / {label}: {resp['Count']} items returned") + except ClientError as exc: + err = exc.response["Error"] + if (err["Code"], err["Message"]) != ("ValidationException", NUMERIC + "bob"): + failures.append(f"{shape} / {label}: {err['Code']} {err['Message']}") + owner.delete_table(TableName=table) + assert not failures, "\n".join(failures) From f0017ce90ef654ac7957ca1e8f62a8b56b61491d Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sun, 25 Oct 2026 00:35:19 +0000 Subject: [PATCH 2/7] fix(query): run the request-time checks on the raw body, enums in the service's order The server ran its pre-IAM checks only when the body deserialized as QueryInput, and the engine checked the ReturnConsumedCapacity and Select enums before them, so a Query carrying an invalid Select and an N value that is not a number answered the Select message, or AccessDeniedException for a principal whose LeadingKeys fence needs a value. The checks now read the raw body in the service's order, measured 2026-10-24: Limit, the expression and non-expression parameter mix, ReturnConsumedCapacity, every number in ExpressionAttributeValues, then Select. The engine and the server run the same function. --- crates/engine/src/query.rs | 41 ++------- crates/engine/src/query_key_values.rs | 95 +++++++++++++++------ crates/engine/src/query_key_values/tests.rs | 66 +++++++++++--- crates/server/src/request_helpers.rs | 22 ++--- tests/test_query_key_value_types.py | 35 ++++++-- 5 files changed, 173 insertions(+), 86 deletions(-) diff --git a/crates/engine/src/query.rs b/crates/engine/src/query.rs index 97e07ead2..a4e633073 100755 --- a/crates/engine/src/query.rs +++ b/crates/engine/src/query.rs @@ -43,42 +43,15 @@ pub async fn handle_query( body: Value, ctx: &OperationContext, ) -> Result { - // Pre-scanned before typed deserialization, sequentially: Query stops at - // the FIRST invalid enum, checking ReturnConsumedCapacity ahead of Select. - // Measured 2026-08-24 (us-east-1): both invalid answers "1 validation - // error detected" naming returnConsumedCapacity alone. (Scan is the - // opposite on both counts: it aggregates, select-first; see handle_scan.) - crate::validate_enum_fields( - &body, - &[crate::EnumField { - json_name: "ReturnConsumedCapacity", - valid: &["INDEXES", "TOTAL", "NONE"], - clause: crate::EnumClause::Named("returnConsumedCapacity"), - }], - )?; - crate::validate_enum_fields( - &body, - &[crate::EnumField { - json_name: "Select", - valid: &[ - "SPECIFIC_ATTRIBUTES", - "COUNT", - "ALL_ATTRIBUTES", - "ALL_PROJECTED_ATTRIBUTES", - ], - clause: crate::EnumClause::Named("select"), - }], - )?; + // Request-time checks on the raw body, in the service's order: Limit, the + // expression and non-expression parameter mix, the ReturnConsumedCapacity + // enum, every number in ExpressionAttributeValues, the Select enum. They + // run before typed deserialization so a field that does not deserialize + // does not stop the ones before it. The server runs the same function + // ahead of the IAM decision; see `query_key_values`. + crate::query_key_values::validate_query_request_body(&body)?; let input: QueryInput = serde_json::from_value(body).map_err(crate::deserialize_error)?; - // Request-time checks in the service's order: Limit, the expression and - // non-expression parameter mix, then every number in - // ExpressionAttributeValues (measured 2026-10-24: a Limit of 0 and a - // parameter mix are reported ahead of an N value that is not a number, and - // that value is reported ahead of a missing table or index). The server - // runs the same function ahead of the IAM decision; see `query_key_values`. - crate::query_key_values::validate_query_request_values(&input)?; - // P118: Fetch key_info first so we can use table_id for index lookup. let key_info = ctx .table_key_info(&input.table_name) diff --git a/crates/engine/src/query_key_values.rs b/crates/engine/src/query_key_values.rs index e49968648..7bfb356ba 100644 --- a/crates/engine/src/query_key_values.rs +++ b/crates/engine/src/query_key_values.rs @@ -12,7 +12,7 @@ //! checked, whether the key condition, the filter or nothing reads it. The //! check runs at request time, after the `Limit` check and the expression //! and non-expression parameter mix check, and before the table is looked -//! up; see [`validate_query_request_values`]; +//! up; see [`validate_query_request_body`]; //! 2. a key condition value whose type is not the key attribute's //! `AttributeDefinitions` type answers `One or more parameter values were //! invalid: Condition parameter type does not match schema type`, on the @@ -33,7 +33,8 @@ use extenddb_core::error::DynamoDbError; use extenddb_core::expression::{ Expr, ExpressionMaps, KeyCondition, PathElement, SortKeyCondition, }; -use extenddb_core::types::{AttributeDefinition, AttributeValue, QueryInput, ScalarAttributeType}; +use extenddb_core::types::{AttributeDefinition, AttributeValue, ScalarAttributeType}; +use serde_json::Value; /// The service's text for a key condition value of the wrong type. pub const KEY_CONDITION_TYPE_MISMATCH: &str = "One or more parameter values were invalid: Condition parameter type does not match schema type"; @@ -44,10 +45,18 @@ pub const LIMIT_BELOW_ONE: &str = "1 validation error detected: Value at 'Limit' /// The bare numeric message the number validator uses for an empty `N`. const EMPTY_NUMBER: &str = "The parameter cannot be converted to a numeric value"; -/// The `Query` checks that run on the request alone, before the table is -/// looked up, in the service's order: `Limit`, the expression and -/// non-expression parameter mix, then every number in -/// `ExpressionAttributeValues`. +/// The `Query` checks that run on the request body alone, before the body is +/// deserialized and before the table is looked up, in the service's order: +/// `Limit`, the expression and non-expression parameter mix, the +/// `ReturnConsumedCapacity` enum, every number in `ExpressionAttributeValues`, +/// then the `Select` enum (measured 2026-10-24: a `Limit` of 0 and a parameter +/// mix are reported ahead of an invalid enum value of either field, an invalid +/// `ReturnConsumedCapacity` ahead of an `N` value that is not a number, and +/// that value ahead of an invalid `Select`). +/// +/// The checks read the raw body, so a field that does not deserialize (an +/// invalid `Select`, for one) does not stop the ones before it. A field of the +/// wrong JSON shape is left to the deserializer, which runs next. /// /// The server runs the same function ahead of the IAM decision, so a principal /// whose `dynamodb:LeadingKeys` fence cannot bind a number that does not parse @@ -57,8 +66,8 @@ const EMPTY_NUMBER: &str = "The parameter cannot be converted to a numeric value /// # Errors /// /// Returns `DynamoDbError::ValidationException` with the service's text. -pub fn validate_query_request_values(input: &QueryInput) -> Result<(), DynamoDbError> { - if let Some(limit) = input.limit +pub fn validate_query_request_body(body: &Value) -> Result<(), DynamoDbError> { + if let Some(limit) = body.get("Limit").and_then(Value::as_i64) && limit < 1 { return Err(DynamoDbError::ValidationException( @@ -66,36 +75,74 @@ pub fn validate_query_request_values(input: &QueryInput) -> Result<(), DynamoDbE )); } - let kce_present = input.key_condition_expression.is_some(); - let has_kc = input.key_conditions.as_ref().is_some_and(|m| !m.is_empty()); - let has_qf = input.query_filter.as_ref().is_some_and(|m| !m.is_empty()); - let has_atg = input - .attributes_to_get - .as_ref() - .is_some_and(|a| !a.is_empty()); + let kce_present = present(body, "KeyConditionExpression"); // KeyConditions combines with FilterExpression and ProjectionExpression, so // it joins the mix check only when KeyConditionExpression is also present. extenddb_core::validation::validate_no_expression_param_mixing( &[ - ("AttributesToGet", has_atg), - ("QueryFilter", has_qf), - ("ConditionalOperator", input.conditional_operator.is_some()), - ("KeyConditions", has_kc && kce_present), + ("AttributesToGet", non_empty(body, "AttributesToGet")), + ("QueryFilter", non_empty(body, "QueryFilter")), + ("ConditionalOperator", present(body, "ConditionalOperator")), + ( + "KeyConditions", + non_empty(body, "KeyConditions") && kce_present, + ), ], &[ ( "ProjectionExpression", - input.projection_expression.is_some(), + present(body, "ProjectionExpression"), ), - ("FilterExpression", input.filter_expression.is_some()), + ("FilterExpression", present(body, "FilterExpression")), ("KeyConditionExpression", kce_present), ], )?; - if let Some(values) = &input.expression_attribute_values { - validate_expression_attribute_value_numbers(values)?; + // Query stops at the first invalid enum, ReturnConsumedCapacity ahead of + // Select (measured 2026-08-24); the numbers sit between the two. + crate::validate_enum_fields( + body, + &[crate::EnumField { + json_name: "ReturnConsumedCapacity", + valid: &["INDEXES", "TOTAL", "NONE"], + clause: crate::EnumClause::Named("returnConsumedCapacity"), + }], + )?; + + if let Some(raw) = body.get("ExpressionAttributeValues") + && let Ok(values) = serde_json::from_value::>(raw.clone()) + { + validate_expression_attribute_value_numbers(&values)?; + } + + crate::validate_enum_fields( + body, + &[crate::EnumField { + json_name: "Select", + valid: &[ + "SPECIFIC_ATTRIBUTES", + "COUNT", + "ALL_ATTRIBUTES", + "ALL_PROJECTED_ATTRIBUTES", + ], + clause: crate::EnumClause::Named("select"), + }], + ) +} + +/// Whether `field` is in the body with a value other than `null`, as a typed +/// `Option` field would deserialize to `Some`. +fn present(body: &Value, field: &str) -> bool { + body.get(field).is_some_and(|v| !v.is_null()) +} + +/// Whether `field` is a non-empty JSON object or array in the body. +fn non_empty(body: &Value, field: &str) -> bool { + match body.get(field) { + Some(Value::Object(map)) => !map.is_empty(), + Some(Value::Array(items)) => !items.is_empty(), + _ => false, } - Ok(()) } /// Refuse the first `N` value in `ExpressionAttributeValues` (including `NS` diff --git a/crates/engine/src/query_key_values/tests.rs b/crates/engine/src/query_key_values/tests.rs index 6130dd875..d98e66f05 100644 --- a/crates/engine/src/query_key_values/tests.rs +++ b/crates/engine/src/query_key_values/tests.rs @@ -6,11 +6,11 @@ use extenddb_core::error::DynamoDbError; use extenddb_core::expression::{ ExpressionMaps, KeyCondition, parse_key_condition, tokenize_with_limit, }; -use extenddb_core::types::{AttributeDefinition, AttributeValue, QueryInput, ScalarAttributeType}; +use extenddb_core::types::{AttributeDefinition, AttributeValue, ScalarAttributeType}; use super::{ KEY_CONDITION_TYPE_MISMATCH, LIMIT_BELOW_ONE, validate_expression_attribute_value_numbers, - validate_key_condition_types, validate_query_request_values, + validate_key_condition_types, validate_query_request_body, }; fn defs(pairs: &[(&str, ScalarAttributeType)]) -> Vec { @@ -226,10 +226,6 @@ fn undefined_placeholder_and_undefined_attribute_are_left_to_other_checks() { assert!(validate_key_condition_types(&kc, &m, &defs(SS)).is_ok()); } -fn query(body: serde_json::Value) -> QueryInput { - serde_json::from_value(body).expect("QueryInput") -} - #[test] fn number_values_that_are_not_numbers_are_refused_with_the_bare_message() { let cases = [ @@ -339,7 +335,7 @@ fn request_time_checks_run_limit_then_mix_then_numbers() { let mut with_limit = bad_number.clone(); with_limit["Limit"] = serde_json::json!(0); assert_eq!( - message(validate_query_request_values(&query(with_limit))).as_deref(), + message(validate_query_request_body(&with_limit)).as_deref(), Some(LIMIT_BELOW_ONE) ); @@ -348,7 +344,7 @@ fn request_time_checks_run_limit_then_mix_then_numbers() { "pk": {"ComparisonOperator": "EQ", "AttributeValueList": [{"S": "x"}]} }); assert_eq!( - message(validate_query_request_values(&query(with_mix))).as_deref(), + message(validate_query_request_body(&with_mix)).as_deref(), Some( "Can not use both expression and non-expression parameters in the same request: \ Non-expression parameters: {KeyConditions} Expression parameters: {KeyConditionExpression}" @@ -356,7 +352,7 @@ fn request_time_checks_run_limit_then_mix_then_numbers() { ); assert_eq!( - message(validate_query_request_values(&query(bad_number))).as_deref(), + message(validate_query_request_body(&bad_number)).as_deref(), Some("The parameter cannot be converted to a numeric value: bob") ); @@ -375,7 +371,7 @@ fn request_time_checks_run_limit_then_mix_then_numbers() { }), ] { assert_eq!( - message(validate_query_request_values(&query(body))).as_deref(), + message(validate_query_request_body(&body)).as_deref(), Some("The parameter cannot be converted to a numeric value: bob") ); } @@ -391,5 +387,53 @@ fn request_time_checks_run_limit_then_mix_then_numbers() { "ExpressionAttributeValues": {":n": {"N": "5"}}, "Limit": 1, }); - assert!(validate_query_request_values(&query(legacy)).is_ok()); + assert!(validate_query_request_body(&legacy).is_ok()); +} + +#[test] +fn enum_checks_sit_on_either_side_of_the_numbers() { + // Measured 2026-10-24: an invalid ReturnConsumedCapacity is reported ahead + // of an N value that is not a number, and that value ahead of an invalid + // Select; a Limit of 0 and a parameter mix ahead of either enum. + let bad_number = serde_json::json!({ + "TableName": "t", + "KeyConditionExpression": "pk = :u", + "ExpressionAttributeValues": {":u": {"N": "bob"}}, + }); + let mut rcc = bad_number.clone(); + rcc["ReturnConsumedCapacity"] = serde_json::json!("bad"); + assert_eq!( + message(validate_query_request_body(&rcc)).as_deref(), + Some( + "1 validation error detected: Value 'bad' at 'returnConsumedCapacity' failed to \ + satisfy constraint: Member must satisfy enum value set: [INDEXES, TOTAL, NONE]" + ) + ); + let mut select = bad_number.clone(); + select["Select"] = serde_json::json!("bad"); + assert_eq!( + message(validate_query_request_body(&select)).as_deref(), + Some("The parameter cannot be converted to a numeric value: bob") + ); + for field in ["ReturnConsumedCapacity", "Select"] { + let mut limit = bad_number.clone(); + limit[field] = serde_json::json!("bad"); + limit["Limit"] = serde_json::json!(0); + assert_eq!( + message(validate_query_request_body(&limit)).as_deref(), + Some(LIMIT_BELOW_ONE), + "{field}" + ); + } + // With the numbers valid, an invalid Select is still reported. + let mut only_select = serde_json::json!({ + "TableName": "t", + "KeyConditionExpression": "pk = :u", + "ExpressionAttributeValues": {":u": {"S": "bob"}}, + }); + only_select["Select"] = serde_json::json!("bad"); + assert!( + message(validate_query_request_body(&only_select)) + .is_some_and(|m| m.contains("at 'select' failed to satisfy constraint")) + ); } diff --git a/crates/server/src/request_helpers.rs b/crates/server/src/request_helpers.rs index 9f34b2632..82c694729 100755 --- a/crates/server/src/request_helpers.rs +++ b/crates/server/src/request_helpers.rs @@ -185,17 +185,17 @@ pub(crate) async fn authorize_request( } // A Query's request-time checks (Limit, the expression and non-expression - // parameter mix, every number in ExpressionAttributeValues) run before the - // IAM decision, for every principal. The engine makes the same checks at - // request time; they run here as well because authorization binds no - // dynamodb:LeadingKeys value to a number it cannot parse, and a fence that - // needs the key present (ForAnyValue, a Null guard, the plain operators) - // would otherwise answer AccessDeniedException where ValidationException is - // the request's error. A body the engine cannot deserialize is left to it. - if operation == "Query" - && let Ok(query) = serde_json::from_value::(input.clone()) - { - extenddb_engine::query_key_values::validate_query_request_values(&query)?; + // parameter mix, the ReturnConsumedCapacity and Select enums, every number + // in ExpressionAttributeValues) run before the IAM decision, for every + // principal. The engine makes the same checks at request time; they run + // here as well because authorization binds no dynamodb:LeadingKeys value + // to a number it cannot parse, and a fence that needs the key present + // (ForAnyValue, a Null guard, the plain operators) would otherwise answer + // AccessDeniedException where ValidationException is the request's error. + // The checks read the raw body, so a field that does not deserialize does + // not skip them. + if operation == "Query" { + extenddb_engine::query_key_values::validate_query_request_body(input)?; } // Every other ARN-bearing field (StreamArn, BackupArn, the export TableArn, diff --git a/tests/test_query_key_value_types.py b/tests/test_query_key_value_types.py index 02ee3b317..f01033488 100644 --- a/tests/test_query_key_value_types.py +++ b/tests/test_query_key_value_types.py @@ -8,12 +8,13 @@ value: ``) or when a key condition value's type is not the key attribute's ``AttributeDefinitions`` type (``One or more parameter values were invalid: Condition parameter type does not match schema type``). Every expected -answer below was measured against the service on 2026-10-24 (15:10 UTC and -21:00 UTC) on tables with S/S, S/N and B/B keys, a GSI and an LSI. The rows -also pin where the two checks fall against the other Query checks: the numeric -check after Limit and the expression parameter mix check and before the table -lookup; the type check after the key schema, filter, projection, unused value -and Select checks and before the ExclusiveStartKey check. +answer below was measured against the service on 2026-10-24 (15:10, 21:00, +21:06 and 23:42 UTC) on tables with S/S, S/N and B/B keys, a GSI and an LSI. +The rows also pin where the two checks fall against the other Query checks: +the numeric check after Limit, the expression parameter mix check and the +ReturnConsumedCapacity enum, and before the Select enum and the table lookup; +the type check after the key schema, filter, projection, unused value and +Select checks and before the ExclusiveStartKey check. Dual-target: runs against the service and against extenddb with the same assertions. The LeadingKeys class needs the extenddb management API and is @@ -180,6 +181,10 @@ def bb_table(dynamodb_client): "1 validation error detected: Cannot specify the ProjectionExpression when choosing to " "get ALL_ATTRIBUTES" ) +RCC_MSG = ( + "1 validation error detected: Value 'bad' at 'returnConsumedCapacity' failed to satisfy " + "constraint: Member must satisfy enum value set: [INDEXES, TOTAL, NONE]" +) V = "ValidationException" RNF = "ResourceNotFoundException" @@ -334,6 +339,21 @@ def kce(expr, values, **extra): kce("pk = :p", {":p": SX}, Limit=0, KeyConditions=LEGACY_PK), V, LIMIT_MSG), ("d6 Limit 0 with N bob and an undefined placeholder", "ss", kce("pk = :p AND sk = :zz", {":p": SX, ":n": NB}, Limit=0), V, LIMIT_MSG), + # The two enum checks against the numeric check, Limit and the mix. + ("f1 invalid Select with N bob", "ss", kce("pk = :u", {":u": NB}, Select="bad"), V, NUMERIC + "bob"), + ("f2 invalid ReturnConsumedCapacity with N bob", "ss", + kce("pk = :u", {":u": NB}, ReturnConsumedCapacity="bad"), V, RCC_MSG), + ("f3 invalid Select with Limit 0", "ss", + kce("pk = :u", {":u": {"S": "alice"}}, Select="bad", Limit=0), V, LIMIT_MSG), + ("f4 invalid ReturnConsumedCapacity with Limit 0", "ss", + kce("pk = :u", {":u": {"S": "alice"}}, ReturnConsumedCapacity="bad", Limit=0), V, LIMIT_MSG), + ("f5 invalid Select with KeyConditions and KeyConditionExpression", "ss", + kce("pk = :u", {":u": {"S": "alice"}}, Select="bad", + KeyConditions={"pk": {"ComparisonOperator": "EQ", "AttributeValueList": [{"S": "alice"}]}}), + V, MIX_MSG), + ("f6 Limit -1 with N bob", "ss", kce("pk = :u", {":u": NB}, Limit=-1), V, LIMIT_MSG), + ("f7 Limit 0 with a type mismatch and N bob", "ss", + kce("pk = :u AND sk = :s", {":u": N5, ":s": NB}, Limit=0), V, LIMIT_MSG), # The other value types the earlier record covers. ("pk = BOOL", "ss", kce("pk = :u", {":u": {"BOOL": True}}), V, TYPE_MISMATCH), ("pk = N bob AND pk > S alice", "ss", @@ -471,6 +491,9 @@ def test_every_fence_form_gets_validation_exception_and_no_items(self): "pk = S bob, N bob unused": { "TableName": table, "KeyConditionExpression": "pk = :u", "ExpressionAttributeValues": {":u": {"S": "bob"}, ":n": NB}}, + "pk = N bob, invalid Select": { + "TableName": table, "KeyConditionExpression": "pk = :u", + "ExpressionAttributeValues": {":u": NB}, "Select": "bad"}, } failures = [] clients = {label: self._client(self._fence(condition, table)) From e4f660d620f8ccb7eefc7a1cbadd8d738f9410af Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sun, 25 Oct 2026 00:35:43 +0000 Subject: [PATCH 3/7] docs(query): name the request-time function in the comments that point at it --- crates/engine/src/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/query.rs b/crates/engine/src/query.rs index a4e633073..9b3e6d7e8 100755 --- a/crates/engine/src/query.rs +++ b/crates/engine/src/query.rs @@ -118,7 +118,7 @@ pub async fn handle_query( )?; } - // Limit >= 1 is checked at request time in validate_query_request_values. + // Limit >= 1 is checked at request time in validate_query_request_body. // For index queries, build a key_info that reflects the index's key schema // so the storage layer uses the correct SK column for the index table. @@ -141,7 +141,7 @@ pub async fn handle_query( }; // The legacy versus expression mutual exclusivity check runs at request - // time in validate_query_request_values. + // time in validate_query_request_body. let has_cond_op = input.conditional_operator.is_some(); // An explicitly empty KeyConditionExpression is rejected up front, before From 4a4e92f4d4ac6a31921645ef896e4cff8fdc14a9 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sat, 24 Oct 2026 02:22:31 +0000 Subject: [PATCH 4/7] fix(cassandra): pass the bare index name in IndexNotFound so the Query sentence matches the other backends --- crates/storage-cassandra/src/data/index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/storage-cassandra/src/data/index.rs b/crates/storage-cassandra/src/data/index.rs index 4ebd72dac..3aa353ac6 100644 --- a/crates/storage-cassandra/src/data/index.rs +++ b/crates/storage-cassandra/src/data/index.rs @@ -147,7 +147,7 @@ pub async fn fetch_index_by_name( let row = rows .into_iter() .next() - .ok_or_else(|| StorageError::IndexNotFound(format!("Index {index_name} not found")))?; + .ok_or_else(|| StorageError::IndexNotFound(index_name.to_owned()))?; let index_id: String = get_column(&row, "index_id", "fetch_index_by_name")?; let index_type: String = get_column(&row, "index_type", "fetch_index_by_name")?; From 03caeed76d08f716fedca6a88319253a8bc36560 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sun, 25 Oct 2026 11:44:43 +0000 Subject: [PATCH 5/7] fix(query): accept Unicode decimal digits in the number check and report a shape fault ahead of the request-time checks The request-time number check refused an N value written with a Unicode decimal digit (fullwidth, Arabic-Indic, Devanagari) that the service accepts as the digit it stands for, so a Query that returns rows on the service answered ValidationException here. The check now folds every general category Nd digit to ASCII before the number rules run and echoes the value as the request wrote it when the rules refuse it; the Nd zero code points of Unicode 15.0 live in a module of their own. The fold applies to the check only: the handler and the server's key binding read the value as written. The write paths are unchanged. The checks run on the raw body before deserialization, so a field of the wrong JSON type elsewhere in the body (a string ExpressionAttributeValues beside a Limit of 0, a string ExpressionAttributeNames beside a malformed N, a numeric Select) answered the check's ValidationException where the service and the previous release answered SerializationException. When a check fails, the body is now deserialized as the typed input and a SerializationException from that is returned instead; a ValidationException the deserializer raises does not preempt the check, and a body that passes every check is not deserialized twice. The server's call before dispatch goes through the same function. Measured against the service on 2026-10-25 (11:31 UTC) on tables with S/N and S/S keys, each request twice. Unit tests cover the digit table, the folded check and the shape-fault order; the wire file gains the measured rows for both. --- crates/engine/src/decimal_digits.rs | 122 ++++++++++++++++ crates/engine/src/lib.rs | 1 + crates/engine/src/query_key_values.rs | 88 ++++++++++-- crates/engine/src/query_key_values/tests.rs | 133 +++++++++++++++++ tests/test_query_key_value_types.py | 152 +++++++++++++++++++- 5 files changed, 481 insertions(+), 15 deletions(-) create mode 100644 crates/engine/src/decimal_digits.rs diff --git a/crates/engine/src/decimal_digits.rs b/crates/engine/src/decimal_digits.rs new file mode 100644 index 000000000..7717634dd --- /dev/null +++ b/crates/engine/src/decimal_digits.rs @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Unicode decimal digits (general category `Nd`) folded to ASCII for the +//! request-time number check. +//! +//! Measured 2026-10-25 (us-east-1, each request twice): the service reads a +//! fullwidth, Arabic-Indic or Devanagari digit in an `N` value as the digit it +//! stands for (`{"N": "\u{ff11}"}` in a `FilterExpression` answers OK; thirty +//! nine Devanagari ones answer `Attempting to store more than 38 significant +//! digits in a Number`), and refuses a superscript (`\u{b2}`), a vulgar +//! fraction (`\u{bd}`), a fullwidth minus (`\u{ff0d}`) and a fullwidth full +//! stop (`\u{ff0e}`) with `The parameter cannot be converted to a numeric +//! value: `. The accepted characters are exactly those of category `Nd`; +//! the refused ones are `No` or `Pd` and `Po`. +//! +//! Every `Nd` run in Unicode 15.0 is ten consecutive code points starting at +//! its zero, so the table below holds the 68 zeros (generated with Python's +//! `unicodedata`, Unicode 15.0.0) and a digit's value is its offset from the +//! run's zero. + +use std::borrow::Cow; + +/// The zero of every `Nd` run in Unicode 15.0, ascending. +const ND_ZEROS: [u32; 68] = [ + 0x0030, 0x0660, 0x06F0, 0x07C0, 0x0966, 0x09E6, 0x0A66, 0x0AE6, 0x0B66, 0x0BE6, 0x0C66, 0x0CE6, + 0x0D66, 0x0DE6, 0x0E50, 0x0ED0, 0x0F20, 0x1040, 0x1090, 0x17E0, 0x1810, 0x1946, 0x19D0, 0x1A80, + 0x1A90, 0x1B50, 0x1BB0, 0x1C40, 0x1C50, 0xA620, 0xA8D0, 0xA900, 0xA9D0, 0xA9F0, 0xAA50, 0xABF0, + 0xFF10, 0x104A0, 0x10D30, 0x11066, 0x110F0, 0x11136, 0x111D0, 0x112F0, 0x11450, 0x114D0, + 0x11650, 0x116C0, 0x11730, 0x118E0, 0x11950, 0x11C50, 0x11D50, 0x11DA0, 0x11F50, 0x16A60, + 0x16AC0, 0x16B50, 0x1D7CE, 0x1D7D8, 0x1D7E2, 0x1D7EC, 0x1D7F6, 0x1E140, 0x1E2F0, 0x1E4F0, + 0x1E950, 0x1FBF0, +]; + +/// The ASCII digit `c` stands for when `c` is a Unicode decimal digit +/// (general category `Nd`), or `None` for every other character. +pub(crate) fn ascii_digit(c: char) -> Option { + let cp = u32::from(c); + // The zero of the run that could hold `cp`: the last zero at or below it. + let run = ND_ZEROS + .partition_point(|&zero| zero <= cp) + .checked_sub(1)?; + let offset = cp - ND_ZEROS[run]; + u8::try_from(offset) + .ok() + .filter(|offset| *offset < 10) + .map(|offset| char::from(b'0' + offset)) +} + +/// `s` with every Unicode decimal digit replaced by the ASCII digit it stands +/// for. Borrows `s` when it has no such digit outside ASCII. +pub(crate) fn fold_decimal_digits(s: &str) -> Cow<'_, str> { + if s.is_ascii() || !s.chars().any(|c| !c.is_ascii() && ascii_digit(c).is_some()) { + return Cow::Borrowed(s); + } + Cow::Owned(s.chars().map(|c| ascii_digit(c).unwrap_or(c)).collect()) +} + +#[cfg(test)] +mod tests { + use super::{ND_ZEROS, ascii_digit, fold_decimal_digits}; + + #[test] + fn measured_digits_fold_to_their_ascii_value() { + // Fullwidth one, Arabic-Indic one, Devanagari one (X545-1 K1 to K3), + // fullwidth seven (H545-2 C11). + assert_eq!(ascii_digit('\u{ff11}'), Some('1')); + assert_eq!(ascii_digit('\u{661}'), Some('1')); + assert_eq!(ascii_digit('\u{967}'), Some('1')); + assert_eq!(ascii_digit('\u{ff17}'), Some('7')); + assert_eq!(ascii_digit('0'), Some('0')); + assert_eq!(ascii_digit('9'), Some('9')); + } + + #[test] + fn measured_controls_are_not_digits() { + // Superscript two, vulgar half, fullwidth minus, fullwidth full stop + // (H545-2 C13 to C16): refused by the service. + for c in ['\u{b2}', '\u{bd}', '\u{ff0d}', '\u{ff0e}'] { + assert_eq!(ascii_digit(c), None, "{c:?}"); + } + // Neighbours of a run and non-digit characters. + assert_eq!(ascii_digit('\u{ff10}'), Some('0')); + assert_eq!(ascii_digit('\u{ff19}'), Some('9')); + assert_eq!(ascii_digit('\u{ff1a}'), None); + assert_eq!(ascii_digit('\u{ff0f}'), None); + assert_eq!(ascii_digit('a'), None); + assert_eq!(ascii_digit('.'), None); + assert_eq!(ascii_digit('e'), None); + assert_eq!(ascii_digit('\u{1FBF9}'), Some('9')); + assert_eq!(ascii_digit('\u{1FBFA}'), None); + assert_eq!(ascii_digit(char::MAX), None); + assert_eq!(ascii_digit('\u{0}'), None); + } + + #[test] + fn zeros_are_ascending_and_distinct_runs() { + for pair in ND_ZEROS.windows(2) { + assert!(pair[0] + 10 <= pair[1], "{pair:?}"); + } + } + + #[test] + fn folding_keeps_every_other_character() { + assert_eq!(fold_decimal_digits("\u{ff11}"), "1"); + assert_eq!(fold_decimal_digits("1\u{ff12}"), "12"); + assert_eq!(fold_decimal_digits("1e\u{ff12}"), "1e2"); + assert_eq!(fold_decimal_digits(&"\u{967}".repeat(39)), "1".repeat(39)); + assert_eq!(fold_decimal_digits("\u{ff0d}1"), "\u{ff0d}1"); + assert_eq!(fold_decimal_digits("1\u{ff0e}5"), "1\u{ff0e}5"); + assert_eq!(fold_decimal_digits("\u{b2}"), "\u{b2}"); + assert_eq!(fold_decimal_digits("-1.5e3"), "-1.5e3"); + assert_eq!(fold_decimal_digits(""), ""); + assert!(matches!( + fold_decimal_digits("bob"), + std::borrow::Cow::Borrowed(_) + )); + assert!(matches!( + fold_decimal_digits("\u{ff0d}"), + std::borrow::Cow::Borrowed(_) + )); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index d24b47286..2cb47da7f 100755 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -12,6 +12,7 @@ mod batch_get_item; mod batch_write_item; pub mod capacity_helpers; mod create_table; +mod decimal_digits; mod delete_item; mod delete_table; mod describe_endpoints; diff --git a/crates/engine/src/query_key_values.rs b/crates/engine/src/query_key_values.rs index 7bfb356ba..68e47b657 100644 --- a/crates/engine/src/query_key_values.rs +++ b/crates/engine/src/query_key_values.rs @@ -33,7 +33,8 @@ use extenddb_core::error::DynamoDbError; use extenddb_core::expression::{ Expr, ExpressionMaps, KeyCondition, PathElement, SortKeyCondition, }; -use extenddb_core::types::{AttributeDefinition, AttributeValue, ScalarAttributeType}; +use extenddb_core::types::{AttributeDefinition, AttributeValue, QueryInput, ScalarAttributeType}; +use serde::de::DeserializeOwned; use serde_json::Value; /// The service's text for a key condition value of the wrong type. @@ -55,18 +56,54 @@ const EMPTY_NUMBER: &str = "The parameter cannot be converted to a numeric value /// that value ahead of an invalid `Select`). /// /// The checks read the raw body, so a field that does not deserialize (an -/// invalid `Select`, for one) does not stop the ones before it. A field of the -/// wrong JSON shape is left to the deserializer, which runs next. +/// invalid `Select`, for one) does not stop the ones before it. When a check +/// fails, the body is deserialized as a [`QueryInput`], and a +/// `SerializationException` from that (a field of the wrong JSON shape +/// anywhere in the body) is the answer instead, as the service reports a shape +/// fault ahead of every validation (measured 2026-10-25: a `Limit` of 0 with a +/// string `ExpressionAttributeValues`, a string `ExpressionAttributeNames` with +/// an `N` value that is not a number, and a `Limit` of 0 with `Select: 5` all +/// answer `SerializationException`). A `ValidationException` the deserializer +/// raises (an empty set, a duplicate, an invalid `AttributeValue`) does not +/// preempt the check. A body that passes every check is not deserialized here. /// /// The server runs the same function ahead of the IAM decision, so a principal /// whose `dynamodb:LeadingKeys` fence cannot bind a number that does not parse /// gets the request's `ValidationException` rather than -/// `AccessDeniedException`. +/// `AccessDeniedException`. That call goes through this function too, so it +/// answers the shape fault first as well. /// /// # Errors /// -/// Returns `DynamoDbError::ValidationException` with the service's text. +/// Returns `DynamoDbError::ValidationException` with the service's text, or +/// `DynamoDbError::SerializationException` for a body of the wrong shape. pub fn validate_query_request_body(body: &Value) -> Result<(), DynamoDbError> { + query_request_checks(body).map_err(|check| shape_fault_first::(body, check)) +} + +/// When a request-time check has failed with `check`, the deserializer's +/// `SerializationException` for `body` if there is one, else `check`. The two +/// enum fields stay in the body: an enum value that is a string but not a +/// member deserializes to a `ValidationException`, which this ignores, and one +/// of another JSON type is a shape fault the service reports ahead of the +/// checks (measured 2026-10-25: `Select: 5` with a `Limit` of 0 answers +/// `SerializationException: NUMBER_VALUE cannot be converted to String`). +pub(crate) fn shape_fault_first( + body: &Value, + check: DynamoDbError, +) -> DynamoDbError { + match serde_json::from_value::(body.clone()) { + Err(e) => match crate::deserialize_error(e) { + shape @ DynamoDbError::SerializationException(_) => shape, + _ => check, + }, + Ok(_) => check, + } +} + +/// The `Query` checks in the service's order; see +/// [`validate_query_request_body`]. +fn query_request_checks(body: &Value) -> Result<(), DynamoDbError> { if let Some(limit) = body.get("Limit").and_then(Value::as_i64) && limit < 1 { @@ -153,6 +190,12 @@ fn non_empty(body: &Value, field: &str) -> bool { /// deterministic when several are invalid. An empty `N` answers the message /// with the colon and nothing after it, as the service does on `Query`. /// +/// A Unicode decimal digit (general category `Nd`) counts as the digit it +/// stands for, as the service reads it (measured 2026-10-25: a fullwidth or +/// Arabic-Indic digit is accepted, thirty nine Devanagari ones answer the 38 +/// digit message); see [`crate::decimal_digits`]. The message echoes the value +/// as the request wrote it. +/// /// # Errors /// /// Returns `DynamoDbError::ValidationException` with the service's text. @@ -162,19 +205,36 @@ pub fn validate_expression_attribute_value_numbers( let mut keys: Vec<&String> = values.keys().collect(); keys.sort(); for key in keys { - match extenddb_core::validation::validate_attribute_number(&values[key]) { - Ok(()) => {} - Err(DynamoDbError::ValidationException(msg)) if msg == EMPTY_NUMBER => { - return Err(DynamoDbError::ValidationException(format!( - "{EMPTY_NUMBER}: " - ))); - } - Err(other) => return Err(other), - } + validate_value_numbers(&values[key])?; } Ok(()) } +/// Every number inside one attribute value (`N`, `NS` members, and the values +/// nested in `L` and `M`), with Unicode decimal digits folded for the check. +fn validate_value_numbers(value: &AttributeValue) -> Result<(), DynamoDbError> { + match value { + AttributeValue::N(n) => validate_number_text(n), + AttributeValue::NS(set) => set.iter().try_for_each(|n| validate_number_text(n)), + AttributeValue::L(list) => list.iter().try_for_each(validate_value_numbers), + AttributeValue::M(map) => map.values().try_for_each(validate_value_numbers), + _ => Ok(()), + } +} + +/// One number text against the number rules, with the service's `Query` +/// message for an empty value and the original text echoed for a malformed one. +fn validate_number_text(text: &str) -> Result<(), DynamoDbError> { + let folded = crate::decimal_digits::fold_decimal_digits(text); + match extenddb_core::validation::number::validate_and_normalize_number(&folded) { + Ok(_) => Ok(()), + Err(DynamoDbError::ValidationException(msg)) if msg.starts_with(EMPTY_NUMBER) => Err( + DynamoDbError::ValidationException(format!("{EMPTY_NUMBER}: {text}")), + ), + Err(other) => Err(other), + } +} + /// Refuse a key condition value whose type is not the key's /// `AttributeDefinitions` type. /// diff --git a/crates/engine/src/query_key_values/tests.rs b/crates/engine/src/query_key_values/tests.rs index d98e66f05..e3a03dc2f 100644 --- a/crates/engine/src/query_key_values/tests.rs +++ b/crates/engine/src/query_key_values/tests.rs @@ -437,3 +437,136 @@ fn enum_checks_sit_on_either_side_of_the_numbers() { .is_some_and(|m| m.contains("at 'select' failed to satisfy constraint")) ); } + +#[test] +fn unicode_decimal_digits_count_as_the_digits_they_stand_for() { + // Measured 2026-10-25 (X545-1 K1 to K6, F1 to F6, U1 to U6; H545-2 C11, + // C12, C17): a fullwidth, Arabic-Indic or Devanagari digit is accepted as a + // key condition, filter or unused value; thirty nine Devanagari ones answer + // the 38 digit message; `1\u{ff12}` and `1e\u{ff12}` are accepted. + for raw in [ + "\u{ff11}", + "\u{661}", + "\u{967}", + "1\u{ff12}", + "1e\u{ff12}", + "\u{ff17}", + ] { + let values = HashMap::from([(":u".to_owned(), n(raw))]); + assert!( + validate_expression_attribute_value_numbers(&values).is_ok(), + "{raw:?}" + ); + } + let values = HashMap::from([(":u".to_owned(), n(&"\u{967}".repeat(39)))]); + assert_eq!( + message(validate_expression_attribute_value_numbers(&values)).as_deref(), + Some("Attempting to store more than 38 significant digits in a Number") + ); + // A fullwidth digit as an NS member (H545-2 C18, X545-1 NS1). + let values = HashMap::from([( + ":u".to_owned(), + AttributeValue::NS(["\u{ff11}".to_owned()].into_iter().collect()), + )]); + assert!(validate_expression_attribute_value_numbers(&values).is_ok()); +} + +#[test] +fn characters_that_are_not_decimal_digits_are_refused_with_the_text_as_sent() { + // Measured 2026-10-25 (X545-1 K7 to K10; H545-2 C13 to C16): a superscript, + // a vulgar fraction, a fullwidth minus and a fullwidth full stop are refused + // and the message echoes the value as the request wrote it. + for raw in ["\u{b2}", "\u{bd}", "\u{ff0d}1", "1\u{ff0e}5", "\u{ff11}bob"] { + let values = HashMap::from([(":u".to_owned(), n(raw))]); + assert_eq!( + message(validate_expression_attribute_value_numbers(&values)).as_deref(), + Some(format!("The parameter cannot be converted to a numeric value: {raw}").as_str()), + "{raw:?}" + ); + } +} + +#[test] +fn a_shape_fault_in_another_field_is_reported_ahead_of_the_checks() { + // Measured 2026-10-25 (X545-1 S1 to S6, S8): a field of the wrong JSON type + // anywhere in the body answers SerializationException even when a Limit, + // mix, enum or number check would fail. + let shape_faults = [ + serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "Limit": 0, "ExpressionAttributeValues": "x", + }), + serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "ExpressionAttributeNames": "x", + "ExpressionAttributeValues": {":p": {"N": "bob"}}, + }), + serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "Limit": "1", "ExpressionAttributeValues": {":p": {"N": "bob"}}, + }), + serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "AttributesToGet": {"a": 1}, "ProjectionExpression": "a", + "ExpressionAttributeValues": {":p": {"S": "x"}}, + }), + serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "QueryFilter": [1], "FilterExpression": "a = :p", + "ExpressionAttributeValues": {":p": {"S": "x"}}, + }), + serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "Select": 5, "ExpressionAttributeValues": {":p": {"N": "bob"}}, + }), + serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "Limit": 0, "Select": 5, "ExpressionAttributeValues": {":p": {"S": "x"}}, + }), + ]; + for body in shape_faults { + assert!( + matches!( + validate_query_request_body(&body), + Err(DynamoDbError::SerializationException(_)) + ), + "{body}" + ); + } + + // A string enum value that is not a member is the checks' own answer, not a + // shape fault (X545-1 S7 and the 2026-10-24 enum rows). + let mut select = serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "Limit": 0, "ExpressionAttributeValues": {":p": {"S": "x"}}, + }); + select["Select"] = serde_json::json!("bad"); + assert_eq!( + message(validate_query_request_body(&select)).as_deref(), + Some(LIMIT_BELOW_ONE) + ); + select["ReturnConsumedCapacity"] = serde_json::json!("bad"); + select["Limit"] = serde_json::json!(1); + assert!( + message(validate_query_request_body(&select)) + .is_some_and(|m| m.contains("at 'returnConsumedCapacity' failed to satisfy constraint")) + ); + + // A ValidationException the deserializer raises does not preempt a check: + // an empty NS beside a Limit of 0 answers the Limit text. + let empty_set = serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "Limit": 0, "ExpressionAttributeValues": {":p": {"S": "x"}, ":s": {"NS": []}}, + }); + assert_eq!( + message(validate_query_request_body(&empty_set)).as_deref(), + Some(LIMIT_BELOW_ONE) + ); + + // A body that passes every check is left to the handler's deserializer. + let shape_fault_only = serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "ExpressionAttributeValues": "x", + }); + assert!(validate_query_request_body(&shape_fault_only).is_ok()); +} diff --git a/tests/test_query_key_value_types.py b/tests/test_query_key_value_types.py index f01033488..cb5c802ea 100644 --- a/tests/test_query_key_value_types.py +++ b/tests/test_query_key_value_types.py @@ -9,7 +9,10 @@ attribute's ``AttributeDefinitions`` type (``One or more parameter values were invalid: Condition parameter type does not match schema type``). Every expected answer below was measured against the service on 2026-10-24 (15:10, 21:00, -21:06 and 23:42 UTC) on tables with S/S, S/N and B/B keys, a GSI and an LSI. +21:06 and 23:42 UTC) and 2026-10-25 (11:31 UTC) on tables with S/S, S/N and +B/B keys, a GSI and an LSI. A Unicode decimal digit in an N value counts as +the digit it stands for; a field of the wrong JSON type anywhere in the body +answers SerializationException ahead of every check. The rows also pin where the two checks fall against the other Query checks: the numeric check after Limit, the expression parameter mix check and the ReturnConsumedCapacity enum, and before the Select enum and the table lookup; @@ -115,6 +118,10 @@ def ns_table(dynamodb_client): for pk in ("alice", "bob"): dynamodb_client.put_item(TableName=name, Item={ "pk": {"S": pk}, "sk": {"N": "5"}, "gpk": {"S": pk}, "gsk": {"S": "g"}}) + # The item the Unicode digit rows read: sk N 1 and a N 1 in ASCII. + dynamodb_client.put_item(TableName=name, Item={ + "pk": {"S": "carol"}, "sk": {"N": "1"}, "a": {"N": "1"}, + "gpk": {"S": "carol"}, "gsk": {"S": "g"}}) yield name @@ -165,6 +172,14 @@ def bb_table(dynamodb_client): ) NO_INDEX = "The table does not have the specified index: nope" UNUSED_X = "Value provided in ExpressionAttributeValues unused in expressions: keys: {:x}" +UNUSED_F = "Value provided in ExpressionAttributeValues unused in expressions: keys: {:f}" +# Unicode decimal digits (general category Nd) and characters that are not. +FW_ONE = "\uff11" # fullwidth digit one +AR_ONE = "\u0661" # Arabic-Indic digit one +DEVA_ONE = "\u0967" # Devanagari digit one +# Superscript two, vulgar half, fullwidth minus then 1, 1 fullwidth full stop 5. +NOT_DIGITS = ["\u00b2", "\u00bd", "\uff0d1", "1\uff0e5"] +CAROL = {"S": "carol"} GSI_CONSISTENT = "Consistent reads are not supported on global secondary indexes" FILTER_BEGINS_WITH_N = ( "Invalid FilterExpression: Incorrect operand type for operator or function; " @@ -354,6 +369,35 @@ def kce(expr, values, **extra): ("f6 Limit -1 with N bob", "ss", kce("pk = :u", {":u": NB}, Limit=-1), V, LIMIT_MSG), ("f7 Limit 0 with a type mismatch and N bob", "ss", kce("pk = :u AND sk = :s", {":u": N5, ":s": NB}, Limit=0), V, LIMIT_MSG), + # Unicode decimal digits are read as the digits they stand for; other + # characters are refused with the value echoed as sent (measured + # 2026-10-25 11:31 UTC on an S/N table). + ("g1 39 Devanagari ones as sk", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": DEVA_ONE * 39}}), V, DIGITS_MSG), + ("g2 39 Devanagari ones in filter", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": "1"}, ":f": {"N": DEVA_ONE * 39}}, + FilterExpression="a = :f"), V, DIGITS_MSG), + ("g3 39 Devanagari ones unused", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": "1"}, ":f": {"N": DEVA_ONE * 39}}), + V, DIGITS_MSG), + *[(f"g4 {raw!r} as sk", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": raw}}), V, NUMERIC + raw) + for raw in NOT_DIGITS], + *[(f"g5 {raw!r} in filter", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": "1"}, ":f": {"N": raw}}, + FilterExpression="a = :f"), V, NUMERIC + raw) + for raw in NOT_DIGITS], + *[(f"g6 {raw!r} unused", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": "1"}, ":f": {"N": raw}}), + V, NUMERIC + raw) + for raw in NOT_DIGITS], + *[(f"g7 {raw!r} unused passes the number check", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": "1"}, ":f": {"N": raw}}), + V, UNUSED_F) + for raw in (FW_ONE, AR_ONE, DEVA_ONE, "1\uff12", "1e\uff12")], + ("g8 NS with a fullwidth digit unused", "ns", + kce("pk = :u AND sk = :s", {":u": CAROL, ":s": {"N": "1"}, ":f": {"NS": [FW_ONE]}}), + V, UNUSED_F), # The other value types the earlier record covers. ("pk = BOOL", "ss", kce("pk = :u", {":u": {"BOOL": True}}), V, TYPE_MISMATCH), ("pk = N bob AND pk > S alice", "ss", @@ -392,6 +436,112 @@ def test_control_matching_types_on_every_table(self, dynamodb_client, ss_table, ExpressionAttributeValues={":u": B_BOB})["Count"] == 1 +class TestUnicodeDecimalDigitsAreRead: + """A Unicode decimal digit in an N value is the digit it stands for: as a + key condition value it reads the item stored with the ASCII digit, as a + filter value it matches the ASCII attribute (measured 2026-10-25 11:31 UTC + on an S/N table holding sk N 1 and a N 1).""" + + DIGITS = [FW_ONE, AR_ONE, DEVA_ONE] + DIGIT_IDS = ["fullwidth", "arabic-indic", "devanagari"] + + @pytest.mark.parametrize("digit", DIGITS, ids=DIGIT_IDS) + def test_key_condition_value_reads_the_ascii_item(self, dynamodb_client, ns_table, digit): + resp = dynamodb_client.query( + TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", + ExpressionAttributeValues={":u": CAROL, ":s": {"N": digit}}) + assert resp["Count"] == 1 + assert resp["Items"][0]["sk"] == {"N": "1"} + + @pytest.mark.parametrize("digit", DIGITS, ids=DIGIT_IDS) + def test_filter_value_matches_the_ascii_attribute(self, dynamodb_client, ns_table, digit): + resp = dynamodb_client.query( + TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", + FilterExpression="a = :f", + ExpressionAttributeValues={":u": CAROL, ":s": {"N": "1"}, ":f": {"N": digit}}) + assert resp["Count"] == 1 + assert resp["Items"][0]["a"] == {"N": "1"} + + @pytest.mark.parametrize("raw", ["1\uff12", "1e\uff12"], ids=["1-fullwidth-2", "1e-fullwidth-2"]) + def test_mixed_digits_are_one_number(self, dynamodb_client, ns_table, raw): + # Read as 12 and 1e2: accepted, matching nothing stored. + as_key = dynamodb_client.query( + TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", + ExpressionAttributeValues={":u": CAROL, ":s": {"N": raw}}) + assert (as_key["Count"], as_key["ScannedCount"]) == (0, 0) + in_filter = dynamodb_client.query( + TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", + FilterExpression="a = :f", + ExpressionAttributeValues={":u": CAROL, ":s": {"N": "1"}, ":f": {"N": raw}}) + assert (in_filter["Count"], in_filter["ScannedCount"]) == (0, 1) + + def test_fullwidth_digit_in_a_number_set_passes_the_check(self, dynamodb_client, ns_table): + resp = dynamodb_client.query( + TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", + FilterExpression="contains(ns, :f)", + ExpressionAttributeValues={":u": CAROL, ":s": {"N": "1"}, ":f": {"NS": [FW_ONE]}}) + assert (resp["Count"], resp["ScannedCount"]) == (0, 1) + + +def _raw_query(client, body: dict): + """POST ``body`` as the Query JSON exactly as written, signed with the + client's credentials, so a field of the wrong JSON type reaches the server + (boto3 would refuse or reshape it).""" + import json + + from botocore.awsrequest import AWSRequest + + req = AWSRequest(method="POST", url=client.meta.endpoint_url, data=json.dumps(body), + headers={"Content-Type": "application/x-amz-json-1.0", + "X-Amz-Target": "DynamoDB_20120810.Query"}) + client._request_signer.sign("Query", req) + resp = client._endpoint.http_session.send(req.prepare()) + return resp.status_code, json.loads(resp.content.decode("utf-8", "replace") or "{}") + + +SHAPE_FAULTS = [ + ("Limit 0 with ExpressionAttributeValues string", + {"KeyConditionExpression": "pk = :p", "Limit": 0, "ExpressionAttributeValues": "x"}), + ("ExpressionAttributeNames string with N bob", + {"KeyConditionExpression": "pk = :p", "ExpressionAttributeNames": "x", + "ExpressionAttributeValues": {":p": NB}}), + ("Limit string with N bob", + {"KeyConditionExpression": "pk = :p", "Limit": "1", "ExpressionAttributeValues": {":p": NB}}), + ("AttributesToGet object with ProjectionExpression", + {"KeyConditionExpression": "pk = :p", "AttributesToGet": {"a": 1}, "ProjectionExpression": "a", + "ExpressionAttributeValues": {":p": SX}}), + ("QueryFilter array with FilterExpression", + {"KeyConditionExpression": "pk = :p", "QueryFilter": [1], "FilterExpression": "a = :p", + "ExpressionAttributeValues": {":p": SX}}), + ("Select number with N bob", + {"KeyConditionExpression": "pk = :p", "Select": 5, "ExpressionAttributeValues": {":p": NB}}), + ("Limit 0 with Select number", + {"KeyConditionExpression": "pk = :p", "Limit": 0, "Select": 5, + "ExpressionAttributeValues": {":p": SX}}), +] + + +class TestShapeFaultsAreReportedFirst: + """A field of the wrong JSON type anywhere in the body answers + SerializationException ahead of the Limit, parameter mix, enum and number + checks (measured 2026-10-25 11:31 UTC). The message texts differ between + the service and extenddb, so only the error type is asserted.""" + + @pytest.mark.parametrize("row,body", SHAPE_FAULTS, ids=[r[0] for r in SHAPE_FAULTS]) + def test_rows(self, dynamodb_client, ss_table, row, body): + status, payload = _raw_query(dynamodb_client, {"TableName": ss_table, **body}) + assert status == 400 + assert payload.get("__type", "").endswith("SerializationException"), payload + + def test_control_without_a_shape_fault_answers_the_check(self, dynamodb_client, ss_table): + status, payload = _raw_query(dynamodb_client, { + "TableName": ss_table, "KeyConditionExpression": "pk = :p", "Limit": 0, + "ExpressionAttributeValues": {":p": NB}}) + assert status == 400 + assert payload.get("__type", "").endswith("ValidationException") + assert payload.get("message", payload.get("Message")) == LIMIT_MSG + + class TestLeadingKeysWithAnInvalidKeyValue: """A principal under each LeadingKeys fence form gets ValidationException and no items for a Query whose ExpressionAttributeValues carries an N value that From bde92d3f11930294c8da437aacabc66ac1b33f04 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sun, 25 Oct 2026 13:46:22 +0000 Subject: [PATCH 6/7] fix(query): leave an unknown type descriptor to the request-time checks, and assert only acceptance for Unicode digits The shape-fault pass returned the deserializer's SerializationException for an AttributeValue with an unknown type descriptor, so Limit 0 beside {"X": "1"} changed class. The service answers the Limit ValidationException for that body (measured 2026-10-25), and the check's answer is returned again. The Unicode digit tests asserted that a key condition or filter written with a Unicode digit reads the item stored with the ASCII digit. This change makes the request-time check accept such a value; it does not change how the handler reads it, so the tests assert only that the value is accepted. A new test checks that a partition key value written with Unicode digits returns no item of a partition a ForAllValues LeadingKeys fence keeps the caller from. --- crates/engine/src/decimal_digits.rs | 13 ++-- crates/engine/src/query_key_values.rs | 12 ++++ crates/engine/src/query_key_values/tests.rs | 23 +++++-- tests/test_query_key_value_types.py | 70 ++++++++++++++------- 4 files changed, 84 insertions(+), 34 deletions(-) diff --git a/crates/engine/src/decimal_digits.rs b/crates/engine/src/decimal_digits.rs index 7717634dd..8aeb24130 100644 --- a/crates/engine/src/decimal_digits.rs +++ b/crates/engine/src/decimal_digits.rs @@ -17,6 +17,11 @@ //! its zero, so the table below holds the 68 zeros (generated with Python's //! `unicodedata`, Unicode 15.0.0) and a digit's value is its offset from the //! run's zero. +//! +//! The fold applies to the check only. The handler and the server's +//! `dynamodb:LeadingKeys` binding read the value as the request wrote it, so +//! both see the same text; reading a Unicode digit as the digit has to change +//! both together. use std::borrow::Cow; @@ -61,8 +66,8 @@ mod tests { #[test] fn measured_digits_fold_to_their_ascii_value() { - // Fullwidth one, Arabic-Indic one, Devanagari one (X545-1 K1 to K3), - // fullwidth seven (H545-2 C11). + // Fullwidth one, Arabic-Indic one, Devanagari one and fullwidth seven, + // measured 2026-10-25. assert_eq!(ascii_digit('\u{ff11}'), Some('1')); assert_eq!(ascii_digit('\u{661}'), Some('1')); assert_eq!(ascii_digit('\u{967}'), Some('1')); @@ -73,8 +78,8 @@ mod tests { #[test] fn measured_controls_are_not_digits() { - // Superscript two, vulgar half, fullwidth minus, fullwidth full stop - // (H545-2 C13 to C16): refused by the service. + // Superscript two, vulgar half, fullwidth minus, fullwidth full stop: + // refused by the service (measured 2026-10-25). for c in ['\u{b2}', '\u{bd}', '\u{ff0d}', '\u{ff0e}'] { assert_eq!(ascii_digit(c), None, "{c:?}"); } diff --git a/crates/engine/src/query_key_values.rs b/crates/engine/src/query_key_values.rs index 68e47b657..54886bf96 100644 --- a/crates/engine/src/query_key_values.rs +++ b/crates/engine/src/query_key_values.rs @@ -88,12 +88,20 @@ pub fn validate_query_request_body(body: &Value) -> Result<(), DynamoDbError> { /// of another JSON type is a shape fault the service reports ahead of the /// checks (measured 2026-10-25: `Select: 5` with a `Limit` of 0 answers /// `SerializationException: NUMBER_VALUE cannot be converted to String`). +/// +/// An AttributeValue with an unknown type descriptor is not a shape fault to +/// the service: `Limit: 0` beside `{"X": "1"}` answers the `Limit` +/// `ValidationException` (measured 2026-10-25), so the deserializer's +/// `SerializationException` for that message does not preempt the check. pub(crate) fn shape_fault_first( body: &Value, check: DynamoDbError, ) -> DynamoDbError { match serde_json::from_value::(body.clone()) { Err(e) => match crate::deserialize_error(e) { + DynamoDbError::SerializationException(msg) if msg.contains(UNKNOWN_TYPE_DESCRIPTOR) => { + check + } shape @ DynamoDbError::SerializationException(_) => shape, _ => check, }, @@ -101,6 +109,10 @@ pub(crate) fn shape_fault_first( } } +/// The deserializer's text for an AttributeValue whose type descriptor is not +/// one of the service's. +const UNKNOWN_TYPE_DESCRIPTOR: &str = "unknown AttributeValue type descriptor"; + /// The `Query` checks in the service's order; see /// [`validate_query_request_body`]. fn query_request_checks(body: &Value) -> Result<(), DynamoDbError> { diff --git a/crates/engine/src/query_key_values/tests.rs b/crates/engine/src/query_key_values/tests.rs index e3a03dc2f..e9958fe59 100644 --- a/crates/engine/src/query_key_values/tests.rs +++ b/crates/engine/src/query_key_values/tests.rs @@ -440,8 +440,7 @@ fn enum_checks_sit_on_either_side_of_the_numbers() { #[test] fn unicode_decimal_digits_count_as_the_digits_they_stand_for() { - // Measured 2026-10-25 (X545-1 K1 to K6, F1 to F6, U1 to U6; H545-2 C11, - // C12, C17): a fullwidth, Arabic-Indic or Devanagari digit is accepted as a + // Measured 2026-10-25: a fullwidth, Arabic-Indic or Devanagari digit is accepted as a // key condition, filter or unused value; thirty nine Devanagari ones answer // the 38 digit message; `1\u{ff12}` and `1e\u{ff12}` are accepted. for raw in [ @@ -463,7 +462,7 @@ fn unicode_decimal_digits_count_as_the_digits_they_stand_for() { message(validate_expression_attribute_value_numbers(&values)).as_deref(), Some("Attempting to store more than 38 significant digits in a Number") ); - // A fullwidth digit as an NS member (H545-2 C18, X545-1 NS1). + // A fullwidth digit as an NS member. let values = HashMap::from([( ":u".to_owned(), AttributeValue::NS(["\u{ff11}".to_owned()].into_iter().collect()), @@ -473,7 +472,7 @@ fn unicode_decimal_digits_count_as_the_digits_they_stand_for() { #[test] fn characters_that_are_not_decimal_digits_are_refused_with_the_text_as_sent() { - // Measured 2026-10-25 (X545-1 K7 to K10; H545-2 C13 to C16): a superscript, + // Measured 2026-10-25: a superscript, // a vulgar fraction, a fullwidth minus and a fullwidth full stop are refused // and the message echoes the value as the request wrote it. for raw in ["\u{b2}", "\u{bd}", "\u{ff0d}1", "1\u{ff0e}5", "\u{ff11}bob"] { @@ -488,7 +487,7 @@ fn characters_that_are_not_decimal_digits_are_refused_with_the_text_as_sent() { #[test] fn a_shape_fault_in_another_field_is_reported_ahead_of_the_checks() { - // Measured 2026-10-25 (X545-1 S1 to S6, S8): a field of the wrong JSON type + // Measured 2026-10-25: a field of the wrong JSON type // anywhere in the body answers SerializationException even when a Limit, // mix, enum or number check would fail. let shape_faults = [ @@ -535,7 +534,7 @@ fn a_shape_fault_in_another_field_is_reported_ahead_of_the_checks() { } // A string enum value that is not a member is the checks' own answer, not a - // shape fault (X545-1 S7 and the 2026-10-24 enum rows). + // shape fault. let mut select = serde_json::json!({ "TableName": "t", "KeyConditionExpression": "pk = :p", "Limit": 0, "ExpressionAttributeValues": {":p": {"S": "x"}}, @@ -563,6 +562,18 @@ fn a_shape_fault_in_another_field_is_reported_ahead_of_the_checks() { Some(LIMIT_BELOW_ONE) ); + // Neither does an unknown type descriptor, which the deserializer reports + // as a SerializationException: the service answers the Limit text + // (measured 2026-10-25). + let unknown_type = serde_json::json!({ + "TableName": "t", "KeyConditionExpression": "pk = :p", + "Limit": 0, "ExpressionAttributeValues": {":p": {"S": "x"}, ":a": {"X": "1"}}, + }); + assert_eq!( + message(validate_query_request_body(&unknown_type)).as_deref(), + Some(LIMIT_BELOW_ONE) + ); + // A body that passes every check is left to the handler's deserializer. let shape_fault_only = serde_json::json!({ "TableName": "t", "KeyConditionExpression": "pk = :p", diff --git a/tests/test_query_key_value_types.py b/tests/test_query_key_value_types.py index cb5c802ea..c336b6acc 100644 --- a/tests/test_query_key_value_types.py +++ b/tests/test_query_key_value_types.py @@ -436,44 +436,31 @@ def test_control_matching_types_on_every_table(self, dynamodb_client, ss_table, ExpressionAttributeValues={":u": B_BOB})["Count"] == 1 -class TestUnicodeDecimalDigitsAreRead: - """A Unicode decimal digit in an N value is the digit it stands for: as a - key condition value it reads the item stored with the ASCII digit, as a - filter value it matches the ASCII attribute (measured 2026-10-25 11:31 UTC - on an S/N table holding sk N 1 and a N 1).""" +class TestUnicodeDecimalDigitsPassTheCheck: + """A Unicode decimal digit in an N value passes the request-time number + check: the service reads it as the digit it stands for (measured 2026-10-25 + 11:31 UTC). Reading such a value as the digit is not part of this change, + so only the acceptance is asserted, in the positions where the request + reaches no numeric key comparison.""" DIGITS = [FW_ONE, AR_ONE, DEVA_ONE] DIGIT_IDS = ["fullwidth", "arabic-indic", "devanagari"] @pytest.mark.parametrize("digit", DIGITS, ids=DIGIT_IDS) - def test_key_condition_value_reads_the_ascii_item(self, dynamodb_client, ns_table, digit): - resp = dynamodb_client.query( - TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", - ExpressionAttributeValues={":u": CAROL, ":s": {"N": digit}}) - assert resp["Count"] == 1 - assert resp["Items"][0]["sk"] == {"N": "1"} - - @pytest.mark.parametrize("digit", DIGITS, ids=DIGIT_IDS) - def test_filter_value_matches_the_ascii_attribute(self, dynamodb_client, ns_table, digit): + def test_filter_value_is_accepted(self, dynamodb_client, ns_table, digit): resp = dynamodb_client.query( TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", FilterExpression="a = :f", ExpressionAttributeValues={":u": CAROL, ":s": {"N": "1"}, ":f": {"N": digit}}) - assert resp["Count"] == 1 - assert resp["Items"][0]["a"] == {"N": "1"} + assert resp["ScannedCount"] == 1 @pytest.mark.parametrize("raw", ["1\uff12", "1e\uff12"], ids=["1-fullwidth-2", "1e-fullwidth-2"]) - def test_mixed_digits_are_one_number(self, dynamodb_client, ns_table, raw): - # Read as 12 and 1e2: accepted, matching nothing stored. - as_key = dynamodb_client.query( - TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", - ExpressionAttributeValues={":u": CAROL, ":s": {"N": raw}}) - assert (as_key["Count"], as_key["ScannedCount"]) == (0, 0) - in_filter = dynamodb_client.query( + def test_mixed_digits_in_the_filter_are_accepted(self, dynamodb_client, ns_table, raw): + resp = dynamodb_client.query( TableName=ns_table, KeyConditionExpression="pk = :u AND sk = :s", FilterExpression="a = :f", ExpressionAttributeValues={":u": CAROL, ":s": {"N": "1"}, ":f": {"N": raw}}) - assert (in_filter["Count"], in_filter["ScannedCount"]) == (0, 1) + assert (resp["Count"], resp["ScannedCount"]) == (0, 1) def test_fullwidth_digit_in_a_number_set_passes_the_check(self, dynamodb_client, ns_table): resp = dynamodb_client.query( @@ -660,3 +647,38 @@ def test_every_fence_form_gets_validation_exception_and_no_items(self): failures.append(f"{shape} / {label}: {err['Code']} {err['Message']}") owner.delete_table(TableName=table) assert not failures, "\n".join(failures) + + def test_a_unicode_digit_key_value_reads_no_fenced_partition(self): + """A partition key value written with Unicode digits is not bound to a + LeadingKeys value, so it must not read a partition the fence keeps the + principal from. The service's answer for a fenced principal cannot be + measured here; the test asserts only that no item of partition 20 is + returned, whatever the answer is.""" + owner = self._client({ + "Version": "2012-10-17", + "Statement": [{"Effect": "Allow", "Action": "dynamodb:*", "Resource": "*"}], + }) + table = f"kv-{uuid.uuid4().hex[:8]}" + owner.create_table( + TableName=table, + AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "N"}], + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + BillingMode="PAY_PER_REQUEST") + wait_for_active(owner, table) + for pk in ("10", "20"): + owner.put_item(TableName=table, Item={"pk": {"N": pk}, "v": {"S": "p" + pk}}) + fenced = self._client(self._fence( + {"ForAllValues:StringEquals": {"dynamodb:LeadingKeys": ["10"]}}, table)) + leaks = [] + for label, value in (("fullwidth", "\uff12\uff10"), ("arabic-indic", "\u0662\u0660"), + ("devanagari", "\u0968\u0966")): + try: + resp = fenced.query( + TableName=table, KeyConditionExpression="pk = :p", + ExpressionAttributeValues={":p": {"N": value}}) + except ClientError: + continue + if any(item.get("v") == {"S": "p20"} for item in resp["Items"]): + leaks.append(label) + owner.delete_table(TableName=table) + assert not leaks, leaks From 3a633fd13a637f796f8bf27667ea83c58dca2bef Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Sun, 25 Oct 2026 16:01:04 +0000 Subject: [PATCH 7/7] test(query): say which Limit text the service uses beside an unknown type descriptor --- crates/engine/src/query_key_values/tests.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/query_key_values/tests.rs b/crates/engine/src/query_key_values/tests.rs index e9958fe59..f1ba2d3e6 100644 --- a/crates/engine/src/query_key_values/tests.rs +++ b/crates/engine/src/query_key_values/tests.rs @@ -563,8 +563,9 @@ fn a_shape_fault_in_another_field_is_reported_ahead_of_the_checks() { ); // Neither does an unknown type descriptor, which the deserializer reports - // as a SerializationException: the service answers the Limit text - // (measured 2026-10-25). + // as a SerializationException: the service answers the Limit + // ValidationException (measured 2026-10-25), in its older + // `Value '0' at 'limit'` form; ExtendDB answers its usual Limit text. let unknown_type = serde_json::json!({ "TableName": "t", "KeyConditionExpression": "pk = :p", "Limit": 0, "ExpressionAttributeValues": {":p": {"S": "x"}, ":a": {"X": "1"}},