From d359e2df206b4a7b888d6b284c0dc318e16e5693 Mon Sep 17 00:00:00 2001 From: Renato Valenzuela Date: Wed, 26 Jul 2023 22:07:45 +0000 Subject: [PATCH 1/2] Change parameter transformation logic --- .../servlet/AwsHttpServletRequest.java | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java index 1bd12c048..4ba1f3cbb 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java @@ -44,6 +44,8 @@ import java.nio.charset.StandardCharsets; import java.time.format.DateTimeFormatter; import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** @@ -546,39 +548,57 @@ protected Map getMultipartFormParametersMap() { } protected String[] getQueryParamValues(MultiValuedTreeMap qs, String key, boolean isCaseSensitive) { + return getQueryParamValuesAsList(qs, key, isCaseSensitive).toArray(new String[0]); + } + + protected List getQueryParamValuesAsList(MultiValuedTreeMap qs, String key, boolean isCaseSensitive) { if (qs != null) { if (isCaseSensitive) { - return qs.get(key).toArray(new String[0]); + return qs.get(key); } for (String k : qs.keySet()) { if (k.toLowerCase(Locale.getDefault()).equals(key.toLowerCase(Locale.getDefault()))) { - return qs.get(k).toArray(new String[0]); + return qs.get(k); } } } - return new String[0]; + return new ArrayList(); } protected Map generateParameterMap(MultiValuedTreeMap qs, ContainerConfig config) { - Map output = new HashMap<>(); + Map output; - Map> params = getFormUrlEncodedParametersMap(); - params.entrySet().stream().parallel().forEach(e -> { - output.put(e.getKey(), e.getValue().toArray(new String[0])); - }); + Map> formEncodedParams = getFormUrlEncodedParametersMap(); + + if (qs == null) { + // Just transform the List values to String[] + output = formEncodedParams.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, (e) -> e.getValue().toArray(new String[0]))); + } else { + Map> 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) + )); - if (qs != null) { - qs.keySet().stream().parallel().forEach(e -> { - List newValues = new ArrayList<>(); - if (output.containsKey(e)) { - String[] values = output.get(e); - newValues.addAll(Arrays.asList(values)); - } - newValues.addAll(Arrays.asList(getQueryParamValues(qs, e, config.isQueryStringCaseSensitive()))); - output.put(e, newValues.toArray(new String[0])); - }); } return output; From 1a0906f0db4d8083b2ac2fa2e5e11600d62dd9eb Mon Sep 17 00:00:00 2001 From: Renato Valenzuela Date: Wed, 18 Oct 2023 00:30:30 +0000 Subject: [PATCH 2/2] Add tests for generateParameterMap --- .../servlet/AwsHttpServletRequest.java | 8 +- .../servlet/AwsHttpServletRequestTest.java | 137 ++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java index 4ba1f3cbb..7a44fa95e 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java @@ -548,7 +548,11 @@ protected Map getMultipartFormParametersMap() { } protected String[] getQueryParamValues(MultiValuedTreeMap qs, String key, boolean isCaseSensitive) { - return getQueryParamValuesAsList(qs, key, isCaseSensitive).toArray(new String[0]); + List value = getQueryParamValuesAsList(qs, key, isCaseSensitive); + if (value == null){ + return null; + } + return value.toArray(new String[0]); } protected List getQueryParamValuesAsList(MultiValuedTreeMap qs, String key, boolean isCaseSensitive) { @@ -564,7 +568,7 @@ protected List getQueryParamValuesAsList(MultiValuedTreeMap(); + return Collections.emptyList(); } protected Map generateParameterMap(MultiValuedTreeMap qs, ContainerConfig config) { diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java index 051c12fc6..2bc433041 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java @@ -1,6 +1,7 @@ package com.amazonaws.serverless.proxy.internal.servlet; import com.amazonaws.serverless.proxy.model.AwsProxyRequest; +import com.amazonaws.serverless.proxy.model.MultiValuedTreeMap; import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder; import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext; import com.amazonaws.serverless.proxy.model.ContainerConfig; @@ -15,6 +16,8 @@ import java.util.Base64; import java.util.List; +import java.util.Map; +import java.util.Arrays; public class AwsHttpServletRequestTest { @@ -33,6 +36,14 @@ public class AwsHttpServletRequestTest { .queryString("one", "two").queryString("json", "{\"name\":\"faisal\"}").build(); private static final AwsProxyRequest multipleParams = new AwsProxyRequestBuilder("/test", "GET") .queryString("one", "two").queryString("one", "three").queryString("json", "{\"name\":\"faisal\"}").build(); + private static final AwsProxyRequest formEncodedAndQueryString = new AwsProxyRequestBuilder("/test", "POST") + .queryString("one", "two").queryString("one", "three") + .queryString("five", "six") + .form("one", "four") + .form("seven", "eight").build(); + private static final AwsProxyRequest differentCasing = new AwsProxyRequestBuilder("/test", "POST") + .queryString("one", "two").queryString("one", "three") + .queryString("ONE", "four").build(); private static final MockLambdaContext mockContext = new MockLambdaContext(); @@ -182,4 +193,130 @@ void queryStringWithMultipleValues_generateQueryString_validQuery() { assertTrue(parsedString.contains("json=%7B%22name%22%3A%22faisal%22%7D")); assertTrue(parsedString.contains("&") && parsedString.indexOf("&") > 0 && parsedString.indexOf("&") < parsedString.length()); } + + @Test + void parameterMap_generateParameterMap_validQuery() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(queryString, mockContext, null, config); + + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config); + } catch (Exception e) { + e.printStackTrace(); + fail("Could not generate parameter map"); + } + assertArrayEquals(new String[]{"two"}, paramMap.get("one")); + assertArrayEquals(new String[]{"four"}, paramMap.get("three")); + assertTrue(paramMap.size() == 2); + } + + @Test + void parameterMap_generateParameterMap_nullParameter() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(queryStringNullValue, mockContext, null, config); + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config); + } catch (Exception e) { + e.printStackTrace(); + fail("Could not generate parameter map"); + } + + assertArrayEquals(new String[]{"two"}, paramMap.get("one")); + assertArrayEquals(new String[]{null}, paramMap.get("three")); + assertTrue(paramMap.size() == 2); + } + + @Test + void parameterMapWithEncodedParams_generateParameterMap_validQuery() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(encodedQueryString, mockContext, null, config); + + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config); + } catch (Exception e) { + e.printStackTrace(); + fail("Could not generate parameter map"); + } + + assertArrayEquals(new String[]{"two"}, paramMap.get("one")); + assertArrayEquals(new String[]{"{\"name\":\"faisal\"}"}, paramMap.get("json")); + assertTrue(paramMap.size() == 2); + } + + @Test + void parameterMapWithMultipleValues_generateParameterMap_validQuery() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(multipleParams, mockContext, null, config); + + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config); + } catch (Exception e) { + e.printStackTrace(); + fail("Could not generate parameter map"); + } + assertArrayEquals(new String[]{"two", "three"}, paramMap.get("one")); + assertArrayEquals(new String[]{"{\"name\":\"faisal\"}"}, paramMap.get("json")); + assertTrue(paramMap.size() == 2); + } + + @Test + void parameterMap_generateParameterMap_formEncodedAndQueryString() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(formEncodedAndQueryString, mockContext, null, config); + + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config); + } catch (Exception e) { + e.printStackTrace(); + fail("Could not generate parameter map"); + } + // Combines form encoded parameters (one=four) with query string (one=two,three) + // The order between them is not officially guaranteed (it could be four,two,three or two,three,four) + // Current implementation gives form encoded parameters first + assertArrayEquals(new String[]{"four", "two", "three"}, paramMap.get("one")); + assertArrayEquals(new String[]{"six"}, paramMap.get("five")); + assertArrayEquals(new String[]{"eight"}, paramMap.get("seven")); + assertTrue(paramMap.size() == 3); + } + + @Test + void parameterMap_generateParameterMap_differentCasing() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(differentCasing, mockContext, null, config); + + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config); + } catch (Exception e) { + e.printStackTrace(); + fail("Could not generate parameter map"); + } + // If a parameter is duplicated but with a different casing, it's replaced with only one of them + assertArrayEquals(paramMap.get("one"), paramMap.get("ONE")); + assertTrue(paramMap.size() == 2); + } + + @Test + void queryParamValues_getQueryParamValues() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(new AwsProxyRequest(), mockContext, null); + MultiValuedTreeMap map = new MultiValuedTreeMap<>(); + map.add("test", "test"); + map.add("test", "test2"); + String[] result1 = request.getQueryParamValues(map, "test", true); + assertArrayEquals(new String[]{"test", "test2"}, result1); + String[] result2 = request.getQueryParamValues(map, "TEST", true); + assertNull(result2); + } + + @Test + void queryParamValues_getQueryParamValues_caseInsensitive() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(new AwsProxyRequest(), mockContext, null); + MultiValuedTreeMap map = new MultiValuedTreeMap<>(); + map.add("test", "test"); + map.add("test", "test2"); + String[] result1 = request.getQueryParamValues(map, "test", false); + assertArrayEquals(new String[]{"test", "test2"}, result1); + String[] result2 = request.getQueryParamValues(map, "TEST", false); + assertArrayEquals(new String[]{"test", "test2"}, result2); + } + }