Copilot commented on code in PR #13520:
URL: https://github.com/apache/apisix/pull/13520#discussion_r3394773518
##########
apisix/plugins/aws-lambda.lua:
##########
@@ -120,17 +132,44 @@ local function request_processor(conf, ctx, params)
end
end
- -- computing canonical query string
- local canonical_qs = {}
- local canonical_qs_i = 0
+ -- computing canonical query string: URI-encode the name and value of
+ -- every pair, then sort the pairs by encoded name and encoded value.
+ -- params.query holds the percent-decoded args: a table value means the
+ -- arg appears multiple times and a true value means an arg without value
+ local query_pairs = {}
+ local query_pairs_i = 0
for k, v in pairs(params.query) do
- canonical_qs_i = canonical_qs_i + 1
- canonical_qs[canonical_qs_i] = ngx.unescape_uri(k) .. "=" ..
ngx.unescape_uri(v)
+ local name = uri_encode(k)
+ if type(v) ~= "table" then
+ v = {v}
+ end
+ for _, value in ipairs(v) do
+ if value == true then
+ value = ""
+ end
+ query_pairs_i = query_pairs_i + 1
+ query_pairs[query_pairs_i] = {name, uri_encode(value)}
+ end
end
- tab_sort(canonical_qs)
+ tab_sort(query_pairs, function(a, b)
+ if a[1] ~= b[1] then
+ return a[1] < b[1]
+ end
+ return a[2] < b[2]
+ end)
+
+ local canonical_qs = {}
+ for i = 1, query_pairs_i do
+ canonical_qs[i] = query_pairs[i][1] .. "=" .. query_pairs[i][2]
+ end
canonical_qs = tab_concat(canonical_qs, "&")
+ -- send exactly the query string that gets signed: lua-resty-http passes
+ -- a string through unmodified, while a table would be re-encoded by
+ -- ngx.encode_args whose output may differ from the signed string
+ params.query = canonical_qs
Review Comment:
Overwriting `params.query` with the canonical (sorted, fully
percent-encoded) query string changes the actual request sent upstream
(reorders repeated params and turns valueless params like `?flag` into
`flag=`). SigV4 validation does not require the wire query string to be
canonical; AWS canonicalizes (encodes + sorts) server-side before computing the
signature. To avoid a potentially breaking behavioral change for upstreams that
care about raw query form, keep `params.query` as originally parsed and use
`canonical_qs` only for signature computation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
