Repository navigation
fix(scan): check the numbers in ExpressionAttributeValues as the service does - #562
Open
LeeroyHannigan wants to merge 3 commits into
Open
LeeroyHannigan wants to merge 3 commits into
LeeroyHannigan wants to merge 3 commits into
Conversation
…time
Scan did not check that the N values in ExpressionAttributeValues are
numbers, so a filter on {"N": "bob"}, a nested one in an L, M or NS, an
overflow, an underflow, a 39 digit value or an empty N answered 200 with
Count 0. The service refuses each with the same ValidationException text
Query uses, whether or not an expression reads the value.
Move the early Scan checks to a request-time function on the raw body,
in the order measured on 2026-10-25: the Limit, Segment and TotalSegments
ranges (one aggregated message, "Value at 'Limit'" with no value echoed),
the expression and non-expression parameter mix, the ReturnConsumedCapacity
enum, every number in ExpressionAttributeValues, then the Select enum.
Both enums invalid now answers the ReturnConsumedCapacity error alone, as
the service does; the earlier aggregation of the two no longer matched it.
The Segment pairing checks stay in the handler after the table lookup.
The two Scan tests that asserted the old segment range text now assert the
measured text.
…tion Every N value shape the service refuses on Scan (a word, a nested L, M or NS member, overflow, underflow, 39 digits, empty, a leading space), read by the filter or by nothing, and the order against every other early check: the Limit, Segment and TotalSegments ranges, the parameter mix and the ReturnConsumedCapacity enum ahead of it; the Select enum, the Segment pairing, the table lookup, IndexName, ConsistentRead, Select with ProjectionExpression, ExclusiveStartKey, undefined placeholder and filter syntax checks behind it. Expected texts measured on 2026-10-25.
… accept Unicode decimal digits The range checks on Limit, Segment and TotalSegments reported every fault in one aggregated message with a capitalised field name and no value for all three fields. The service uses that form for a Limit inside the int32 range and saturates the value into the older form below it (Limit -2147483649 answers Value '-2147483648' at 'limit'); the Segment and TotalSegments texts and the aggregation are a separate question. The checks now report the first fault in the order Limit, Segment, TotalSegments; Limit follows the measured int32 rule and the two segment fields keep the texts of the previous release. The two assertions in tests/test_query_scan.py go back to the previous text. A field of the wrong JSON type in another field (a string ExpressionAttributeValues beside Limit 0, a string TotalSegments beside Segment -1, a string ExpressionAttributeNames beside a malformed N, an object AttributesToGet beside ProjectionExpression) answered the check's ValidationException where the service and the previous release answer SerializationException. When a check fails the body is deserialized as the typed input and a SerializationException from that is returned instead, with the helper the Query path uses. A Unicode decimal digit in an N value passes the request-time check shared with Query, as it does on the service. The filter compares the value as the request wrote it, as on the previous release. The unit test for the parameter mix no longer asserts that an empty ScanFilter beside a FilterExpression passes, which the service refuses as a parameter mix; the engine's handling of an empty ScanFilter and an empty AttributesToGet is unchanged and covered by a separate issue. Measured against the service on 2026-10-25 (10:38, 10:55 and 11:31 UTC), each request twice.
LeeroyHannigan
requested review from
amrith,
c33howard,
jcshepherd,
pdf-amzn,
robinnsc and
yesyayen
as code owners
October 25, 2026 16:01
This was referenced Oct 25, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #545. Stacked on #544 (target branch
fix/query-key-value-types), which adds the request-time number check for Query that this change reuses.What changed
Scandid not check that theNvalues inExpressionAttributeValuesare numbers. A filter on{"N": "bob"}, abobnested in anL, anMor anNS,1e200, a 39-digit value or an emptyNanswered 200 withCount: 0. The service refuses each with aValidationExceptionbefore any row is read, whether or not an expression reads the value.The early Scan checks now run in one function on the raw request body,
validate_scan_request_bodyincrates/engine/src/scan_request.rs, in the order measured against DynamoDB on 2026-10-25 (us-east-1, 09:23 to 09:28, 10:38, 10:55 and 11:31 UTC, each request sent twice):Limit(at least 1),Segment(0 to 999999) andTotalSegments(1 to 1000000), each checked when present, with or without the other segment field, the first fault in that order. ALimitbelow 1 inside the int32 range answers1 validation error detected: Value at 'Limit' failed to satisfy constraint: Member must have value greater than or equal to 1; below int32 the service saturates the value and answers the older form,Value '-2147483648' at 'limit' ..., and so does the engine. TheSegmentandTotalSegmentstexts are the previous release's (see the gaps below).AttributesToGet,ScanFilter,ConditionalOperatoragainstProjectionExpression,FilterExpression).ReturnConsumedCapacityenum.ExpressionAttributeValues,NSmembers and values nested inLandMincluded, throughvalidate_expression_attribute_value_numbersfromquery_key_values.rs, which accepts a Unicode decimal digit (general categoryNd) as the digit it stands for, as the service does. The filter then compares the value as the request wrote it, as on main; the service reads it as the digit there too, which is left to a separate issue. TheNSmembers are checked when the value map deserializes; anNSwith a duplicate member is reported by the deserializer (Input collection ... contains duplicates) ahead of a malformed member in the same set, where the service reports the malformed member. The texts:The parameter cannot be converted to a numeric value: bob,Number overflow. Attempting to store a number with magnitude larger than supported range,Number underflow. Attempting to store a number with magnitude smaller than supported range,Attempting to store more than 38 significant digits in a Number, andThe parameter cannot be converted to a numeric value:for an emptyN.Selectenum.Each step is reported ahead of the next, ahead of the table lookup, and ahead of the checks the handler makes after deserializing the body (Segment pairing, IndexName, ConsistentRead on a GSI, Select COUNT with ProjectionExpression, ExclusiveStartKey, undefined placeholder, unused value, filter syntax). A field of the wrong JSON type anywhere in the body (a string
ExpressionAttributeValuesbesideLimit: 0, a stringTotalSegmentsbesideSegment: -1, a stringExpressionAttributeNamesbeside a malformedN, an objectAttributesToGetbesideProjectionExpression) answersSerializationExceptionahead of all of them: when a check fails, the body is deserialized as the typed input and aSerializationExceptionfrom that is returned instead, through the helper #544 adds for Query. The helper leaves anAttributeValuewith an unknown type descriptor to the check:Limit: 0beside{"X": "1"}answers theLimitValidationException, as on the service. A body that passes every check is deserialized once, by the handler.Two behaviours changed beyond the number check, each because the check order above requires it and each measured on 2026-10-25:
Limitbelow 1 inside the int32 range answersValue at 'Limit'with no value (Value '0' at 'limit'before), and theSegmentandTotalSegmentsranges are checked at request time, each when present, where the engine checked them after the table lookup and only when both fields were present. Their texts are unchanged;tests/test_query_scan.pyis unchanged.ReturnConsumedCapacityerror alone. The handler aggregated the two withSelectfirst (measured 2026-08-24); the service does not do that today.crates/engine/src/scan.rsdrops the enum pre-scan, the typed parameter mix check, the range arms of the segment match and the Limit check (520 to 462 lines). The Segment pairing checks and their text stay where they were.Tests
crates/engine/src/scan_request/tests.rs: seven unit tests covering the measured texts and orders above, the int32 boundary pair, the shape-fault order with its controls and the Unicode digit rows.cargo test -p extenddb-engine --lib: 208 passed.tests/test_scan_number_values.py: 69 wire tests, dual-target. TheSegmentandTotalSegmentsrange rows assert that aValidationExceptionnaming the field is reported ahead of the number, not a text. Against the binary built from 4a4e92f (the head of fix(query): check Query values against the key schema and the number rules #544 before its fold round): 49 failed, 20 passed (the twenty are rows the base already satisfied: the parameter mix, theReturnConsumedCapacityenum, the segment range positions, the four shape-fault rows, the well-formed and Unicode digit rows that pass with no number check). Against this branch on SQLite: 69 passed..github/workflows/integration.yml, jobsrun-integrationandrun-integration-sqlite, stepRun integration tests.Gates
Run locally on SQLite at the head:
cargo +1.97.0 fmt --all -- --check(pass);cargo +1.97.0 clippy -p extenddb-engine --all-targets -- -D warnings(pass);cargo test -p extenddb-engine --lib(208 passed); every wire file intests/that mentions Scan, 24 files (818 passed, 7 skipped, 0 failed). On the earlier head of this branch: the eight files intests/python/that mention Scan (157 passed). The host's disk did not allow a workspace test build, so the full workspace suite, the 1.88.0 run and the PostgreSQL wire run are left to CI. No dependency changed.Pre-existing gaps surfaced, not changed here
Measured on 2026-10-25 against the service, each present on main:
Segmentnot belowTotalSegments: the service answersThe Segment parameter is zero-based and must be less than parameter TotalSegmentswith no suffix; the engine appends: Segment: 5 is not less than TotalSegments: 2.1 validation error detected: Cannot specify the ProjectionExpression when choosing to get only the Count; engineCannot specify the ProjectionExpression when Select is COUNT.SegmentandTotalSegmentsrange texts: the service answersValue at 'Segment'andValue at 'TotalSegments'with no value for integers inside int32, aggregates several range faults into oneN validation errors detectedmessage in the order Limit, Segment, TotalSegments, and uses the olderValue '<n>' at '<field>'form with the value saturated when any field is outside int32 or a float; the engine keepsValue '<n>' at 'segment',Value '<n>' at 'totalSegments'andThe parameter TotalSegments should be greater than or equal to 1, and reports one fault.ScanFilteras present for the parameter mix (ScanFilter: {}with aFilterExpressionanswers the mix) and refusesAttributesToGet: []withValue at 'AttributesToGet' failed to satisfy constraint: Member must have length greater than or equal to 1ahead of the number check; the engine accepts both.Nin a legacyScanFilterAttributeValueListor inExclusiveStartKeyanswers the number text on the service; the engine does not check those.ExpressionAttributeValuesnested 32 levels or deeper answersNesting Levels have exceeded supported limitson the service; the engine does not check the depth on Scan or Query.PutItem,UpdateItem, the batch and transaction writes) refuse a Unicode decimal digit in anNthat the service accepts and stores as the ASCII value;GetItemandExclusiveStartKeyby such a key answerInternalServerError; and a filter written with a Unicode digit compares the text as written where the service reads the digit.Unmeasured
onethentwo). The engine reports the first in placeholder order; the wire test accepts either.AccessDeniedExceptionor the numberValidationException(no restricted principal was available). The engine runs the check in the operation handler only.ValidationExceptionthe deserializer raises (an empty set, a duplicate, an invalidAttributeValue): the engine reports the check's error for the latter.