Skip to content

fix(dynamodb): PartiQL UPDATE param order, nested REMOVE, non-ASCII WHERE panic#2013

Merged
vieiralucas merged 1 commit into
mainfrom
wt-bh-ddb
Jun 27, 2026
Merged

fix(dynamodb): PartiQL UPDATE param order, nested REMOVE, non-ASCII WHERE panic#2013
vieiralucas merged 1 commit into
mainfrom
wt-bh-ddb

Conversation

@vieiralucas

@vieiralucas vieiralucas commented Jun 27, 2026

Copy link
Copy Markdown
Member

Bug-hunt batch 2 (cycle 6). Three DynamoDB fixes.

  • PartiQL UPDATE positional params swapped (HIGH, data corruption). UPDATE t SET x=? WHERE y=? bound the SET value from the WHERE parameter and vice versa — WHERE was evaluated from parameter index 0 and SET started after it. Since SET is textually first, SET now consumes parameters[0..set_count] and WHERE the rest.
  • UpdateExpression nested REMOVE no-op (HIGH). REMOVE profile.middle / REMOVE tags[0] removed a literal top-level key named "profile.middle" instead of descending. A new remove_path resolves dotted map paths and list indices (mirroring the SET writer); missing paths stay a silent no-op like AWS.
  • Non-ASCII WHERE panic (HIGH, DoS). split_partiql_and_clauses uppercased with Unicode to_uppercase() (can change byte length) then sliced by byte indices bounds-checked against the original string — a length-shrinking char ('ff' -> "FF") ran the slice past the end and dropped the connection. Now to_ascii_uppercase(), keeping buffers byte-aligned.

Tests

Unit: remove_path (nested map / list index / missing-path no-op), non-ASCII split no-panic. E2E: parameterized UPDATE order round-trip, nested REMOVE round-trip. Full dynamodb lib suite (322) + partiql e2e (7) green.

Builds clean; clippy --all-targets -D warnings clean; fmt clean.


Summary by cubic

Fixes three DynamoDB issues: corrects PartiQL UPDATE parameter binding order, makes REMOVE handle nested paths and list indices, and prevents a panic on non-ASCII WHERE clauses. Prevents wrong-row matches/writes and avoids connection drops on Unicode input.

  • Bug Fixes
    • PartiQL UPDATE: bind ? in textual order. SET consumes the first N params; WHERE uses the rest.
    • UpdateExpression REMOVE: resolves dotted paths and list indices (e.g., profile.middle, tags[0]). Missing paths are no-ops.
    • WHERE parser: use ASCII uppercasing to keep byte alignment and avoid slicing panics on chars like .

Written for commit 60d431d. Summary will update on new commits.

Review in cubic

…HERE panic

Three DynamoDB correctness/robustness fixes from the cycle-6 bug audit.

- PartiQL UPDATE positional parameters were swapped. `UPDATE t SET x=? WHERE
  y=?` bound the SET value from the WHERE parameter and vice versa, because
  WHERE was evaluated from parameter index 0 and SET started after it. Since
  SET is textually first, SET now consumes parameters[0..set_count] and WHERE
  the rest — a silent wrong-row-match + wrong-value-write before.
- UpdateExpression REMOVE of a nested path was a no-op. `REMOVE profile.middle`
  / `REMOVE tags[0]` removed a literal top-level key named "profile.middle"
  (which never exists) instead of descending into the document. A new
  remove_path resolves dotted map paths and list indices (mirroring the SET
  writer), so the nested attribute is actually deleted; missing paths stay a
  silent no-op as in AWS.
- split_partiql_and_clauses could panic on a non-ASCII WHERE clause. It built
  an uppercased copy with Unicode to_uppercase() (which can change byte length)
  then sliced it by byte indices bounds-checked against the original string, so
  a length-shrinking char (e.g. 'ff' -> "FF") ran the slice past the end and
  dropped the connection. Now uses to_ascii_uppercase(), keeping the buffers
  byte-aligned.

Adds unit tests (remove_path nested/list/missing, non-ASCII split no-panic) and
e2e tests (parameterized UPDATE order, nested REMOVE round-trip).
@vieiralucas vieiralucas merged commit 56241bd into main Jun 27, 2026
104 checks passed
@vieiralucas vieiralucas deleted the wt-bh-ddb branch June 27, 2026 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant