diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java index 5a2534a2b..f318b7277 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java @@ -301,8 +301,16 @@ public Enumeration 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 values = new ArrayList<>(Arrays.asList(getQueryParamValues(queryString, s, config.isQueryStringCaseSensitive()))); + List 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) { @@ -486,10 +494,10 @@ private MultiValuedTreeMap 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; 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 97beabe64..6b5c90200 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 @@ -540,7 +540,7 @@ private void addPart(Map> params, String fieldName, Part newP protected String[] getQueryParamValues(MultiValuedTreeMap qs, String key, boolean isCaseSensitive) { List value = getQueryParamValuesAsList(qs, key, isCaseSensitive); - if (value == null){ + if (value == null) { return null; } return value.toArray(new String[0]); @@ -563,38 +563,56 @@ protected List getQueryParamValuesAsList(MultiValuedTreeMap generateParameterMap(MultiValuedTreeMap qs, ContainerConfig config) { + return generateParameterMap(qs, config, false); + } + + protected Map generateParameterMap(MultiValuedTreeMap qs, ContainerConfig config, boolean decodeQueryParams) { Map output; Map> formEncodedParams = getFormUrlEncodedParametersMap(); if (qs == null) { // Just transform the List 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> 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 decodedQs = new MultiValuedTreeMap(); + if (decodeQueryParams) { + for (Map.Entry> entry : qs.entrySet()) { + String k = decodeValueIfEncoded(entry.getKey()); + List 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> 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; } diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java index 6406be478..9c4b7971b 100644 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java +++ b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java @@ -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]; + } } @@ -348,18 +357,43 @@ public Enumeration getParameterNames() { if (request.getMultiValueQueryStringParameters() == null) { return Collections.enumeration(formParameterNames); } - return Collections.enumeration(Stream.concat(formParameterNames.stream(), - request.getMultiValueQueryStringParameters().keySet().stream()).collect(Collectors.toSet())); + + Set 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 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 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 { @@ -370,7 +404,7 @@ public String[] getParameterValues(String s) { @Override public Map getParameterMap() { - return generateParameterMap(request.getMultiValueQueryStringParameters(), config); + return generateParameterMap(request.getMultiValueQueryStringParameters(), config, request.getRequestSource() == RequestSource.ALB); } 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 358d673a1..83c747243 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 @@ -17,7 +17,6 @@ import java.util.Base64; import java.util.List; import java.util.Map; -import java.util.Arrays; public class AwsHttpServletRequestTest { @@ -38,10 +37,14 @@ public class AwsHttpServletRequestTest { .queryString("one", "two").queryString("three", "four").build(); private static final AwsProxyRequest queryStringNullValue = new AwsProxyRequestBuilder("/test", "GET") .queryString("one", "two").queryString("three", null).build(); + private static final AwsProxyRequest queryStringEmptyValue = new AwsProxyRequestBuilder("/test", "GET") + .queryString("one", "two").queryString("three", "").build(); private static final AwsProxyRequest encodedQueryString = new AwsProxyRequestBuilder("/test", "GET") - .queryString("one", "two").queryString("json", "{\"name\":\"faisal\"}").build(); + .queryString("one", "two").queryString("json value@1", "{\"name\":\"faisal\"}").build(); + private static final AwsProxyRequest encodedQueryStringAlb = new AwsProxyRequestBuilder("/test", "GET") + .queryString("one", "two").queryString("json value@1", "{\"name\":\"faisal\"}").alb().build(); private static final AwsProxyRequest multipleParams = new AwsProxyRequestBuilder("/test", "GET") - .queryString("one", "two").queryString("one", "three").queryString("json", "{\"name\":\"faisal\"}").build(); + .queryString("one", "two").queryString("one", "three").queryString("json value@1", "{\"name\":\"faisal\"}").build(); private static final AwsProxyRequest formEncodedAndQueryString = new AwsProxyRequestBuilder("/test", "POST") .queryString("one", "two").queryString("one", "three") .queryString("five", "six") @@ -200,6 +203,20 @@ void queryString_generateQueryString_nullParameterIsEmpty() { assertTrue(parsedString.endsWith("three=")); } + @Test + void queryString_generateQueryString_emptyParameterIsEmpty() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(queryStringEmptyValue, mockContext, null, config); + String parsedString = null; + try { + parsedString = request.generateQueryString(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), true, config.getUriEncoding()); + } catch (ServletException e) { + e.printStackTrace(); + fail("Could not generate query string"); + } + + assertTrue(parsedString.endsWith("three=")); + } + @Test void queryStringWithEncodedParams_generateQueryString_validQuery() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(encodedQueryString, mockContext, null, config); @@ -212,7 +229,23 @@ void queryStringWithEncodedParams_generateQueryString_validQuery() { fail("Could not generate query string"); } assertTrue(parsedString.contains("one=two")); - assertTrue(parsedString.contains("json=%7B%22name%22%3A%22faisal%22%7D")); + assertTrue(parsedString.contains("json+value%401=%7B%22name%22%3A%22faisal%22%7D")); + assertTrue(parsedString.contains("&") && parsedString.indexOf("&") > 0 && parsedString.indexOf("&") < parsedString.length()); + } + + @Test + void queryStringWithEncodedParams_alb_generateQueryString_validQuery() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(encodedQueryStringAlb, mockContext, null, config); + + String parsedString = null; + try { + parsedString = request.generateQueryString(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), false, config.getUriEncoding()); + } catch (ServletException e) { + e.printStackTrace(); + fail("Could not generate query string"); + } + assertTrue(parsedString.contains("one=two")); + assertTrue(parsedString.contains("json+value%401=%7B%22name%22%3A%22faisal%22%7D")); assertTrue(parsedString.contains("&") && parsedString.indexOf("&") > 0 && parsedString.indexOf("&") < parsedString.length()); } @@ -229,7 +262,7 @@ void queryStringWithMultipleValues_generateQueryString_validQuery() { } assertTrue(parsedString.contains("one=two")); assertTrue(parsedString.contains("one=three")); - assertTrue(parsedString.contains("json=%7B%22name%22%3A%22faisal%22%7D")); + assertTrue(parsedString.contains("json+value%401=%7B%22name%22%3A%22faisal%22%7D")); assertTrue(parsedString.contains("&") && parsedString.indexOf("&") > 0 && parsedString.indexOf("&") < parsedString.length()); } @@ -265,6 +298,22 @@ void parameterMap_generateParameterMap_nullParameter() { assertTrue(paramMap.size() == 2); } + @Test + void parameterMap_generateParameterMap_emptyParameter() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(queryStringEmptyValue, 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[]{""}, paramMap.get("three")); + assertTrue(paramMap.size() == 2); + } + @Test void parameterMapWithEncodedParams_generateParameterMap_validQuery() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(encodedQueryString, mockContext, null, config); @@ -278,7 +327,24 @@ void parameterMapWithEncodedParams_generateParameterMap_validQuery() { } assertArrayEquals(new String[]{"two"}, paramMap.get("one")); - assertArrayEquals(new String[]{"{\"name\":\"faisal\"}"}, paramMap.get("json")); + assertArrayEquals(new String[]{"{\"name\":\"faisal\"}"}, paramMap.get("json value@1")); + assertTrue(paramMap.size() == 2); + } + + @Test + void parameterMapWithEncodedParams_alb_generateParameterMap_validQuery() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(encodedQueryStringAlb, mockContext, null, config); + + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config, true); + } 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 value@1")); assertTrue(paramMap.size() == 2); } @@ -294,7 +360,7 @@ void parameterMapWithMultipleValues_generateParameterMap_validQuery() { fail("Could not generate parameter map"); } assertArrayEquals(new String[]{"two", "three"}, paramMap.get("one")); - assertArrayEquals(new String[]{"{\"name\":\"faisal\"}"}, paramMap.get("json")); + assertArrayEquals(new String[]{"{\"name\":\"faisal\"}"}, paramMap.get("json value@1")); assertTrue(paramMap.size() == 2); } @@ -319,12 +385,32 @@ void parameterMap_generateParameterMap_formEncodedAndQueryString() { } @Test - void parameterMap_generateParameterMap_differentCasing() { - AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(differentCasing, mockContext, null, config); + void parameterMap_generateParameterMap_differentCasing_caseSensitive() { + ContainerConfig caseSensitiveConfig = ContainerConfig.defaultConfig(); + caseSensitiveConfig.setQueryStringCaseSensitive(true); + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(differentCasing, mockContext, null, caseSensitiveConfig); + Map paramMap = null; + try { + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), caseSensitiveConfig); + } catch (Exception e) { + e.printStackTrace(); + fail("Could not generate parameter map"); + } + assertArrayEquals(new String[] {"two", "three"}, paramMap.get("one")); + assertArrayEquals(new String[] {"four"}, paramMap.get("ONE")); + assertTrue(paramMap.size() == 2); + } + + @Test + void parameterMap_generateParameterMap_differentCasing_caseInsensitive() { + ContainerConfig caseInsensitiveConfig = ContainerConfig.defaultConfig(); + caseInsensitiveConfig.setQueryStringCaseSensitive(false); + + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(differentCasing, mockContext, null, caseInsensitiveConfig); Map paramMap = null; try { - paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), config); + paramMap = request.generateParameterMap(request.getAwsProxyRequest().getMultiValueQueryStringParameters(), caseInsensitiveConfig); } catch (Exception e) { e.printStackTrace(); fail("Could not generate parameter map"); @@ -346,6 +432,17 @@ void queryParamValues_getQueryParamValues() { assertNull(result2); } + @Test + void queryParamValues_getQueryParamValues_nullValue() { + AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(new AwsProxyRequest(), mockContext, null); + MultiValuedTreeMap map = new MultiValuedTreeMap<>(); + map.add("test", null); + String[] result1 = request.getQueryParamValues(map, "test", true); + assertArrayEquals(new String[] {null}, result1); + String[] result2 = request.getQueryParamValues(map, "TEST", true); + assertNull(result2); + } + @Test void queryParamValues_getQueryParamValues_caseInsensitive() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(new AwsProxyRequest(), mockContext, null); @@ -358,4 +455,17 @@ void queryParamValues_getQueryParamValues_caseInsensitive() { assertArrayEquals(new String[]{"test", "test2"}, result2); } + @Test + void queryParamValues_getQueryParamValues_multipleCaseInsensitive() { + 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[]{"test2"}, result1); + String[] result2 = request.getQueryParamValues(map, "TEST", false); + assertArrayEquals(new String[]{"test2"}, result2); + } + } diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestTest.java index 4edcf5241..736da27b4 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestTest.java @@ -33,7 +33,8 @@ public class AwsProxyHttpServletRequestTest { private static final String FORM_PARAM_NAME = "name"; private static final String FORM_PARAM_NAME_VALUE = "Stef"; private static final String FORM_PARAM_TEST = "test_cookie_param"; - private static final String QUERY_STRING_NAME_VALUE = "Bob"; + private static final String QUERY_STRING_NAME_VALUE = "Bob B!"; + private static final String QUERY_STRING_NAME = "name$"; private static final String REQUEST_SCHEME_HTTP = "http"; private static final String USER_AGENT = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/102.0.5005.61 Safari/537.36"; private static final String REFERER = "https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent/Firefox"; @@ -73,10 +74,9 @@ public class AwsProxyHttpServletRequestTest { } private static final AwsProxyRequestBuilder REQUEST_QUERY = new AwsProxyRequestBuilder("/hello", "POST") - .queryString(FORM_PARAM_NAME, QUERY_STRING_NAME_VALUE); + .queryString(QUERY_STRING_NAME, QUERY_STRING_NAME_VALUE); private static final AwsProxyRequestBuilder REQUEST_QUERY_EMPTY_VALUE = new AwsProxyRequestBuilder("/hello", "POST") - .queryString(FORM_PARAM_NAME, ""); - + .queryString(QUERY_STRING_NAME, ""); public void initAwsProxyHttpServletRequestTest(String type) { requestType = type; @@ -102,7 +102,7 @@ private HttpServletRequest getRequest(AwsProxyRequestBuilder req, Context lambda } } - + @MethodSource("data") @ParameterizedTest void headers_getHeader_validRequest(String type) { @@ -269,7 +269,7 @@ void queryParameters_getParameterMap_nonNull(String type) { HttpServletRequest request = getRequest(REQUEST_QUERY, null, null); assertNotNull(request); assertEquals(1, request.getParameterMap().size()); - assertEquals(QUERY_STRING_NAME_VALUE, request.getParameterMap().get(FORM_PARAM_NAME)[0]); + assertEquals(QUERY_STRING_NAME_VALUE, request.getParameterMap().get(QUERY_STRING_NAME)[0]); } @MethodSource("data") @@ -279,7 +279,7 @@ void queryParameters_getParameterMap_nonNull_EmptyParamValue(String type) { HttpServletRequest request = getRequest(REQUEST_QUERY_EMPTY_VALUE, null, null); assertNotNull(request); assertEquals(1, request.getParameterMap().size()); - assertEquals("", request.getParameterMap().get(FORM_PARAM_NAME)[0]); + assertEquals("", request.getParameterMap().get(QUERY_STRING_NAME)[0]); } @MethodSource("data") @@ -300,7 +300,7 @@ void queryParameters_getParameterNames_notNull(String type) { List parameterNames = Collections.list(request.getParameterNames()); assertNotNull(request); assertEquals(1, parameterNames.size()); - assertTrue(parameterNames.contains(FORM_PARAM_NAME)); + assertTrue(parameterNames.contains(QUERY_STRING_NAME)); } @MethodSource("data") @@ -470,7 +470,6 @@ void requestURL_getUrlWithContextPath_expectStageAsContextPath(String type) { LambdaContainerHandler.getContainerConfig().setUseStageAsServletContext(true); HttpServletRequest servletRequest = getRequest(req, null, null); String requestUrl = servletRequest.getRequestURL().toString(); - System.out.println(requestUrl); assertTrue(requestUrl.contains("/test-stage/")); LambdaContainerHandler.getContainerConfig().setUseStageAsServletContext(false); } diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilder.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilder.java index 40f0ae7ad..e42130453 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilder.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilder.java @@ -16,6 +16,7 @@ import com.amazonaws.serverless.proxy.model.*; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.commons.io.IOUtils; import org.apache.hc.core5.http.ContentType; @@ -49,7 +50,7 @@ public class AwsProxyRequestBuilder { private AwsProxyRequest request; private MultipartEntityBuilder multipartBuilder; - + //------------------------------------------------------------- // Constructors //------------------------------------------------------------- @@ -86,22 +87,41 @@ public AwsProxyRequestBuilder(String path, String httpMethod) { this.request.getRequestContext().setIdentity(identity); } - - //------------------------------------------------------------- + //------------------------------------------------------------- // Methods - Public //------------------------------------------------------------- public AwsProxyRequestBuilder alb() { - this.request.setRequestContext(new AwsProxyRequestContext()); - this.request.getRequestContext().setElb(new AlbContext()); - this.request.getRequestContext().getElb().setTargetGroupArn( + /* + * This method sets up the requestContext to look like an ALB request and also + * re-encodes URL query params, since ALBs do not decode them. This now returns + * a new AwsProxyRequestBuilder with the new query param state, so the original + * builder maintains the original configured state and can be then be reused in + * further unit tests. For now the simplest way to accomplish a deep copy is by + * serializing to JSON then deserializing. + */ + + ObjectMapper objectMapper = new ObjectMapper(); + AwsProxyRequest albRequest = null; + try { + String json = objectMapper.writeValueAsString(this.request); + albRequest = objectMapper.readValue(json, AwsProxyRequest.class); + } catch (JsonProcessingException jpe) { + throw new RuntimeException(jpe); + } + + if (albRequest.getRequestContext() == null) { + albRequest.setRequestContext(new AwsProxyRequestContext()); + } + albRequest.getRequestContext().setElb(new AlbContext()); + albRequest.getRequestContext().getElb().setTargetGroupArn( "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/lambda-target/d6190d154bc908a5" ); // ALB does not decode query string parameters so we re-encode them all - if (request.getMultiValueQueryStringParameters() != null) { + if (albRequest.getMultiValueQueryStringParameters() != null) { MultiValuedTreeMap newQs = new MultiValuedTreeMap<>(); - for (Map.Entry> e : request.getMultiValueQueryStringParameters().entrySet()) { + for (Map.Entry> e : albRequest.getMultiValueQueryStringParameters().entrySet()) { for (String v : e.getValue()) { try { // this is a terrible hack. In our Spring tests we use the comma as a control character for lists @@ -114,9 +134,9 @@ public AwsProxyRequestBuilder alb() { } } } - request.setMultiValueQueryStringParameters(newQs); + albRequest.setMultiValueQueryStringParameters(newQs); } - return this; + return new AwsProxyRequestBuilder(albRequest); } public AwsProxyRequestBuilder stage(String stageName) { @@ -142,6 +162,9 @@ public AwsProxyRequestBuilder json() { public AwsProxyRequestBuilder form(String key, String value) { + if (key == null || value == null) { + throw new IllegalArgumentException("form() does not support null key or value"); + } if (request.getMultiValueHeaders() == null) { request.setMultiValueHeaders(new Headers()); } @@ -150,7 +173,12 @@ public AwsProxyRequestBuilder form(String key, String value) { if (body == null) { body = ""; } - body += (body.equals("")?"":"&") + key + "=" + value; + // URL-encode key and value to form expected body of a form post + try { + body += (body.equals("") ? "" : "&") + URLEncoder.encode(key, "UTF-8") + "=" + URLEncoder.encode(value, "UTF-8"); + } catch (UnsupportedEncodingException ex) { + throw new RuntimeException("Could not encode form parameter: " + key + "=" + value, ex); + } request.setBody(body); return this; } @@ -214,35 +242,15 @@ public AwsProxyRequestBuilder multiValueQueryString(MultiValuedTreeMap()); } - if (request.getRequestSource() == RequestSource.API_GATEWAY) { - this.request.getMultiValueQueryStringParameters().add(key, value); - } - // ALB does not decode parameters automatically like API Gateway. - if (request.getRequestSource() == RequestSource.ALB) { - try { - //if (URLDecoder.decode(value, ContainerConfig.DEFAULT_CONTENT_CHARSET).equals(value)) { - // TODO: Assume we are always given an unencoded value, smarter check here to encode - // only if necessary - this.request.getMultiValueQueryStringParameters().add( - key, - URLEncoder.encode(value, ContainerConfig.DEFAULT_CONTENT_CHARSET) - ); - //} - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } - - } + this.request.getMultiValueQueryStringParameters().add(key, value); return this; } - public AwsProxyRequestBuilder body(String body) { this.request.setBody(body); return this; @@ -475,11 +483,11 @@ public HttpApiV2ProxyRequest toHttpApiV2Request() { request.getMultiValueQueryStringParameters().forEach((k, v) -> { for (String s : v) { rawQueryString.append("&"); - rawQueryString.append(k); - rawQueryString.append("="); try { // same terrible hack as the alb() method. Because our spring tests use commas as control characters // we do not encode it + rawQueryString.append(URLEncoder.encode(k, "UTF-8").replaceAll("%2C", ",")); + rawQueryString.append("="); rawQueryString.append(URLEncoder.encode(s, "UTF-8").replaceAll("%2C", ",")); } catch (UnsupportedEncodingException e) { throw new RuntimeException(e); diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilderTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilderTest.java new file mode 100644 index 000000000..b851e31dc --- /dev/null +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilderTest.java @@ -0,0 +1,157 @@ +package com.amazonaws.serverless.proxy.internal.testutils; + +import java.io.IOException; +import java.net.URLEncoder; +import jakarta.ws.rs.core.HttpHeaders; +import jakarta.ws.rs.core.MediaType; + +import com.amazonaws.serverless.proxy.model.*; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public class AwsProxyRequestBuilderTest { + + private static final String TEST_KEY = "testkey"; + private static final String TEST_VALUE = "testvalue"; + private static final String TEST_KEY_FOR_ENCODING = "test@key 1"; + private static final String TEST_VALUE_FOR_ENCODING = "test value!!"; + + + void baseConstructorAsserts(AwsProxyRequest request) { + assertEquals(0, request.getMultiValueHeaders().size()); + assertEquals(0, request.getHeaders().size()); + assertEquals(0, request.getMultiValueQueryStringParameters().size()); + assertNotNull(request.getRequestContext()); + assertNotNull(request.getRequestContext().getRequestId()); + assertNotNull(request.getRequestContext().getExtendedRequestId()); + assertEquals("test", request.getRequestContext().getStage()); + assertEquals("HTTP/1.1", request.getRequestContext().getProtocol()); + assertNotNull(request.getRequestContext().getRequestTimeEpoch()); + assertNotNull(request.getRequestContext().getIdentity()); + assertEquals("127.0.0.1", request.getRequestContext().getIdentity().getSourceIp()); + } + + @Test + void constructor_path_httpMethod() { + + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "GET"); + AwsProxyRequest request = builder.build(); + assertEquals("/path", request.getPath()); + assertEquals("GET", request.getHttpMethod()); + baseConstructorAsserts(request); + } + + @Test + void constructor_path_nullHttpMethod() { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path"); + AwsProxyRequest request = builder.build(); + assertNull(request.getHttpMethod()); + assertEquals("/path", request.getPath()); + baseConstructorAsserts(request); + } + + @Test + void constructor_nullPath_nullHttpMethod() { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder(); + AwsProxyRequest request = builder.build(); + assertNull(request.getHttpMethod()); + assertNull(request.getPath()); + baseConstructorAsserts(request); + } + + @Test + void form_key_value() { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "POST"); + builder.form(TEST_KEY, TEST_VALUE); + AwsProxyRequest request = builder.build(); + assertEquals(1, request.getMultiValueHeaders().get(HttpHeaders.CONTENT_TYPE).size()); + assertEquals(MediaType.APPLICATION_FORM_URLENCODED, request.getMultiValueHeaders().getFirst(HttpHeaders.CONTENT_TYPE)); + assertNull(request.getHeaders().get(HttpHeaders.CONTENT_TYPE)); + assertNotNull(request.getBody()); + assertEquals(TEST_KEY + "=" + TEST_VALUE, request.getBody()); + } + + @Test + void form_key_nullKey_nullValue() { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "POST"); + assertThrows(IllegalArgumentException.class, () -> builder.form(null, TEST_VALUE)); + assertThrows(IllegalArgumentException.class, () -> builder.form(TEST_KEY, null)); + assertThrows(IllegalArgumentException.class, () -> builder.form(null, null)); + } + + @Test + void form_keyEncoded_valueEncoded() throws IOException { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "POST"); + builder.form(TEST_KEY_FOR_ENCODING, TEST_VALUE_FOR_ENCODING); + AwsProxyRequest request = builder.build(); + + assertEquals(1, request.getMultiValueHeaders().get(HttpHeaders.CONTENT_TYPE).size()); + assertEquals(MediaType.APPLICATION_FORM_URLENCODED, request.getMultiValueHeaders().getFirst(HttpHeaders.CONTENT_TYPE)); + assertNull(request.getHeaders().get(HttpHeaders.CONTENT_TYPE)); + assertNotNull(request.getBody()); + String expected = URLEncoder.encode(TEST_KEY_FOR_ENCODING, "UTF-8") + "=" + URLEncoder.encode(TEST_VALUE_FOR_ENCODING, "UTF-8"); + assertEquals(expected, request.getBody()); + } + + @Test + void queryString_key_value() { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "POST"); + builder.queryString(TEST_KEY, TEST_VALUE); + AwsProxyRequest request = builder.build(); + + assertNull(request.getQueryStringParameters()); + assertEquals(1, request.getMultiValueQueryStringParameters().size()); + assertEquals(TEST_KEY, request.getMultiValueQueryStringParameters().keySet().iterator().next()); + assertEquals(TEST_VALUE, request.getMultiValueQueryStringParameters().get(TEST_KEY).get(0)); + assertEquals(TEST_VALUE, request.getMultiValueQueryStringParameters().getFirst(TEST_KEY)); + } + + @Test + void queryString_keyNotEncoded_valueNotEncoded() { + // builder should not URL encode key or value for query string + // in the case of an ALB where values should be encoded, the builder alb() method will handle it + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "POST"); + builder.queryString(TEST_KEY_FOR_ENCODING, TEST_VALUE_FOR_ENCODING); + AwsProxyRequest request = builder.build(); + + assertNull(request.getQueryStringParameters()); + assertEquals(1, request.getMultiValueQueryStringParameters().size()); + assertEquals(TEST_KEY_FOR_ENCODING, request.getMultiValueQueryStringParameters().keySet().iterator().next()); + assertEquals(TEST_VALUE_FOR_ENCODING, request.getMultiValueQueryStringParameters().get(TEST_KEY_FOR_ENCODING).get(0)); + assertEquals(TEST_VALUE_FOR_ENCODING, request.getMultiValueQueryStringParameters().getFirst(TEST_KEY_FOR_ENCODING)); + } + + @Test + void queryString_alb_key_value() { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "POST"); + builder.queryString(TEST_KEY, TEST_VALUE); + AwsProxyRequest request = builder.alb().build(); + + assertNull(request.getQueryStringParameters()); + assertEquals(1, request.getMultiValueQueryStringParameters().size()); + assertEquals(TEST_KEY, request.getMultiValueQueryStringParameters().keySet().iterator().next()); + assertEquals(TEST_VALUE, request.getMultiValueQueryStringParameters().get(TEST_KEY).get(0)); + assertEquals(TEST_VALUE, request.getMultiValueQueryStringParameters().getFirst(TEST_KEY)); + } + + @Test + void alb_keyEncoded_valueEncoded() throws IOException { + AwsProxyRequestBuilder builder = new AwsProxyRequestBuilder("/path", "POST"); + MultiValuedTreeMap map = new MultiValuedTreeMap<>(); + map.add(TEST_KEY_FOR_ENCODING, TEST_VALUE_FOR_ENCODING); + builder.multiValueQueryString(map); + AwsProxyRequest request = builder.alb().build(); + + String expectedKey = URLEncoder.encode(TEST_KEY_FOR_ENCODING, "UTF-8"); + String expectedValue = URLEncoder.encode(TEST_VALUE_FOR_ENCODING, "UTF-8"); + assertEquals(1, request.getMultiValueQueryStringParameters().size()); + assertEquals(expectedKey, request.getMultiValueQueryStringParameters().keySet().iterator().next()); + assertEquals(expectedValue, request.getMultiValueQueryStringParameters().get(expectedKey).get(0)); + assertEquals(expectedValue, request.getMultiValueQueryStringParameters().getFirst(expectedKey)); + assertEquals(expectedKey, request.getMultiValueQueryStringParameters().keySet().iterator().next()); + assertEquals(expectedValue, request.getMultiValueQueryStringParameters().get(expectedKey).get(0)); + assertEquals(expectedValue, request.getMultiValueQueryStringParameters().getFirst(expectedKey)); + } + +}