Skip to content

Commit

Permalink
Merge pull request #1089 from jpfennelly/976-query-params-not-urldecoded
Browse files Browse the repository at this point in the history 
Query parameter values not URL decoded when using a RequestStreamHandler implementation
  • Loading branch information
@deki
deki authored Nov 27, 2024
2 parents 24908a1 + 269a327 commit a8ac356
Show file tree
Hide file tree
Showing 7 changed files with 428 additions and 94 deletions.
14 changes: 11 additions & 3 deletions .../com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,16 @@ public Enumeration<String> getParameterNames() {
@Override
@SuppressFBWarnings("PZLA_PREFER_ZERO_LENGTH_ARRAYS") // suppressing this as according to the specs we should be returning null here if we can't find params
public String[] getParameterValues(String s) {
List<String> values = new ArrayList<>(Arrays.asList(getQueryParamValues(queryString, s, config.isQueryStringCaseSensitive())));

List<String> values = getQueryParamValuesAsList(queryString, s, config.isQueryStringCaseSensitive());

// copy list so we don't modifying the underlying multi-value query params
if (values != null) {
values = new ArrayList<>(values);
} else {
values = new ArrayList<>();
}

values.addAll(Arrays.asList(getFormBodyParameterCaseInsensitive(s)));

if (values.size() == 0) {
Expand Down Expand Up @@ -486,10 +494,10 @@ private MultiValuedTreeMap<String, String> parseRawQueryString(String qs) {

String[] kv = value.split(QUERY_STRING_KEY_VALUE_SEPARATOR);
String key = URLDecoder.decode(kv[0], LambdaContainerHandler.getContainerConfig().getUriEncoding());
String val = kv.length == 2 ? kv[1] : "";
String val = kv.length == 2 ? AwsHttpServletRequest.decodeValueIfEncoded(kv[1]) : "";
qsMap.add(key, val);
} catch (UnsupportedEncodingException e) {
log.error("Unsupported encoding in query string key: " + SecurityUtils.crlf(value), e);
log.error("Unsupported encoding in query string key-value pair: " + SecurityUtils.crlf(value), e);
}
}
return qsMap;
Expand Down
66 changes: 42 additions & 24 deletions .../src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ private void addPart(Map<String, List<Part>> params, String fieldName, Part newP

protected String[] getQueryParamValues(MultiValuedTreeMap<String, String> qs, String key, boolean isCaseSensitive) {
List<String> value = getQueryParamValuesAsList(qs, key, isCaseSensitive);
if (value == null){
if (value == null) {
return null;
}
return value.toArray(new String[0]);
Expand All @@ -563,38 +563,56 @@ protected List<String> getQueryParamValuesAsList(MultiValuedTreeMap<String, Stri
}

protected Map<String, String[]> generateParameterMap(MultiValuedTreeMap<String, String> qs, ContainerConfig config) {
return generateParameterMap(qs, config, false);
}

protected Map<String, String[]> generateParameterMap(MultiValuedTreeMap<String, String> qs, ContainerConfig config, boolean decodeQueryParams) {
Map<String, String[]> output;

Map<String, List<String>> formEncodedParams = getFormUrlEncodedParametersMap();

if (qs == null) {
// Just transform the List<String> values to String[]
output = formEncodedParams.entrySet().stream()
return formEncodedParams.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, (e) -> e.getValue().toArray(new String[0])));
} else {
Map<String, List<String>> queryStringParams;
if (config.isQueryStringCaseSensitive()) {
queryStringParams = qs;
} else {
// If it's case insensitive, we check the entire map on every parameter
queryStringParams = qs.entrySet().stream().parallel().collect(
Collectors.toMap(
Map.Entry::getKey,
e -> getQueryParamValuesAsList(qs, e.getKey(), false)
));
}

// Merge formEncodedParams and queryStringParams Maps
output = Stream.of(formEncodedParams, queryStringParams).flatMap(m -> m.entrySet().stream())
.collect(
Collectors.toMap(
Map.Entry::getKey,
e -> e.getValue().toArray(new String[0]),
// If a parameter is in both Maps, we merge the list of values (and ultimately transform to String[])
(formParam, queryParam) -> Stream.of(formParam, queryParam).flatMap(Stream::of).toArray(String[]::new)
));
}

// decode all keys and values in map
final MultiValuedTreeMap<String, String> decodedQs = new MultiValuedTreeMap<String, String>();
if (decodeQueryParams) {
for (Map.Entry<String, List<String>> entry : qs.entrySet()) {
String k = decodeValueIfEncoded(entry.getKey());
List<String> v = getQueryParamValuesAsList(qs, entry.getKey(), false).stream()
.map(AwsHttpServletRequest::decodeValueIfEncoded)
.collect(Collectors.toList());
// addAll in case map has 2 keys that are identical once decoded
decodedQs.addAll(k, v);
}
} else {
decodedQs.putAll(qs);
}

Map<String, List<String>> queryStringParams;
if (config.isQueryStringCaseSensitive()) {
queryStringParams = decodedQs;
} else {
// If it's case insensitive, we check the entire map on every parameter
queryStringParams = decodedQs.entrySet().stream().parallel().collect(
Collectors.toMap(
Map.Entry::getKey,
e -> getQueryParamValuesAsList(decodedQs, e.getKey(), false)
));
}

// Merge formEncodedParams and queryStringParams Maps
output = Stream.of(formEncodedParams, queryStringParams).flatMap(m -> m.entrySet().stream())
.collect(
Collectors.toMap(
Map.Entry::getKey,
e -> e.getValue().toArray(new String[0]),
// If a parameter is in both Maps, we merge the list of values (and ultimately transform to String[])
(formParam, queryParam) -> Stream.of(formParam, queryParam).flatMap(Stream::of).toArray(String[]::new)
));

return output;
}
Expand Down
62 changes: 48 additions & 14 deletions ...main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,17 +328,26 @@ public String getContentType() {

@Override
public String getParameter(String s) {
String queryStringParameter = getFirstQueryParamValue(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive());
if (queryStringParameter != null) {
return queryStringParameter;
}

String[] bodyParams = getFormBodyParameterCaseInsensitive(s);
if (bodyParams.length == 0) {
return null;
} else {
return bodyParams[0];
// decode key if ALB
if (request.getRequestSource() == RequestSource.ALB) {
s = decodeValueIfEncoded(s);
}

String queryStringParameter = getFirstQueryParamValue(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive());
if (queryStringParameter != null) {
if (request.getRequestSource() == RequestSource.ALB) {
queryStringParameter = decodeValueIfEncoded(queryStringParameter);
}
return queryStringParameter;
}

String[] bodyParams = getFormBodyParameterCaseInsensitive(s);
if (bodyParams.length == 0) {
return null;
} else {
return bodyParams[0];
}
}


Expand All @@ -348,18 +357,43 @@ public Enumeration<String> getParameterNames() {
if (request.getMultiValueQueryStringParameters() == null) {
return Collections.enumeration(formParameterNames);
}
return Collections.enumeration(Stream.concat(formParameterNames.stream(),
request.getMultiValueQueryStringParameters().keySet().stream()).collect(Collectors.toSet()));

Set<String> paramNames = request.getMultiValueQueryStringParameters().keySet();
if (request.getRequestSource() == RequestSource.ALB) {
paramNames = paramNames.stream().map(AwsProxyHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toSet());
}

return Collections.enumeration(
Stream.concat(formParameterNames.stream(), paramNames.stream())
.collect(Collectors.toSet()));
}


@Override
@SuppressFBWarnings("PZLA_PREFER_ZERO_LENGTH_ARRAYS") // suppressing this as according to the specs we should be returning null here if we can't find params
public String[] getParameterValues(String s) {
List<String> values = new ArrayList<>(Arrays.asList(getQueryParamValues(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive())));

// decode key if ALB
if (request.getRequestSource() == RequestSource.ALB) {
s = decodeValueIfEncoded(s);
}

values.addAll(Arrays.asList(getFormBodyParameterCaseInsensitive(s)));
List<String> values = getQueryParamValuesAsList(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive());

// copy list so we don't modifying the underlying multi-value query params
if (values != null) {
values = new ArrayList<>(values);
} else {
values = new ArrayList<>();
}

// decode values if ALB
if (values != null && request.getRequestSource() == RequestSource.ALB) {
values = values.stream().map(AwsHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toList());
}

values.addAll(Arrays.asList(getFormBodyParameterCaseInsensitive(s)));

if (values.size() == 0) {
return null;
} else {
Expand All @@ -370,7 +404,7 @@ public String[] getParameterValues(String s) {

@Override
public Map<String, String[]> getParameterMap() {
return generateParameterMap(request.getMultiValueQueryStringParameters(), config);
return generateParameterMap(request.getMultiValueQueryStringParameters(), config, request.getRequestSource() == RequestSource.ALB);
}


Expand Down
Loading

0 comments on commit a8ac356

Please sign in to comment.