Skip to content

fix(lambda-http): build query strings in a deterministic order - #1182

Open
abhiishek340 wants to merge 1 commit into
aws:mainfrom
abhiishek340:fix/deterministic-query-string-order
Open

abhiishek340 wants to merge 1 commit into
aws:mainfrom
abhiishek340:fix/deterministic-query-string-order

Conversation

@abhiishek340

Copy link
Copy Markdown

📬 Issue #, if available: Fixes #1181

✍️ Description of changes:

ALB, API Gateway REST (v1) and VPC Lattice requests rebuild the URI query string in build_request_uri from a QueryMap. QueryMap wraps a HashMap with a per-instance random seed, so the same event produced a differently ordered query string each time. Apps that sign, hash or compare the raw URL (CakePHP FormProtection, Laravel signed URLs, cache keys) fail intermittently. API Gateway HTTP (v2) isn't affected because it uses raw_query_string.

This PR implements option 2 from the issue: build_request_uri now serializes the pairs sorted by key. It uses the same form_urlencoded serializer that QueryMap::to_query_string uses, so escaping is unchanged. The sort is stable, so repeated keys keep their value order (b=2&b=1 stays b=2&b=1).

Sorting doesn't restore the original request order. The issue's option 1 would need an insertion-ordered map in query_map and confirmation that ALB preserves key order in its JSON, so I kept this change local to lambda-http. It makes the URI stable either way, and it matches order-independent schemes such as Laravel's, which sign with keys sorted.

Testing:

  • New alb_request_query_string_order_is_deterministic test: it deserializes the issue's ALB event 64 times and asserts one exact URI. It fails on main (e.g. ?b=2&b=1&parent_id=1&a=1&type=leader_employee) and passes with this change.
  • cargo test -p lambda_http --all-features: 98 unit and 6 doc tests pass.
  • cargo check -p lambda_http --no-default-features --features <each of alb, apigw_rest, apigw_http, apigw_websockets, vpc_lattice>: no new warnings compared to main.

🔏 By submitting this pull request

  • I confirm that I've ran cargo +nightly fmt.
  • I confirm that I've ran cargo clippy --fix.
  • I confirm that I've made a best effort attempt to update all relevant documentation.
  • I confirm that my contribution is made under the terms of the Apache 2.0 license.

ALB, API Gateway REST and VPC Lattice requests rebuild the URI query
from a QueryMap, which is backed by a HashMap. Its iteration order is
random per instance, so the same event produced a differently ordered
query string each time, breaking apps that sign or compare the raw URL.

Sort the pairs by key before serializing. The sort is stable, so the
values of a repeated key keep their order.

Fixes aws#1181

This branch has not been deployed

No deployments
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.

Query string parameter order is randomized per request for ALB (and other map-based) events, which breaks apps that rely on the raw request URL

1 participant