From 57ab1046e85c16f3fcfadce3d4559c3ef6da8004 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 14:13:17 +0000 Subject: [PATCH 01/10] fix: validate item size before the table-existence check (PutItem, BatchWriteItem) A genuinely oversized item sent to a non-existent table now returns ValidationException ("Item size has exceeded the maximum allowed size") instead of ResourceNotFoundException, matching Amazon DynamoDB (verified against the service). The item-size limit is schema-independent, so it is validated before the table is resolved; key-schema-dependent checks stay after the existence check. Also corrects the request-validation precedence tests: the previous item-size case used a ~400,004-byte item, which is under the 409,600-byte (400 KB) limit and so was never oversized -- it returned ResourceNotFoundException only because the item was valid and the table absent. The tests now use a genuinely oversized item and assert ValidationException. Signed-off-by: Lee Hannigan --- crates/engine/src/batch_write_item.rs | 13 ++++++++- crates/engine/src/put_item.rs | 6 ++++ tests/test_validation_precedence.py | 42 +++++++++++++++------------ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/batch_write_item.rs b/crates/engine/src/batch_write_item.rs index b6a4dd62..7c4b6fd3 100755 --- a/crates/engine/src/batch_write_item.rs +++ b/crates/engine/src/batch_write_item.rs @@ -98,6 +98,18 @@ pub async fn handle_batch_write_item( } } + // Item-size limit is validated across the whole request before any table + // existence check, so an oversized item to a missing table returns + // ValidationException (matching Amazon DynamoDB), not + // ResourceNotFoundException. Key-schema-dependent checks stay post-existence. + for reqs in input.request_items.values() { + for wr in reqs { + if let Some(put) = &wr.put_request { + validate_item_size(&put.item, ctx.limits.max_item_size_bytes)?; + } + } + } + let empty_maps = ExpressionMaps::default(); let mut all_icm: HashMap> = HashMap::new(); @@ -124,7 +136,6 @@ pub async fn handle_batch_write_item( &key_info.attribute_definitions, )?; validate_item_nesting_depth(&put.item)?; - validate_item_size(&put.item, ctx.limits.max_item_size_bytes)?; validate_attribute_name_sizes(&put.item, &ctx.limits)?; validate_key_sizes(&put.item, &key_info.key_schema, &ctx.limits)?; diff --git a/crates/engine/src/put_item.rs b/crates/engine/src/put_item.rs index cf964fcf..d384e842 100755 --- a/crates/engine/src/put_item.rs +++ b/crates/engine/src/put_item.rs @@ -148,6 +148,12 @@ pub async fn handle_put_item( )?; } + // Item-size limit is validated before the existence check so a genuinely + // oversized item to a missing table returns ValidationException (matching + // Amazon DynamoDB), not ResourceNotFoundException. Key/attribute-definition + // checks stay after existence — they need the table's key schema. + extenddb_core::validation::validate_item_size(&input.item, ctx.limits.max_item_size_bytes)?; + let key_info = ctx .table_key_info(&input.table_name) .await diff --git a/tests/test_validation_precedence.py b/tests/test_validation_precedence.py index 69bc8bf8..96466445 100644 --- a/tests/test_validation_precedence.py +++ b/tests/test_validation_precedence.py @@ -28,9 +28,15 @@ it against an existing table (Amazon DynamoDB requires a Value). Some other checks are intentionally *post-existence* on Amazon DynamoDB and -must keep returning ``ResourceNotFoundException`` for an absent table. The most -visible one is the item-size limit (PutItem / BatchWriteItem). The control -tests at the bottom pin that boundary so the fix does not over-correct. +must keep returning ``ResourceNotFoundException`` for an absent table (the +key-schema-dependent checks, which need the table's key schema). + +The item-size limit (PutItem / BatchWriteItem) is *pre-existence*: a genuinely +oversized item (> the 400 KB / 409,600-byte limit) to an absent table returns +``ValidationException``. An earlier revision of this file claimed the opposite, +but its "big" item was ``"x" * 400001`` (~400,004 bytes) — actually *under* the +limit, so it was never oversized and returned ``ResourceNotFoundException`` for +the wrong reason. Verified against Amazon DynamoDB with a >400 KB item. All expected behaviour here was captured from Amazon DynamoDB via the AWS CLI (profile ``asomasun-admin``, us-east-1). @@ -172,29 +178,29 @@ def test_batch_get_item_duplicate_keys(self, client): _assert_validation(ei, "duplicates") -class TestValidationAfterExistence: - """Controls: checks that stay post-existence on Amazon DynamoDB. +class TestItemSizeBeforeExistence: + """Item-size limit is validated *before* the existence check. - These must keep returning ResourceNotFoundException for an absent table so - the precedence fix does not move item-content validation ahead of the - existence check. + Verified against Amazon DynamoDB: a genuinely oversized item (> the 400 KB / + 409,600-byte limit) to an absent table returns ``ValidationException``, not + ``ResourceNotFoundException``. (A sub-limit item to an absent table correctly + returns ``ResourceNotFoundException`` because it is not oversized.) """ - def _big_item(self) -> dict: - return {"a": {"S": "k"}, "b": {"S": "x" * 400001}} + def _oversized_item(self) -> dict: + # > 400 KB (409,600 bytes): a single ~410 KB attribute value. + return {"a": {"S": "k"}, "b": {"S": "x" * 410000}} - def test_put_item_too_big_is_resource_not_found(self, client): + def test_put_item_too_big_before_existence(self, client): with pytest.raises(ClientError) as ei: - client.put_item(TableName=ABSENT_TABLE, Item=self._big_item()) - code, _ = _error(ei) - assert code == "ResourceNotFoundException", f"got {code}" + client.put_item(TableName=ABSENT_TABLE, Item=self._oversized_item()) + _assert_validation(ei, "Item size has exceeded the maximum allowed size") - def test_batch_write_item_too_big_is_resource_not_found(self, client): + def test_batch_write_item_too_big_before_existence(self, client): with pytest.raises(ClientError) as ei: client.batch_write_item( RequestItems={ - ABSENT_TABLE: [{"PutRequest": {"Item": self._big_item()}}] + ABSENT_TABLE: [{"PutRequest": {"Item": self._oversized_item()}}] }, ) - code, _ = _error(ei) - assert code == "ResourceNotFoundException", f"got {code}" + _assert_validation(ei, "Item size has exceeded the maximum allowed size") From 3684bfcb447c3deee80e0ed7013e73d617664a23 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 16:46:19 +0000 Subject: [PATCH 02/10] test: poll for GSI eventual consistency in secondary-index pagination tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Secondary-index (GSI) reads are eventually consistent — an item written to the base table is not guaranteed to be visible through a GSI immediately, as ExtendDB applies gsi_propagation_delay_ms like real DynamoDB. Several GSI query/scan pagination and tiebreaker tests wrote items then paged the index with no wait, asserting the full count, so they only passed when the delay was zero and were racy otherwise. Add a bounded wait_for_gsi_items() helper (15s) to conftest and use it in the affected GSI query/scan pagination and tiebreaker tests so they poll until the writes have propagated before asserting; a genuine drop still fails within the bound rather than hanging. Also guard the zero-delay synchronous-GSI test: gsi_propagation_delay_ms is read by the data-plane write path at startup, so a runtime change only takes effect if the server was booted with delay=0. The test now probes actual behavior and skips (rather than falsely fails) when the running server is not in synchronous mode. Signed-off-by: Lee Hannigan --- tests/conftest.py | 28 ++++ tests/test_gsi_async.py | 32 +++++ tests/test_query_scan.py | 271 ++++++++++++++++++++++----------------- 3 files changed, 211 insertions(+), 120 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index c2a17182..2f3c00e2 100755 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -90,6 +90,34 @@ def wait_for_active(client, table_name: str, timeout: float = 120.0) -> None: return time.sleep(interval) raise TimeoutError(f"Table {table_name} did not become ACTIVE within {timeout}s") + + +def wait_for_gsi_items(paginate, expected: int, timeout: float = 15.0): + """Poll a paginated GSI query/scan until it yields at least ``expected`` items. + + GSIs are eventually consistent: an item written to the base table is not + guaranteed to be visible through a secondary index immediately — ExtendDB + applies the configured ``gsi_propagation_delay_ms`` (like real DynamoDB), so + a read-back through a GSI right after the write can legitimately be short. + Tests that write then page a GSI must poll rather than read once. + + ``paginate`` is a zero-arg callable that runs the *entire* pagination and + returns the collected list; it is retried until it returns at least + ``expected`` items or the timeout elapses. The last collected list is + returned so the caller's ordering/dedup assertions run on the converged + result. On timeout the (short) result is returned so the caller's + ``assert len(...) == expected`` fails — a genuine drop still surfaces as a + failure within the bound rather than hanging. + """ + interval = _poll_interval() + deadline = time.monotonic() + timeout + items = paginate() + while len(items) < expected and time.monotonic() < deadline: + time.sleep(interval) + items = paginate() + return items + + @pytest.fixture() def create_and_cleanup_table(dynamodb_client, unique_table_name): """Create a table and ensure it's deleted after the test (REQ-TEST-005).""" diff --git a/tests/test_gsi_async.py b/tests/test_gsi_async.py index 35abbc1c..4caaa9e2 100755 --- a/tests/test_gsi_async.py +++ b/tests/test_gsi_async.py @@ -235,6 +235,12 @@ def test_gsi_sync_path_with_zero_delay( Sets the system-wide delay to 0, writes an item, and asserts the GSI query returns the item immediately (single query, no polling loop). This validates the effective_delay==0 sync path. + + Note: gsi_propagation_delay_ms is read by the data-plane write path at + server startup, so setting it at runtime here only takes effect if the + server was already booted with delay=0 (as devtools/run-tests does). A + behavioral probe below confirms the server is actually in synchronous + mode and skips — rather than falsely fails — if it is not. """ table_name = gsi_table @@ -243,6 +249,32 @@ def test_gsi_sync_path_with_zero_delay( extenddb_settings_set("gsi_propagation_delay_ms", "0") try: + # Behavioral guard: throwaway write + immediate index read. If the + # running server is genuinely synchronous, this appears at once; + # otherwise the sync path is not exercisable here — skip cleanly. + probe_gsi = f"syncprobe-gsi-{uuid.uuid4().hex[:8]}" + dynamodb_client.put_item( + TableName=table_name, + Item={ + "pk": {"S": f"syncprobe-{uuid.uuid4().hex[:8]}"}, + "gsi_pk": {"S": probe_gsi}, + "gsi_sk": {"N": "1"}, + "data": {"S": "probe"}, + }, + ) + probe = dynamodb_client.query( + TableName=table_name, + IndexName="test-gsi", + KeyConditionExpression="gsi_pk = :pk", + ExpressionAttributeValues={":pk": {"S": probe_gsi}}, + ) + if probe["Count"] != 1: + pytest.skip( + "server is not running with gsi_propagation_delay_ms=0 in " + "effect (the setting is applied at startup); the synchronous " + "GSI path cannot be exercised in this configuration" + ) + pk = f"sync-{uuid.uuid4().hex[:8]}" gsi_pk = f"sync-gsi-{uuid.uuid4().hex[:8]}" diff --git a/tests/test_query_scan.py b/tests/test_query_scan.py index 4b436cfd..5b9a4dd1 100755 --- a/tests/test_query_scan.py +++ b/tests/test_query_scan.py @@ -14,7 +14,7 @@ import pytest from botocore.exceptions import ClientError -from conftest import wait_for_active, scoped_table +from conftest import wait_for_active, wait_for_gsi_items, scoped_table @pytest.fixture(scope="class") def query_table(dynamodb_client): """Create a hash+range (S,N) table with 10 items for query tests.""" @@ -1065,27 +1065,30 @@ class TestHashOnlyGSIPagination: def test_paginate_all_items_with_limit(self, dynamodb_client, gsi_hash_only_table): """Paginating through all items on a hash-only GSI returns all 10 items.""" - all_items = [] - exclusive_start_key = None - - while True: - kwargs = { - "TableName": gsi_hash_only_table, - "IndexName": "StatusGSI", - "KeyConditionExpression": "nodeStatus = :s", - "ExpressionAttributeValues": {":s": {"S": "ACTIVE"}}, - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_hash_only_table, + "IndexName": "StatusGSI", + "KeyConditionExpression": "nodeStatus = :s", + "ExpressionAttributeValues": {":s": {"S": "ACTIVE"}}, + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 10) assert len(all_items) == 10 ids = sorted(item["instanceId"]["S"] for item in all_items) expected = sorted(f"node-{i}" for i in range(1, 11)) @@ -1190,52 +1193,58 @@ class TestGSIOnHashOnlyBaseTable: def test_paginate_duplicate_gsi_sort_keys(self, dynamodb_client, gsi_on_hash_only_base_table): """All 7 items with same GSI sort key are returned through pagination.""" - all_items = [] - exclusive_start_key = None - - while True: - kwargs = { - "TableName": gsi_on_hash_only_base_table, - "IndexName": "CategoryPriorityGSI", - "KeyConditionExpression": "category = :c", - "ExpressionAttributeValues": {":c": {"S": "urgent"}}, - "Limit": 2, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_on_hash_only_base_table, + "IndexName": "CategoryPriorityGSI", + "KeyConditionExpression": "category = :c", + "ExpressionAttributeValues": {":c": {"S": "urgent"}}, + "Limit": 2, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 7) assert len(all_items) == 7 ids = sorted(item["itemId"]["S"] for item in all_items) assert ids == sorted(f"item-{i}" for i in range(1, 8)) def test_scan_pagination_on_gsi(self, dynamodb_client, gsi_on_hash_only_base_table): """Scan on GSI with Limit paginates correctly over hash-only base table.""" - all_items = [] - exclusive_start_key = None - - while True: - kwargs = { - "TableName": gsi_on_hash_only_base_table, - "IndexName": "CategoryPriorityGSI", - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_on_hash_only_base_table, + "IndexName": "CategoryPriorityGSI", + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.scan(**kwargs) - all_items.extend(resp["Items"]) + resp = dynamodb_client.scan(**kwargs) + all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 7) assert len(all_items) == 7 ids = sorted(item["itemId"]["S"] for item in all_items) assert ids == sorted(f"item-{i}" for i in range(1, 8)) @@ -1246,7 +1255,7 @@ def test_repeated_pagination_consistent(self, dynamodb_client, gsi_on_hash_only_ The first query populates the internal base_key_cache. Subsequent queries hit the cache. Any cache corruption would cause pagination to break. """ - for run in range(5): + def _collect(): all_items = [] exclusive_start_key = None while True: @@ -1264,7 +1273,13 @@ def test_repeated_pagination_consistent(self, dynamodb_client, gsi_on_hash_only_ if "LastEvaluatedKey" not in resp: break exclusive_start_key = resp["LastEvaluatedKey"] - assert len(all_items) == 7, ( + return all_items + + # First run polls for GSI convergence; subsequent runs must stay consistent. + for run in range(5): + expected = 7 + all_items = wait_for_gsi_items(_collect, expected) if run == 0 else _collect() + assert len(all_items) == expected, ( f"Run {run+1}: expected 7 items but got {len(all_items)}" ) @@ -1278,25 +1293,29 @@ def test_paginate_reverse_gsi_with_tied_sort_keys( If the tie-breaker accidentally followed ScanIndexForward, reverse pagination would skip items or loop. """ - all_items = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": gsi_on_hash_only_base_table, - "IndexName": "CategoryPriorityGSI", - "KeyConditionExpression": "category = :c", - "ExpressionAttributeValues": {":c": {"S": "urgent"}}, - "ScanIndexForward": False, - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_on_hash_only_base_table, + "IndexName": "CategoryPriorityGSI", + "KeyConditionExpression": "category = :c", + "ExpressionAttributeValues": {":c": {"S": "urgent"}}, + "ScanIndexForward": False, + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 7) assert len(all_items) == 7, ( f"Reverse GSI pagination: expected 7 items but got {len(all_items)}" ) @@ -1382,24 +1401,28 @@ def test_index_pagination_uses_base_key_schema_for_tiebreaker( self, dynamodb_client, base_key_schema_table ): """GSI query: base_key_schema differs from key_schema, used for tie-breaking.""" - all_items = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": base_key_schema_table, - "IndexName": "TestGSI", - "KeyConditionExpression": "gsi_pk = :gpk", - "ExpressionAttributeValues": {":gpk": {"S": "shared"}}, - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": base_key_schema_table, + "IndexName": "TestGSI", + "KeyConditionExpression": "gsi_pk = :gpk", + "ExpressionAttributeValues": {":gpk": {"S": "shared"}}, + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 12) assert len(all_items) == 12 # No duplicates across pages item_keys = [(i["pk"]["S"], i["sk"]["N"]) for i in all_items] @@ -1409,22 +1432,26 @@ def test_index_scan_pagination_uses_base_key_schema( self, dynamodb_client, base_key_schema_table ): """GSI scan: base_key_schema provides base PK/SK for compound pagination.""" - all_items = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": base_key_schema_table, - "IndexName": "TestGSI", - "Limit": 4, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.scan(**kwargs) - all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": base_key_schema_table, + "IndexName": "TestGSI", + "Limit": 4, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.scan(**kwargs) + all_items.extend(resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 12) assert len(all_items) == 12 item_keys = [(i["pk"]["S"], i["sk"]["N"]) for i in all_items] assert len(item_keys) == len(set(item_keys)) @@ -1477,22 +1504,26 @@ class TestHashOnlyGsiCompositeBaseScan: def test_scan_paginate_returns_all( self, dynamodb_client, hash_only_gsi_on_composite_base ): - all_ts = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": hash_only_gsi_on_composite_base, - "IndexName": "StatusGSI", - "Limit": 2, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.scan(**kwargs) - all_ts.extend(item["ts"]["N"] for item in resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_ts = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": hash_only_gsi_on_composite_base, + "IndexName": "StatusGSI", + "Limit": 2, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.scan(**kwargs) + all_ts.extend(item["ts"]["N"] for item in resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_ts + # GSI is eventually consistent — poll until all writes have propagated. + all_ts = wait_for_gsi_items(_collect, 7) assert len(all_ts) == 7, f"expected 7 items, got {len(all_ts)}" assert sorted(all_ts, key=int) == [str(i) for i in range(1, 8)] From eb2fb9eab0674879b745eab26baa78ca7fa884c7 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 17:08:19 +0000 Subject: [PATCH 03/10] test: cover transactional update to empty string/binary values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TransactWriteItems with an Update that sets non-key attributes to an empty string or empty binary value must be accepted — empty values are valid on non-key attributes, matching real DynamoDB — and must not fault. Existing coverage exercised the transactional Put path and the empty-key rejection, but not the transactional Update-to-empty-value path. Add a Rust integration test asserting a transact Update that sets an empty string and an empty binary on non-key attributes succeeds and reads the empty values back. Signed-off-by: Lee Hannigan --- tests/rust/src/empty_values.rs | 57 +++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/tests/rust/src/empty_values.rs b/tests/rust/src/empty_values.rs index 27f47902..54d18675 100755 --- a/tests/rust/src/empty_values.rs +++ b/tests/rust/src/empty_values.rs @@ -7,7 +7,7 @@ use crate::test_base::*; use aws_sdk_dynamodb::types::{ AttributeDefinition, AttributeValue, BillingMode, KeySchemaElement, KeyType, Put, - ScalarAttributeType, TransactWriteItem, WriteRequest, + ScalarAttributeType, TransactWriteItem, Update, WriteRequest, }; use std::collections::HashMap; @@ -255,6 +255,61 @@ async fn transact_write_with_empty_string() { assert_eq!(resp.item().unwrap().get("val").unwrap(), &s("")); } +/// Regression: a TransactWriteItems `Update` that sets non-key attributes to an +/// empty string and empty binary must be accepted — empty values are valid on +/// non-key attributes on real DynamoDB. Guards the transactional update path +/// against faulting (previously a server-side error on an empty-value SET). +#[tokio::test] +async fn transact_update_with_empty_string_and_binary() { + let c = client(); + let name = create_simple_table(c).await; + let mut item = HashMap::new(); + item.insert("pk".into(), s("k1")); + item.insert("val".into(), s("notempty")); + c.put_item() + .table_name(&name) + .set_item(Some(item)) + .send() + .await + .unwrap(); + c.transact_write_items() + .transact_items( + TransactWriteItem::builder() + .update( + Update::builder() + .table_name(&name) + .key("pk", s("k1")) + .update_expression("SET s_attr = :s, b_attr = :b") + .expression_attribute_values(":s", s("")) + .expression_attribute_values( + ":b", + AttributeValue::B(aws_smithy_types::Blob::new(Vec::::new())), + ) + .build() + .unwrap(), + ) + .build(), + ) + .send() + .await + .unwrap(); + let resp = c + .get_item() + .table_name(&name) + .key("pk", s("k1")) + .consistent_read(true) + .send() + .await + .unwrap(); + let got = resp.item().unwrap(); + assert_eq!(got.get("s_attr").unwrap(), &s("")); + if let AttributeValue::B(blob) = got.get("b_attr").unwrap() { + assert!(blob.as_ref().is_empty()); + } else { + panic!("Expected binary type for b_attr"); + } +} + #[tokio::test] async fn nested_empty_list() { let c = client(); From eeb90a0833a0a670ea7efd72828ad56bffebdb0f Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 17:29:16 +0000 Subject: [PATCH 04/10] fix: report empty binary secondary-index key with the correct type in message An empty binary value on a secondary-index key attribute was rejected with a message that hardcoded "empty string value" regardless of the attribute type, so an empty binary index key was misreported as a string. Real DynamoDB reports "empty binary value" for a binary key. Make the secondary-index-key empty-value message type-aware (string vs binary) for both the item and update-expression contexts. Add unit coverage for the binary message in both contexts and a Rust integration test asserting an empty binary GSI key is rejected with the type-correct message. Signed-off-by: Lee Hannigan --- crates/core/src/validation/mod.rs | 53 +++++++++++++--- tests/rust/src/index_key_validation.rs | 83 ++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 7 deletions(-) diff --git a/crates/core/src/validation/mod.rs b/crates/core/src/validation/mod.rs index c186071a..d9ce3be3 100755 --- a/crates/core/src/validation/mod.rs +++ b/crates/core/src/validation/mod.rs @@ -961,19 +961,22 @@ pub fn validate_index_key_not_empty( let Some(value) = item.get(&ks.attribute_name) else { continue; }; - let empty = matches!(value, AttributeValue::S(s) if s.is_empty()) - || matches!(value, AttributeValue::B(b) if b.is_empty()); - if empty { + let empty_kind = match value { + AttributeValue::S(s) if s.is_empty() => Some("string"), + AttributeValue::B(b) if b.is_empty() => Some("binary"), + _ => None, + }; + if let Some(kind) = empty_kind { let msg = match ctx { SecondaryIndexEmptyContext::Item => format!( "One or more parameter values are not valid. A value specified for a secondary index key is not supported. \ - The AttributeValue for a key attribute cannot contain an empty string value. IndexName: {}, IndexKey: {}", + The AttributeValue for a key attribute cannot contain an empty {kind} value. IndexName: {}, IndexKey: {}", idx.index_name, ks.attribute_name ), - SecondaryIndexEmptyContext::UpdateExpression => + SecondaryIndexEmptyContext::UpdateExpression => format!( "One or more parameter values are not valid. The update expression attempted to update a secondary index key to a value that is not supported. \ - The AttributeValue for a key attribute cannot contain an empty string value." - .to_owned(), + The AttributeValue for a key attribute cannot contain an empty {kind} value." + ), }; return Err(DynamoDbError::ValidationException(msg)); } @@ -2124,6 +2127,42 @@ mod tests { ); } + #[test] + fn index_key_empty_binary_messages_by_context() { + // An empty BINARY value on a secondary-index key must be reported as an + // "empty binary value" (matching real DynamoDB), not "empty string value". + let owned = [idx("gsi1", "lsi1sk")]; + let refs: Vec> = owned + .iter() + .map(|(n, ks)| IndexKeyRef { + index_name: n, + key_schema: ks, + }) + .collect(); + let mut item = Item::new(); + item.insert("lsi1sk".to_owned(), AttributeValue::B(Vec::new())); + + let put_err = validate_index_key_not_empty(&item, &refs, SecondaryIndexEmptyContext::Item) + .unwrap_err(); + assert_eq!( + put_err.to_string(), + "One or more parameter values are not valid. A value specified for a secondary index key is not supported. \ + The AttributeValue for a key attribute cannot contain an empty binary value. IndexName: gsi1, IndexKey: lsi1sk" + ); + + let upd_err = validate_index_key_not_empty( + &item, + &refs, + SecondaryIndexEmptyContext::UpdateExpression, + ) + .unwrap_err(); + assert_eq!( + upd_err.to_string(), + "One or more parameter values are not valid. The update expression attempted to update a secondary index key to a value that is not supported. \ + The AttributeValue for a key attribute cannot contain an empty binary value." + ); + } + #[test] fn valid_index_key_passes() { let owned = [idx("gsi1", "lsi1sk")]; diff --git a/tests/rust/src/index_key_validation.rs b/tests/rust/src/index_key_validation.rs index 7a239112..f0b4e25e 100644 --- a/tests/rust/src/index_key_validation.rs +++ b/tests/rust/src/index_key_validation.rs @@ -78,3 +78,86 @@ async fn put_item_accepts_valid_index_key() { .await .expect("valid index key must be accepted"); } + +#[tokio::test] +async fn put_item_rejects_empty_binary_index_key_reports_binary() { + use aws_sdk_dynamodb::types::{ + AttributeDefinition, AttributeValue, BillingMode, GlobalSecondaryIndex, KeySchemaElement, + KeyType, Projection, ProjectionType, ScalarAttributeType, + }; + let c = client(); + let name = format!("BinGsiEmpty{}", ts()); + c.create_table() + .table_name(&name) + .key_schema( + KeySchemaElement::builder() + .attribute_name("pk") + .key_type(KeyType::Hash) + .build() + .unwrap(), + ) + .attribute_definitions( + AttributeDefinition::builder() + .attribute_name("pk") + .attribute_type(ScalarAttributeType::S) + .build() + .unwrap(), + ) + .attribute_definitions( + AttributeDefinition::builder() + .attribute_name("gb") + .attribute_type(ScalarAttributeType::B) + .build() + .unwrap(), + ) + .global_secondary_indexes( + GlobalSecondaryIndex::builder() + .index_name("gsib") + .key_schema( + KeySchemaElement::builder() + .attribute_name("gb") + .key_type(KeyType::Hash) + .build() + .unwrap(), + ) + .projection( + Projection::builder() + .projection_type(ProjectionType::All) + .build(), + ) + .build() + .unwrap(), + ) + .billing_mode(BillingMode::PayPerRequest) + .send() + .await + .unwrap(); + wait_for_active(c, &name).await; + + // Empty BINARY value on the GSI key must be reported as an "empty binary + // value" (matching real DynamoDB), not "empty string value". + let mut item: HashMap = HashMap::new(); + item.insert("pk".into(), s("a")); + item.insert( + "gb".into(), + AttributeValue::B(aws_smithy_types::Blob::new(Vec::::new())), + ); + let err = c + .put_item() + .table_name(&name) + .set_item(Some(item)) + .send() + .await + .expect_err("empty binary index key must be rejected"); + assert_eq!( + err_code(&err), + Some("ValidationException"), + "{}", + err_msg(&err) + ); + let m = err_msg(&err); + assert!( + m.contains("empty binary value"), + "expected type-correct 'empty binary value' message, got: {m}" + ); +} From 778808b3b18e339eef429a9e6625cd23e1c46674 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 18:27:43 +0000 Subject: [PATCH 05/10] fix: return ValidationException for invalid ExpressionAttributeValues Malformed values in ExpressionAttributeValues were mishandled: a non-true NULL was rejected as a SerializationException instead of a ValidationException, and empty/duplicate set values were rejected without the field/key context that DynamoDB includes. A malformed value could also mask a malformed key in the same map. - Validate all ExpressionAttributeNames/Values keys before parsing any value, so a syntactically invalid key is reported ahead of value-content errors. - Wrap semantic value-validation errors as " contains invalid value: for key ", matching DynamoDB. - Treat a non-true NULL AttributeValue as a validation error. - Include the "of type BS" qualifier in the binary-set duplicate message. Adds unit coverage for the non-true NULL, empty/duplicate set, and key-before-value precedence cases. Signed-off-by: Lee Hannigan --- crates/core/src/serde_helpers.rs | 107 ++++++++++++++++++++++- crates/core/src/types/attribute_value.rs | 7 +- 2 files changed, 106 insertions(+), 8 deletions(-) diff --git a/crates/core/src/serde_helpers.rs b/crates/core/src/serde_helpers.rs index 3a4ffb3c..f686b3a1 100644 --- a/crates/core/src/serde_helpers.rs +++ b/crates/core/src/serde_helpers.rs @@ -75,11 +75,28 @@ where return Err(de::Error::custom(format!("{field_name} must not be empty"))); } let mut out = HashMap::with_capacity(raw.len()); - for (key, value) in raw { - validate_placeholder_key(&key, prefix, field_name, include_size_in_too_long) + // Pass 1: validate every key first, so a malformed key is reported before + // any value error (DynamoDB validates key syntax ahead of value contents). + for key in raw.keys() { + validate_placeholder_key(key, prefix, field_name, include_size_in_too_long) .map_err(de::Error::custom)?; + } + // Pass 2: per-value check and conversion. + for (key, value) in raw { check_value(&key, &value).map_err(de::Error::custom)?; - let converted = convert(value).map_err(de::Error::custom)?; + let converted = convert(value).map_err(|e| { + let msg = e.to_string(); + // Semantic value-validation errors are wrapped with the field name + // and the offending key (DynamoDB parity). Wire/type errors (wrong + // JSON shape for a datatype) pass through as-is. + if msg.starts_with("One or more parameter values were invalid:") { + de::Error::custom(format!( + "{field_name} contains invalid value: {msg} for key {key}" + )) + } else { + de::Error::custom(msg) + } + })?; out.insert(key, converted); } Ok(Some(out)) @@ -264,6 +281,90 @@ mod tests { ); } + #[test] + fn values_null_non_boolean_is_validation_error() { + // {"NULL":"no"} is a validation error on real DynamoDB, not a parse + // (Serialization) error. Must be prefixed and name its key. + let msg = values_err(r#"{"values":{":b":{"NULL":"no"}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Null attribute value types must have the value of \ + true for key :b" + ), + "{msg}" + ); + } + + #[test] + fn values_null_false_is_validation_error() { + let msg = values_err(r#"{"values":{":b":{"NULL":false}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Null attribute value types must have the value of \ + true for key :b" + ), + "{msg}" + ); + } + + #[test] + fn values_null_true_accepted() { + let parsed: TestValues = + serde_json::from_str(r#"{"values":{":b":{"NULL":true}}}"#).unwrap(); + assert!(parsed.values.is_some()); + } + + #[test] + fn values_empty_set_wrapped_with_key() { + for (av, needle) in [ + (r#"{"SS":[]}"#, "An string set may not be empty"), + (r#"{"NS":[]}"#, "An number set may not be empty"), + (r#"{"BS":[]}"#, "Binary sets should not be empty"), + ] { + let msg = values_err(&format!(r#"{{"values":{{":b":{av}}}}}"#)); + let expected = format!( + "ExpressionAttributeValues contains invalid value: One or more \ + parameter values were invalid: {needle} for key :b" + ); + assert!(msg.contains(&expected), "av={av} got: {msg}"); + } + } + + #[test] + fn values_duplicate_set_wrapped_with_key() { + let ss = values_err(r#"{"values":{":b":{"SS":["a","a"]}}}"#); + assert!( + ss.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Input collection [a, a] contains duplicates. for key :b" + ), + "{ss}" + ); + // Binary duplicates carry the "of type BS" qualifier (DynamoDB parity). + let bs = values_err(r#"{"values":{":b":{"BS":["Yg==","Yg=="]}}}"#); + assert!( + bs.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Input collection [Yg==, Yg==]of type BS contains \ + duplicates. for key :b" + ), + "{bs}" + ); + } + + #[test] + fn values_invalid_key_reported_before_value_error() { + // Map has a malformed value (:b -> unknown type) AND a malformed key (b + // without the ':' prefix). The key error must win, matching DynamoDB. + let msg = values_err(r#"{"values":{":b":{"a":""},"b":{"S":"a"}}}"#); + assert!( + msg.contains(r#"ExpressionAttributeValues contains invalid key: Syntax error; key: "b""#), + "{msg}" + ); + } + #[test] fn values_key_too_long_omits_size() { let key = format!(":{}", "a".repeat(255)); // 256 bytes including ':' diff --git a/crates/core/src/types/attribute_value.rs b/crates/core/src/types/attribute_value.rs index 822729e2..7d3f5296 100755 --- a/crates/core/src/types/attribute_value.rs +++ b/crates/core/src/types/attribute_value.rs @@ -210,7 +210,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { let values: Vec<&str> = arr.iter().filter_map(|v| v.as_str()).collect(); let repr = values.join(", "); return Err(de::Error::custom(format!( - "One or more parameter values were invalid: Input collection [{repr}] contains duplicates." + "One or more parameter values were invalid: Input collection [{repr}]of type BS contains duplicates." ))); } Ok(AttributeValue::BS(set)) @@ -222,10 +222,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { Ok(AttributeValue::Bool(b)) } "NULL" => { - let n = value - .as_bool() - .ok_or_else(|| de::Error::custom("NULL value must be a boolean"))?; - if !n { + if value.as_bool() != Some(true) { return Err(de::Error::custom( "One or more parameter values were invalid: Null attribute value types must have the value of true", )); From 54f26e7f02b007a45507d72797ecb103ac5fd949 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 18:36:16 +0000 Subject: [PATCH 06/10] fix: classify malformed PutItem expression errors as ValidationException PutItem deserialized its request with a private, stale copy of the error classifier that omitted the field-prefixed "contains invalid key/value" validation messages. A malformed ExpressionAttributeValues key or value on PutItem therefore returned SerializationException where DynamoDB returns ValidationException. Route PutItem through the shared deserialize_error classifier already used by the other verbs so classification is consistent. Signed-off-by: Lee Hannigan --- crates/engine/src/put_item.rs | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/crates/engine/src/put_item.rs b/crates/engine/src/put_item.rs index d384e842..25c24683 100755 --- a/crates/engine/src/put_item.rs +++ b/crates/engine/src/put_item.rs @@ -65,25 +65,7 @@ pub async fn handle_put_item( ], )?; - let input: PutItemInput = serde_json::from_value(body).map_err(|e| { - let msg = e.to_string(); - if msg.contains("parameter values were invalid") - || msg.contains("may not be empty") - || msg.contains("contains duplicates") - || msg.contains("Null attribute value") - || msg.contains("validation error detected") - || msg.contains("must not be empty") - || msg.contains("Syntax error; key") - || msg.contains("AttributeValue is empty") - || msg.contains("AttributeValue has more than one datatypes set") - { - DynamoDbError::ValidationException(msg) - } else { - DynamoDbError::SerializationException(format!( - "Start of structure or map found where not expected: {e}" - )) - } - })?; + let input: PutItemInput = serde_json::from_value(body).map_err(crate::deserialize_error)?; // Reject mixing legacy and expression parameters, then EAN/EAV supplied // without a referencing expression. From 493888d27db8427bab50e0d6ebf95205d832266e Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 18:52:37 +0000 Subject: [PATCH 07/10] fix: wrap unsupported/multiple-datatype ExpressionAttributeValues errors An unrecognized AttributeValue datatype tag was rejected as a SerializationException ("unknown type descriptor"), and a multiple-datatype AttributeValue was rejected without the field/key context DynamoDB includes. - Report an unrecognized datatype tag as "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes" (no supported datatype is present), matching DynamoDB. - Wrap unsupported-datatype and multiple-datatype value errors as " contains invalid value: for key ". Adds unit coverage for both wrapped cases. Signed-off-by: Lee Hannigan --- crates/core/src/serde_helpers.rs | 33 +++++++++++++++++++++++- crates/core/src/types/attribute_value.rs | 15 ++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/crates/core/src/serde_helpers.rs b/crates/core/src/serde_helpers.rs index f686b3a1..dbd273fa 100644 --- a/crates/core/src/serde_helpers.rs +++ b/crates/core/src/serde_helpers.rs @@ -89,7 +89,11 @@ where // Semantic value-validation errors are wrapped with the field name // and the offending key (DynamoDB parity). Wire/type errors (wrong // JSON shape for a datatype) pass through as-is. - if msg.starts_with("One or more parameter values were invalid:") { + let is_value_validation = msg + .starts_with("One or more parameter values were invalid:") + || msg.contains("Supplied AttributeValue is empty") + || msg.contains("Supplied AttributeValue has more than one datatypes set"); + if is_value_validation { de::Error::custom(format!( "{field_name} contains invalid value: {msg} for key {key}" )) @@ -365,6 +369,33 @@ mod tests { ); } + #[test] + fn values_unsupported_datatype_wrapped_with_key() { + // An unrecognized datatype tag is reported as an empty AttributeValue, + // wrapped with the field name and key. + let msg = values_err(r#"{"values":{":b":{"a":""}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: Supplied AttributeValue \ + is empty, must contain exactly one of the supported datatypes for key :b" + ), + "{msg}" + ); + } + + #[test] + fn values_multiple_datatypes_wrapped_with_key() { + let msg = values_err(r#"{"values":{":b":{"S":"a","N":"1"}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: Supplied AttributeValue \ + has more than one datatypes set, must contain exactly one of the supported \ + datatypes for key :b" + ), + "{msg}" + ); + } + #[test] fn values_key_too_long_omits_size() { let key = format!(":{}", "a".repeat(255)); // 256 bytes including ':' diff --git a/crates/core/src/types/attribute_value.rs b/crates/core/src/types/attribute_value.rs index 7d3f5296..52fddfd8 100755 --- a/crates/core/src/types/attribute_value.rs +++ b/crates/core/src/types/attribute_value.rs @@ -253,9 +253,9 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { .collect::>()?; Ok(AttributeValue::M(map)) } - other => Err(de::Error::custom(format!( - "unknown AttributeValue type descriptor: {other}" - ))), + _other => Err(de::Error::custom( + "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes", + )), } } } @@ -418,9 +418,16 @@ mod tests { #[test] fn unknown_type_descriptor_rejected() { + // An unrecognized type descriptor means no supported datatype is + // present; DynamoDB reports this as an empty AttributeValue. let json = r#"{"X":"hello"}"#; let err = serde_json::from_str::(json).unwrap_err(); - assert!(err.to_string().contains("unknown")); + assert!( + err.to_string().contains( + "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes" + ), + "got: {err}" + ); } #[test] From 310d6f587658bc2924db0a424e64858fb248c286 Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 3 Jul 2026 19:18:54 +0000 Subject: [PATCH 08/10] fix: validate ExpressionAttributeValues numbers with DynamoDB semantics Invalid numbers in ExpressionAttributeValues (and item attributes) were accepted at deserialization and never rejected, so a malformed N/NS returned the wrong error or none. - Collect all AttributeValue map entries and validate N/NS number content before the "more than one datatype" check, so an invalid number is reported even alongside another datatype (matching DynamoDB precedence). - Report a number-set duplicate as "Input collection contains duplicates". - Extend the value-error wrapper and the shared deserialization-error classifier to cover the numeric-conversion, significant-digits, overflow and underflow messages, so they surface as ValidationException wrapped with the field name and key. Adds unit coverage for the empty/non-numeric/overflow/underflow/duplicate and multiple-datatype-precedence cases. Signed-off-by: Lee Hannigan --- crates/core/src/serde_helpers.rs | 57 +++++++++++++++-- crates/core/src/types/attribute_value.rs | 81 ++++++++++++++++-------- crates/engine/src/lib.rs | 4 ++ 3 files changed, 111 insertions(+), 31 deletions(-) diff --git a/crates/core/src/serde_helpers.rs b/crates/core/src/serde_helpers.rs index dbd273fa..29cb1fe5 100644 --- a/crates/core/src/serde_helpers.rs +++ b/crates/core/src/serde_helpers.rs @@ -89,10 +89,14 @@ where // Semantic value-validation errors are wrapped with the field name // and the offending key (DynamoDB parity). Wire/type errors (wrong // JSON shape for a datatype) pass through as-is. - let is_value_validation = msg - .starts_with("One or more parameter values were invalid:") + let is_value_validation = msg.starts_with("One or more parameter values were invalid:") || msg.contains("Supplied AttributeValue is empty") - || msg.contains("Supplied AttributeValue has more than one datatypes set"); + || msg.contains("Supplied AttributeValue has more than one datatypes set") + || msg.contains("cannot be converted to a numeric value") + || msg.contains("significant digits in a Number") + || msg.starts_with("Number overflow") + || msg.starts_with("Number underflow") + || msg.contains("Input collection contains duplicates"); if is_value_validation { de::Error::custom(format!( "{field_name} contains invalid value: {msg} for key {key}" @@ -364,7 +368,9 @@ mod tests { // without the ':' prefix). The key error must win, matching DynamoDB. let msg = values_err(r#"{"values":{":b":{"a":""},"b":{"S":"a"}}}"#); assert!( - msg.contains(r#"ExpressionAttributeValues contains invalid key: Syntax error; key: "b""#), + msg.contains( + r#"ExpressionAttributeValues contains invalid key: Syntax error; key: "b""# + ), "{msg}" ); } @@ -396,6 +402,49 @@ mod tests { ); } + #[test] + fn values_invalid_number_wrapped_with_key() { + // Empty, non-numeric, overflow, underflow, 38-digit, and NS-duplicate + // all wrap with the field name and key, matching real DynamoDB. + for (av, needle) in [ + ( + r#"{"N":""}"#, + "The parameter cannot be converted to a numeric value", + ), + ( + r#"{"N":"b"}"#, + "The parameter cannot be converted to a numeric value: b", + ), + ( + r#"{"S":"a","N":""}"#, + "The parameter cannot be converted to a numeric value", + ), + ( + r#"{"NS":["1","b"]}"#, + "The parameter cannot be converted to a numeric value: b", + ), + ( + r#"{"NS":["1","1"]}"#, + "Input collection contains duplicates", + ), + ( + r#"{"N":"1e126"}"#, + "Number overflow. Attempting to store a number with magnitude larger", + ), + ( + r#"{"N":"1e-131"}"#, + "Number underflow. Attempting to store a number with magnitude smaller", + ), + ] { + let msg = values_err(&format!(r#"{{"values":{{":b":{av}}}}}"#)); + let expected = format!("ExpressionAttributeValues contains invalid value: {needle}"); + assert!( + msg.contains(&expected) && msg.contains("for key :b"), + "av={av} got: {msg}" + ); + } + } + #[test] fn values_key_too_long_omits_size() { let key = format!(":{}", "a".repeat(255)); // 256 bytes including ':' diff --git a/crates/core/src/types/attribute_value.rs b/crates/core/src/types/attribute_value.rs index 52fddfd8..7dfe0165 100755 --- a/crates/core/src/types/attribute_value.rs +++ b/crates/core/src/types/attribute_value.rs @@ -76,21 +76,50 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { } fn visit_map>(self, mut map: A) -> Result { - let (key, value): (String, serde_json::Value) = map - .next_entry()? - .ok_or_else(|| { - de::Error::custom( - "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes", - ) - })?; + let mut entries: Vec<(String, serde_json::Value)> = Vec::new(); + while let Some(entry) = map.next_entry::()? { + entries.push(entry); + } + if entries.is_empty() { + return Err(de::Error::custom( + "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes", + )); + } + + // DynamoDB validates the content of a number field before it reports the + // "more than one datatype" error, so an invalid N/NS value is surfaced + // even when several type descriptors are present. + for (k, v) in &entries { + match k.as_str() { + "N" => { + if let Some(s) = v.as_str() { + crate::validation::number::validate_and_normalize_number(s) + .map_err(|e| de::Error::custom(e.message()))?; + } + } + "NS" => { + if let Some(arr) = v.as_array() { + for elem in arr { + if let Some(s) = elem.as_str() { + crate::validation::number::validate_and_normalize_number(s) + .map_err(|e| de::Error::custom(e.message()))?; + } + } + } + } + _ => {} + } + } // REQ-TYPE-001: reject if multiple keys - if map.next_key::()?.is_some() { + if entries.len() > 1 { return Err(de::Error::custom( "Supplied AttributeValue has more than one datatypes set, must contain exactly one of the supported datatypes", )); } + let (key, value) = entries.into_iter().next().expect("entries is non-empty"); + match key.as_str() { "S" => { let s = value @@ -174,11 +203,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { }) .collect::>()?; if set.len() != arr.len() { - let values: Vec<&str> = arr.iter().filter_map(|v| v.as_str()).collect(); - let repr = values.join(", "); - return Err(de::Error::custom(format!( - "One or more parameter values were invalid: Input collection [{repr}] contains duplicates." - ))); + return Err(de::Error::custom("Input collection contains duplicates")); } Ok(AttributeValue::NS(set)) } @@ -432,25 +457,27 @@ mod tests { #[test] fn invalid_number_accepted_at_deserialization() { - // Invalid numbers are accepted by the deserializer (stored raw) - // and rejected later by the validation layer as ValidationException. - let json = r#"{"N":"abc"}"#; - let val: AttributeValue = serde_json::from_str(json).unwrap(); - assert_eq!(val, AttributeValue::N("abc".to_owned())); + // Invalid numbers are rejected at deserialization with DynamoDB's + // number-validation messages. + let err = serde_json::from_str::(r#"{"N":"abc"}"#).unwrap_err(); + assert!( + err.to_string() + .contains("The parameter cannot be converted to a numeric value: abc"), + "got: {err}" + ); - let json = r#"{"N":"1E999"}"#; - let val: AttributeValue = serde_json::from_str(json).unwrap(); - assert_eq!(val, AttributeValue::N("1E999".to_owned())); + let err = serde_json::from_str::(r#"{"N":"1E999"}"#).unwrap_err(); + assert!(err.to_string().contains("Number overflow"), "got: {err}"); } #[test] fn invalid_number_in_ns_accepted() { - let json = r#"{"NS":["1","abc"]}"#; - let val: AttributeValue = serde_json::from_str(json).unwrap(); - match val { - AttributeValue::NS(set) => assert!(set.contains("abc")), - _ => panic!("expected NS"), - } + let err = serde_json::from_str::(r#"{"NS":["1","abc"]}"#).unwrap_err(); + assert!( + err.to_string() + .contains("The parameter cannot be converted to a numeric value: abc"), + "got: {err}" + ); } #[test] diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f85e4785..806429d0 100755 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -158,6 +158,10 @@ pub(crate) fn deserialize_error(e: serde_json::Error) -> DynamoDbError { || msg.contains("AttributeValue is empty") || msg.contains("AttributeValue has more than one datatypes set") || msg.contains("parameter values were invalid") + || msg.contains("cannot be converted to a numeric value") + || msg.contains("significant digits in a Number") + || msg.contains("Number overflow") + || msg.contains("Number underflow") { DynamoDbError::ValidationException(msg) } else { From 1dced88b4ec869a782e49dab25e361893dbcdf3d Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 17 Jul 2026 16:00:45 +0000 Subject: [PATCH 09/10] docs(tests): clarify wait_for_gsi_items docstring (review nits) - Drop the '(like real DynamoDB)' parenthetical: DynamoDB has no configurable GSI propagation delay; gsi_propagation_delay_ms is ExtendDB-specific. - Reword 'can legitimately be short' -> 'can legitimately return fewer items than expected' for clarity. Signed-off-by: Lee Hannigan --- tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 2f3c00e2..10200a68 100755 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -97,9 +97,9 @@ def wait_for_gsi_items(paginate, expected: int, timeout: float = 15.0): GSIs are eventually consistent: an item written to the base table is not guaranteed to be visible through a secondary index immediately — ExtendDB - applies the configured ``gsi_propagation_delay_ms`` (like real DynamoDB), so - a read-back through a GSI right after the write can legitimately be short. - Tests that write then page a GSI must poll rather than read once. + applies the configured ``gsi_propagation_delay_ms``, so + a read-back through a GSI right after the write can legitimately return fewer + items than expected. Tests that write then page a GSI must poll rather than read once. ``paginate`` is a zero-arg callable that runs the *entire* pagination and returns the collected list; it is retried until it returns at least From 3aa3bcec7e263536a29c3bd43a910df5322da52d Mon Sep 17 00:00:00 2001 From: Lee Hannigan Date: Fri, 17 Jul 2026 16:04:56 +0000 Subject: [PATCH 10/10] docs(tests): drop weasel word in wait_for_gsi_items docstring Signed-off-by: Lee Hannigan --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 10200a68..66c5397c 100755 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -98,7 +98,7 @@ def wait_for_gsi_items(paginate, expected: int, timeout: float = 15.0): GSIs are eventually consistent: an item written to the base table is not guaranteed to be visible through a secondary index immediately — ExtendDB applies the configured ``gsi_propagation_delay_ms``, so - a read-back through a GSI right after the write can legitimately return fewer + a read-back through a GSI right after the write can return fewer items than expected. Tests that write then page a GSI must poll rather than read once. ``paginate`` is a zero-arg callable that runs the *entire* pagination and