Skip to content

Commit a8ac356

Browse files
author
Dennis Kieselhorst
authored
Merge pull request aws#1089 from jpfennelly/976-query-params-not-urldecoded
Query parameter values not URL decoded when using a RequestStreamHandler implementation
2 parents 24908a1 + 269a327 commit a8ac356

7 files changed

Lines changed: 428 additions & 94 deletions

File tree

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,16 @@ public Enumeration<String> getParameterNames() {
301301
@Override
302302
@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
303303
public String[] getParameterValues(String s) {
304-
List<String> values = new ArrayList<>(Arrays.asList(getQueryParamValues(queryString, s, config.isQueryStringCaseSensitive())));
305304

305+
List<String> values = getQueryParamValuesAsList(queryString, s, config.isQueryStringCaseSensitive());
306+
307+
// copy list so we don't modifying the underlying multi-value query params
308+
if (values != null) {
309+
values = new ArrayList<>(values);
310+
} else {
311+
values = new ArrayList<>();
312+
}
313+
306314
values.addAll(Arrays.asList(getFormBodyParameterCaseInsensitive(s)));
307315

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

487495
String[] kv = value.split(QUERY_STRING_KEY_VALUE_SEPARATOR);
488496
String key = URLDecoder.decode(kv[0], LambdaContainerHandler.getContainerConfig().getUriEncoding());
489-
String val = kv.length == 2 ? kv[1] : "";
497+
String val = kv.length == 2 ? AwsHttpServletRequest.decodeValueIfEncoded(kv[1]) : "";
490498
qsMap.add(key, val);
491499
} catch (UnsupportedEncodingException e) {
492-
log.error("Unsupported encoding in query string key: " + SecurityUtils.crlf(value), e);
500+
log.error("Unsupported encoding in query string key-value pair: " + SecurityUtils.crlf(value), e);
493501
}
494502
}
495503
return qsMap;

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java‎

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ private void addPart(Map<String, List<Part>> params, String fieldName, Part newP
540540

541541
protected String[] getQueryParamValues(MultiValuedTreeMap<String, String> qs, String key, boolean isCaseSensitive) {
542542
List<String> value = getQueryParamValuesAsList(qs, key, isCaseSensitive);
543-
if (value == null){
543+
if (value == null) {
544544
return null;
545545
}
546546
return value.toArray(new String[0]);
@@ -563,38 +563,56 @@ protected List<String> getQueryParamValuesAsList(MultiValuedTreeMap<String, Stri
563563
}
564564

565565
protected Map<String, String[]> generateParameterMap(MultiValuedTreeMap<String, String> qs, ContainerConfig config) {
566+
return generateParameterMap(qs, config, false);
567+
}
568+
569+
protected Map<String, String[]> generateParameterMap(MultiValuedTreeMap<String, String> qs, ContainerConfig config, boolean decodeQueryParams) {
566570
Map<String, String[]> output;
567571

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

570574
if (qs == null) {
571575
// Just transform the List<String> values to String[]
572-
output = formEncodedParams.entrySet().stream()
576+
return formEncodedParams.entrySet().stream()
573577
.collect(Collectors.toMap(Map.Entry::getKey, (e) -> e.getValue().toArray(new String[0])));
574-
} else {
575-
Map<String, List<String>> queryStringParams;
576-
if (config.isQueryStringCaseSensitive()) {
577-
queryStringParams = qs;
578-
} else {
579-
// If it's case insensitive, we check the entire map on every parameter
580-
queryStringParams = qs.entrySet().stream().parallel().collect(
581-
Collectors.toMap(
582-
Map.Entry::getKey,
583-
e -> getQueryParamValuesAsList(qs, e.getKey(), false)
584-
));
585-
}
586-
587-
// Merge formEncodedParams and queryStringParams Maps
588-
output = Stream.of(formEncodedParams, queryStringParams).flatMap(m -> m.entrySet().stream())
589-
.collect(
590-
Collectors.toMap(
591-
Map.Entry::getKey,
592-
e -> e.getValue().toArray(new String[0]),
593-
// If a parameter is in both Maps, we merge the list of values (and ultimately transform to String[])
594-
(formParam, queryParam) -> Stream.of(formParam, queryParam).flatMap(Stream::of).toArray(String[]::new)
595-
));
578+
}
596579

580+
// decode all keys and values in map
581+
final MultiValuedTreeMap<String, String> decodedQs = new MultiValuedTreeMap<String, String>();
582+
if (decodeQueryParams) {
583+
for (Map.Entry<String, List<String>> entry : qs.entrySet()) {
584+
String k = decodeValueIfEncoded(entry.getKey());
585+
List<String> v = getQueryParamValuesAsList(qs, entry.getKey(), false).stream()
586+
.map(AwsHttpServletRequest::decodeValueIfEncoded)
587+
.collect(Collectors.toList());
588+
// addAll in case map has 2 keys that are identical once decoded
589+
decodedQs.addAll(k, v);
590+
}
591+
} else {
592+
decodedQs.putAll(qs);
597593
}
594+
595+
Map<String, List<String>> queryStringParams;
596+
if (config.isQueryStringCaseSensitive()) {
597+
queryStringParams = decodedQs;
598+
} else {
599+
// If it's case insensitive, we check the entire map on every parameter
600+
queryStringParams = decodedQs.entrySet().stream().parallel().collect(
601+
Collectors.toMap(
602+
Map.Entry::getKey,
603+
e -> getQueryParamValuesAsList(decodedQs, e.getKey(), false)
604+
));
605+
}
606+
607+
// Merge formEncodedParams and queryStringParams Maps
608+
output = Stream.of(formEncodedParams, queryStringParams).flatMap(m -> m.entrySet().stream())
609+
.collect(
610+
Collectors.toMap(
611+
Map.Entry::getKey,
612+
e -> e.getValue().toArray(new String[0]),
613+
// If a parameter is in both Maps, we merge the list of values (and ultimately transform to String[])
614+
(formParam, queryParam) -> Stream.of(formParam, queryParam).flatMap(Stream::of).toArray(String[]::new)
615+
));
598616

599617
return output;
600618
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -328,17 +328,26 @@ public String getContentType() {
328328

329329
@Override
330330
public String getParameter(String s) {
331-
String queryStringParameter = getFirstQueryParamValue(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive());
332-
if (queryStringParameter != null) {
333-
return queryStringParameter;
334-
}
335331

336-
String[] bodyParams = getFormBodyParameterCaseInsensitive(s);
337-
if (bodyParams.length == 0) {
338-
return null;
339-
} else {
340-
return bodyParams[0];
332+
// decode key if ALB
333+
if (request.getRequestSource() == RequestSource.ALB) {
334+
s = decodeValueIfEncoded(s);
341335
}
336+
337+
String queryStringParameter = getFirstQueryParamValue(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive());
338+
if (queryStringParameter != null) {
339+
if (request.getRequestSource() == RequestSource.ALB) {
340+
queryStringParameter = decodeValueIfEncoded(queryStringParameter);
341+
}
342+
return queryStringParameter;
343+
}
344+
345+
String[] bodyParams = getFormBodyParameterCaseInsensitive(s);
346+
if (bodyParams.length == 0) {
347+
return null;
348+
} else {
349+
return bodyParams[0];
350+
}
342351
}
343352

344353

@@ -348,18 +357,43 @@ public Enumeration<String> getParameterNames() {
348357
if (request.getMultiValueQueryStringParameters() == null) {
349358
return Collections.enumeration(formParameterNames);
350359
}
351-
return Collections.enumeration(Stream.concat(formParameterNames.stream(),
352-
request.getMultiValueQueryStringParameters().keySet().stream()).collect(Collectors.toSet()));
360+
361+
Set<String> paramNames = request.getMultiValueQueryStringParameters().keySet();
362+
if (request.getRequestSource() == RequestSource.ALB) {
363+
paramNames = paramNames.stream().map(AwsProxyHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toSet());
364+
}
365+
366+
return Collections.enumeration(
367+
Stream.concat(formParameterNames.stream(), paramNames.stream())
368+
.collect(Collectors.toSet()));
353369
}
354370

355371

356372
@Override
357373
@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
358374
public String[] getParameterValues(String s) {
359-
List<String> values = new ArrayList<>(Arrays.asList(getQueryParamValues(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive())));
375+
376+
// decode key if ALB
377+
if (request.getRequestSource() == RequestSource.ALB) {
378+
s = decodeValueIfEncoded(s);
379+
}
360380

361-
values.addAll(Arrays.asList(getFormBodyParameterCaseInsensitive(s)));
381+
List<String> values = getQueryParamValuesAsList(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive());
382+
383+
// copy list so we don't modifying the underlying multi-value query params
384+
if (values != null) {
385+
values = new ArrayList<>(values);
386+
} else {
387+
values = new ArrayList<>();
388+
}
389+
390+
// decode values if ALB
391+
if (values != null && request.getRequestSource() == RequestSource.ALB) {
392+
values = values.stream().map(AwsHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toList());
393+
}
362394

395+
values.addAll(Arrays.asList(getFormBodyParameterCaseInsensitive(s)));
396+
363397
if (values.size() == 0) {
364398
return null;
365399
} else {
@@ -370,7 +404,7 @@ public String[] getParameterValues(String s) {
370404

371405
@Override
372406
public Map<String, String[]> getParameterMap() {
373-
return generateParameterMap(request.getMultiValueQueryStringParameters(), config);
407+
return generateParameterMap(request.getMultiValueQueryStringParameters(), config, request.getRequestSource() == RequestSource.ALB);
374408
}
375409

376410

0 commit comments

Comments
 (0)