From 45df2b0aec44c44f96ccfa5649e59ea4e0c01f90 Mon Sep 17 00:00:00 2001 From: Jay Fennelly Date: Sun, 27 Oct 2024 19:11:20 -0700 Subject: [PATCH 1/8] Fix AwsProxyRequestBuilder to be immutable when calling the alb() method --- .../testutils/AwsProxyRequestBuilder.java | 76 +++++---- .../testutils/AwsProxyRequestBuilderTest.java | 157 ++++++++++++++++++ 2 files changed, 199 insertions(+), 34 deletions(-) create mode 100644 aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilderTest.java 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..4d1bea427 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..6263eeaf9 --- /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)); + } + +} From 3ad77463f5f0504812fe061e7fd40b62051ea694 Mon Sep 17 00:00:00 2001 From: Jay Fennelly Date: Sun, 27 Oct 2024 19:27:14 -0700 Subject: [PATCH 2/8] Clean up and add additional tests --- .../servlet/AwsHttpServletRequestTest.java | 101 +++++++++++++++++- .../AwsProxyHttpServletRequestTest.java | 15 ++- 2 files changed, 104 insertions(+), 12 deletions(-) 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..44d767440 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,8 +37,12 @@ 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(); + private static final AwsProxyRequest encodedQueryStringAlb = new AwsProxyRequestBuilder("/test", "GET") + .queryString("one", "two").queryString("json", "{\"name\":\"faisal\"}").alb().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") @@ -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); @@ -216,6 +233,22 @@ void queryStringWithEncodedParams_generateQueryString_validQuery() { 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=%7B%22name%22%3A%22faisal%22%7D")); + assertTrue(parsedString.contains("&") && parsedString.indexOf("&") > 0 && parsedString.indexOf("&") < parsedString.length()); + } + @Test void queryStringWithMultipleValues_generateQueryString_validQuery() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(multipleParams, mockContext, null, config); @@ -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); @@ -319,12 +368,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 +415,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 +438,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..a06066490 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 @@ -34,6 +34,7 @@ public class AwsProxyHttpServletRequestTest { 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 = "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); } From 7c05a6f6e63e43578b968294adda576c40329ccd Mon Sep 17 00:00:00 2001 From: Jay Fennelly Date: Sun, 27 Oct 2024 19:37:15 -0700 Subject: [PATCH 3/8] Allow generateParameterMap to optionally decode query params (for ALB requests) --- .../servlet/AwsHttpServletRequest.java | 68 ++++++++++++------- .../servlet/AwsHttpServletRequestTest.java | 17 +++++ 2 files changed, 60 insertions(+), 25 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 ea8ef4a1a..84e4f96c5 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 @@ -584,7 +584,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]); @@ -607,39 +607,57 @@ 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().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/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 44d767440..179b58a1e 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 @@ -331,6 +331,23 @@ void parameterMapWithEncodedParams_generateParameterMap_validQuery() { 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")); + assertTrue(paramMap.size() == 2); + } + @Test void parameterMapWithMultipleValues_generateParameterMap_validQuery() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(multipleParams, mockContext, null, config); From 916f2a25e6c0380a0bfbe7f3a027b4e4c9a49b21 Mon Sep 17 00:00:00 2001 From: Jay Fennelly Date: Sun, 27 Oct 2024 19:39:39 -0700 Subject: [PATCH 4/8] Update unit tests to test decoding of query param names --- .../servlet/AwsHttpServletRequestTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 179b58a1e..904a35508 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 @@ -40,11 +40,11 @@ public class AwsHttpServletRequestTest { 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", "{\"name\":\"faisal\"}").alb().build(); + .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") @@ -229,7 +229,7 @@ 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()); } @@ -245,7 +245,7 @@ void queryStringWithEncodedParams_alb_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()); } @@ -262,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()); } @@ -327,7 +327,7 @@ 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); } @@ -344,7 +344,7 @@ void parameterMapWithEncodedParams_alb_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); } @@ -360,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); } From f68e7552398816faa6b7a3e80a9373c27882387a Mon Sep 17 00:00:00 2001 From: Jay Fennelly Date: Sun, 27 Oct 2024 19:43:13 -0700 Subject: [PATCH 5/8] Add handling decoding of parameters for ALB requests and decode paramter names in HTTP API requests --- .../AwsHttpApiV2ProxyHttpServletRequest.java | 4 +-- .../servlet/AwsProxyHttpServletRequest.java | 35 +++++++++++++++++-- .../AwsProxyHttpServletRequestTest.java | 4 +-- 3 files changed, 36 insertions(+), 7 deletions(-) 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 537e10759..1caf6dace 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 @@ -483,10 +483,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/AwsProxyHttpServletRequest.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java index fe514e65d..c10b1ef5e 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 @@ -325,8 +325,17 @@ public String getContentType() { @Override public String getParameter(String s) { + + // 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; } @@ -345,15 +354,35 @@ 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) { + + // decode key if ALB + if (request.getRequestSource() == RequestSource.ALB) { + s = decodeValueIfEncoded(s); + } + + // TODO lots of back and forth arrays and lists here, sort it out! List values = new ArrayList<>(Arrays.asList(getQueryParamValues(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()))); + // List values = getQueryParamValuesAsList(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()); + + // decode values if ALB + if (request.getRequestSource() == RequestSource.ALB) { + values = values.stream().map(AwsHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toList()); + } values.addAll(Arrays.asList(getFormBodyParameterCaseInsensitive(s))); @@ -367,7 +396,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/AwsProxyHttpServletRequestTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestTest.java index a06066490..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,8 +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 = "name"; + 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"; From 1109c8fe9da414fb998c8e715cb5e92e5644ca85 Mon Sep 17 00:00:00 2001 From: Jay Fennelly Date: Mon, 25 Nov 2024 10:52:28 -0800 Subject: [PATCH 6/8] Convert tabs to spaces --- .../servlet/AwsHttpServletRequest.java | 34 +-- .../servlet/AwsProxyHttpServletRequest.java | 61 ++-- .../servlet/AwsHttpServletRequestTest.java | 10 +- .../testutils/AwsProxyRequestBuilder.java | 38 +-- .../testutils/AwsProxyRequestBuilderTest.java | 282 +++++++++--------- 5 files changed, 213 insertions(+), 212 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 84e4f96c5..e7df944ff 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 @@ -607,7 +607,7 @@ protected List getQueryParamValuesAsList(MultiValuedTreeMap generateParameterMap(MultiValuedTreeMap qs, ContainerConfig config) { - return generateParameterMap(qs, config, false); + return generateParameterMap(qs, config, false); } protected Map generateParameterMap(MultiValuedTreeMap qs, ContainerConfig config, boolean decodeQueryParams) { @@ -621,22 +621,22 @@ protected Map generateParameterMap(MultiValuedTreeMap e.getValue().toArray(new String[0]))); } - // 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; + // 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 { 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 c10b1ef5e..188073841 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 @@ -326,25 +326,25 @@ public String getContentType() { @Override public String getParameter(String s) { - // decode key if ALB - if (request.getRequestSource() == RequestSource.ALB) { - s = decodeValueIfEncoded(s); - } + // 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; - } + 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]; - } + String[] bodyParams = getFormBodyParameterCaseInsensitive(s); + if (bodyParams.length == 0) { + return null; + } else { + return bodyParams[0]; + } } @@ -357,40 +357,41 @@ public Enumeration getParameterNames() { Set paramNames = request.getMultiValueQueryStringParameters().keySet(); if (request.getRequestSource() == RequestSource.ALB) { - paramNames = paramNames.stream().map(AwsProxyHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toSet()); + paramNames = paramNames.stream().map(AwsProxyHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toSet()); } - + return Collections.enumeration( - Stream.concat(formParameterNames.stream(), paramNames.stream()) - .collect(Collectors.toSet())); + 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) { - - // decode key if ALB - if (request.getRequestSource() == RequestSource.ALB) { - s = decodeValueIfEncoded(s); - } - - // TODO lots of back and forth arrays and lists here, sort it out! + + // TODO lots of back and forth arrays and lists here, sort it out! List values = new ArrayList<>(Arrays.asList(getQueryParamValues(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()))); // List values = getQueryParamValuesAsList(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()); + // decode key if ALB + if (request.getRequestSource() == RequestSource.ALB) { + s = decodeValueIfEncoded(s); + } + // decode values if ALB - if (request.getRequestSource() == RequestSource.ALB) { - values = values.stream().map(AwsHttpServletRequest::decodeValueIfEncoded).collect(Collectors.toList()); + 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 { return values.toArray(new String[0]); } + } 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 904a35508..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 @@ -386,9 +386,9 @@ void parameterMap_generateParameterMap_formEncodedAndQueryString() { @Test void parameterMap_generateParameterMap_differentCasing_caseSensitive() { - ContainerConfig caseSensitiveConfig = ContainerConfig.defaultConfig(); - caseSensitiveConfig.setQueryStringCaseSensitive(true); - AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(differentCasing, mockContext, null, caseSensitiveConfig); + 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); @@ -403,8 +403,8 @@ void parameterMap_generateParameterMap_differentCasing_caseSensitive() { @Test void parameterMap_generateParameterMap_differentCasing_caseInsensitive() { - ContainerConfig caseInsensitiveConfig = ContainerConfig.defaultConfig(); - caseInsensitiveConfig.setQueryStringCaseSensitive(false); + ContainerConfig caseInsensitiveConfig = ContainerConfig.defaultConfig(); + caseInsensitiveConfig.setQueryStringCaseSensitive(false); AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(differentCasing, mockContext, null, caseInsensitiveConfig); 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 4d1bea427..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 @@ -92,26 +92,26 @@ public AwsProxyRequestBuilder(String path, String httpMethod) { //------------------------------------------------------------- public AwsProxyRequestBuilder alb() { - /* - * 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); - } - + /* + * 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.setRequestContext(new AwsProxyRequestContext()); } albRequest.getRequestContext().setElb(new AlbContext()); albRequest.getRequestContext().getElb().setTargetGroupArn( 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 index 6263eeaf9..b851e31dc 100644 --- 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 @@ -12,146 +12,146 @@ 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)); - } + 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)); + } } From a202873b939895df6eb023908325daab96cfed2f Mon Sep 17 00:00:00 2001 From: Jay Fennelly Date: Mon, 25 Nov 2024 12:10:56 -0800 Subject: [PATCH 7/8] Improve object creation and conversion between lists and arrays --- .../servlet/AwsHttpApiV2ProxyHttpServletRequest.java | 10 +++++++++- .../internal/servlet/AwsProxyHttpServletRequest.java | 12 ++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) 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 1caf6dace..5af7a9dc0 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 @@ -298,8 +298,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) { 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 188073841..bd3e0df58 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 @@ -370,14 +370,19 @@ public Enumeration getParameterNames() { @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) { - // TODO lots of back and forth arrays and lists here, sort it out! - List values = new ArrayList<>(Arrays.asList(getQueryParamValues(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()))); - // List values = getQueryParamValuesAsList(request.getMultiValueQueryStringParameters(), s, config.isQueryStringCaseSensitive()); // decode key if ALB if (request.getRequestSource() == RequestSource.ALB) { s = decodeValueIfEncoded(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) { @@ -391,7 +396,6 @@ public String[] getParameterValues(String s) { } else { return values.toArray(new String[0]); } - } From 1785eb57c7d72f6c1b8468f2e3ad13ddb8298589 Mon Sep 17 00:00:00 2001 From: Dennis Kieselhorst Date: Wed, 27 Nov 2024 08:39:32 +0100 Subject: [PATCH 8/8] bring back parallel() processing --- .../proxy/internal/servlet/AwsHttpServletRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e7df944ff..f6a06b7cf 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 @@ -641,7 +641,7 @@ protected Map generateParameterMap(MultiValuedTreeMap getQueryParamValuesAsList(decodedQs, e.getKey(), false)