Query parameter values not URL decoded when using HTTP API V2 Gateway with a RequestStreamHandler implementation #1089
Conversation
…ter names in HTTP API requests
There was a problem hiding this comment.
Thanks for the very detailed PR. I added a couple of minor comments.
Also something that wasn't clear to me is: if the problem is with API GW V2, then why are there changes related to ALB too? Is there a separate bug with ALB encoding as well? Or is it related to the same issue?
| } | ||
|
|
||
| // decode all keys and values in map |
There was a problem hiding this comment.
It's a fairly minor thing, but all this new code has a mix of spaces and tabs (first indentation is 4 spaces, then only tabs). Even though the project has some files with a mix of both, the vast majority of files are using spaces only. So try to keep this one with spaces only too. (The tests added are also using tabs, and hopefully you can change them to spaces)
| List<String> values = new ArrayList<>(Arrays.asList(getQueryParamValues(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()))); | ||
| // List<String> values = getQueryParamValuesAsList(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()); |
There was a problem hiding this comment.
I imagine this comment shouldn't be here
Issue #, if available: 976
Description of changes:
AwsProxyRequestBuilder, used by unit tests, to be immutable when calling thealb()method. Previously, a call to thealb()method would encode query parameters, modifying the state of the builder. Subsequent usage of the builder for non-ALB unit tests would use the now-encoded query parameters. This wasn't an issue in current current tests because no tests were testing query param decoding. Added unit tests for the builder class itself.generateParameterMap()method that takes a flag to determine if it should decode query parameters so that functionality for ALB and non-ALB requests can be supportedBy submitting this pull request