fixes multivalue query param requests for HTTP APIs#3365
fixes multivalue query param requests for HTTP APIs#3365jlapier wants to merge 1 commit intoaws:developfrom
Conversation
When a query parameter is specified multiple times in a URL, e.g. `path?mykey=1&mykey=2&mykey=3` this should create an event with all appropriate values. Prior to this commit, when using `sam local start-api` with an HTTP API, only the first value is present in the queryStringParameters of the event. In payload format v2 [1] used in HTTP APIs, multiple values should be combined with commas and included in the queryStringParameters field. This commit fixes the behavior for query string parameters. [1] https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html
hawflau
left a comment
hawflau
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Just left two comments. Can you also please check the check failures and fix the merge conflict?
| # Flask returns an ImmutableMultiDict so convert to a dictionary that becomes | ||
| # a dict(str: list) then iterate over | ||
| for query_string_key, query_string_list in flask_request.args.lists(): | ||
| query_string_value_length = len(query_string_list) |
There was a problem hiding this comment.
it would be better to just keep the last line of L848-L854, i.e.
query_string_dict[query_string_key] = ",".join(query_string_list)
",".join(query_string_list) returns an empty string if query_string_list is an empty list.
Or, we could make the entire function even shorter:
query_string_dict = {
query_string_key: ",".join(query_string_list)
for query_string_key, query_string_list in flask_request.args.lists()
}
return query_string_dict
| self.request_mock.mimetype = "application/json" | ||
| query_param_args_mock = Mock() | ||
| query_param_args_mock.lists.return_value = {"query": ["params"]}.items() | ||
| query_param_args_mock.lists.return_value = {"query": ["param1","param2"]}.items() |
There was a problem hiding this comment.
you can run make black to fix the formatting issue here
|
Closing due to no response. I re-submitted a new one (#3788) to resolve merge conflicts, apply recommendations and add one more test. |
|
Thanks for following up on this @hawflau! My apologies for being unable to find the time to respond to feedback. |
4 participants
When a query parameter is specified multiple times in a URL, e.g.
path?mykey=1&mykey=2&mykey=3this should create an event with all appropriate values.
Prior to this commit, when using
sam local start-apiwith an HTTP API, only the first value is present in thequeryStringParametersof the event. In payload format v2 used in HTTP APIs, multiple values should be combined with commas and included in thequeryStringParametersfield. This commit fixes the behavior for query string parameters.Which issue(s) does this change fix?
#3194
Why is this change necessary?
The local API does not match the behavior of an HTTP API in API Gateway.
How does it address the issue?
It formats multivalue query parameters per the payload format version 2.
What side effects does this change have?
Previously truncated query parameter values will be included in the queryStringParameters field of an event.
Checklist
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.