From 5cf97f98b3022a3691f08548bfc149d5e481015d Mon Sep 17 00:00:00 2001 From: sapessi Date: Fri, 22 Jun 2018 09:45:27 -0700 Subject: [PATCH] Changes to query string handling to address #162. parameter values come from request decoded. the getQueryString return an encoded string. --- .../servlet/AwsHttpServletRequest.java | 29 +++++- .../servlet/AwsProxyHttpServletRequest.java | 68 ++++++++------ .../EncodingQueryStringParameterMap.java | 89 ------------------- .../servlet/AwsHttpServletRequestTest.java | 17 +++- .../proxy/spring/SpringAwsProxyTest.java | 13 +++ .../proxy/spring/SpringBootAppTest.java | 10 +++ .../proxy/spring/echoapp/EchoResource.java | 8 ++ .../spring/springbootapp/TestApplication.java | 4 + .../spring/springbootapp/TestController.java | 12 +++ 9 files changed, 127 insertions(+), 123 deletions(-) delete mode 100644 aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/EncodingQueryStringParameterMap.java 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 7c74772da..b10bf9f3f 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 @@ -24,6 +24,7 @@ import javax.servlet.AsyncContext; import javax.servlet.DispatcherType; import javax.servlet.ServletContext; +import javax.servlet.ServletException; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; @@ -288,9 +289,12 @@ protected Cookie[] parseCookieHeaderValue(String headerValue) { * Given a map of key/values query string parameters from API Gateway, creates a query string as it would have * been in the original url. * @param parameters A Map<String, String> of query string parameters + * @param encode Whether the key and values should be URL encoded + * @param encodeCharset Charset to use for encoding the query string * @return The generated query string for the URI */ - protected String generateQueryString(EncodingQueryStringParameterMap parameters) { + protected String generateQueryString(Map parameters, boolean encode, String encodeCharset) + throws ServletException { if (parameters == null || parameters.size() == 0) { return null; } @@ -300,12 +304,31 @@ protected String generateQueryString(EncodingQueryStringParameterMap parameters) StringBuilder queryStringBuilder = new StringBuilder(); - parameters.keySet().stream().forEach(k -> parameters.get(k).stream().forEach(v -> { + /*parameters.keySet().stream().forEach(k -> parameters.stream().forEach(v -> { queryStringBuilder.append("&"); queryStringBuilder.append(k); queryStringBuilder.append("="); queryStringBuilder.append(v); - })); + }));*/ + try { + for (Map.Entry e : parameters.entrySet()) { + queryStringBuilder.append("&"); + if (encode) { + queryStringBuilder.append(URLEncoder.encode(e.getKey(), encodeCharset)); + } else { + queryStringBuilder.append(e.getKey()); + } + queryStringBuilder.append("="); + if (encode) { + queryStringBuilder.append(URLEncoder.encode(e.getValue(), encodeCharset)); + } else { + queryStringBuilder.append(e.getValue()); + } + + } + } catch (UnsupportedEncodingException e) { + throw new ServletException("Invalid charset passed for query string encoding", e); + } queryString = queryStringBuilder.toString(); queryString = queryString.substring(1); // remove the first & - faster to do it here than adding logic in the Lambda 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 82b3624d9..3c474d9cd 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 @@ -87,7 +87,6 @@ public class AwsProxyHttpServletRequest extends AwsHttpServletRequest { private Map> urlEncodedFormParameters; private Map multipartFormParameters; private Map caseInsensitiveHeaders; - private EncodingQueryStringParameterMap queryStringParameters; private static Logger log = LoggerFactory.getLogger(AwsProxyHttpServletRequest.class); private ContainerConfig config; @@ -107,9 +106,6 @@ public AwsProxyHttpServletRequest(AwsProxyRequest awsProxyRequest, Context lambd this.securityContext = awsSecurityContext; this.config = config; - this.queryStringParameters = new EncodingQueryStringParameterMap(config.isQueryStringCaseSensitive(), config.getUriEncoding()); - this.queryStringParameters.putAllMapEncoding(request.getQueryStringParameters()); - this.caseInsensitiveHeaders = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); this.caseInsensitiveHeaders.putAll(awsProxyRequest.getHeaders()); } @@ -230,7 +226,12 @@ public String getContextPath() { @Override public String getQueryString() { - return this.generateQueryString(queryStringParameters); + try { + return this.generateQueryString(request.getQueryStringParameters(), true, config.getUriEncoding()); + } catch (ServletException e) { + log.error("Could not generate query string", e); + return null; + } } @@ -441,11 +442,7 @@ public ServletInputStream getInputStream() @Override public String getParameter(String s) { - String paramKey = s; - if (config.isQueryStringCaseSensitive()) { - paramKey = paramKey.toLowerCase(Locale.getDefault()); - } - String queryStringParameter = queryStringParameters.getFirst(paramKey); + String queryStringParameter = getQueryParamValue(s, config.isQueryStringCaseSensitive()); if (queryStringParameter != null) { return queryStringParameter; } @@ -462,7 +459,9 @@ public String getParameter(String s) { @Override public Enumeration getParameterNames() { List paramNames = new ArrayList<>(); - paramNames.addAll(queryStringParameters.keySet()); + if (request.getQueryStringParameters() != null) { + paramNames.addAll(request.getQueryStringParameters().keySet()); + } paramNames.addAll(getFormUrlEncodedParametersMap().keySet()); return Collections.enumeration(paramNames); } @@ -471,16 +470,11 @@ 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) { - String paramKey = s; - if (config.isQueryStringCaseSensitive()) { - paramKey = paramKey.toLowerCase(Locale.getDefault()); - } List values = new ArrayList<>(); - List queryParamValues = queryStringParameters.get(paramKey); - if (queryParamValues != null) { - values.addAll(queryParamValues); + String queryValue = getQueryParamValue(s, config.isQueryStringCaseSensitive()); + if (queryValue != null) { + values.add(queryValue); } - //values.addAll(queryStringParameters.get(paramKey)); String[] formBodyValues = getFormBodyParameterCaseInsensitive(s); if (formBodyValues != null) { @@ -504,9 +498,17 @@ public Map getParameterMap() { output.put(e.getKey(), e.getValue().toArray(new String[0])); }); - queryStringParameters.keySet().stream().parallel().forEach(e -> { - output.put(e, queryStringParameters.get(e).toArray(new String[0])); - }); + if (request.getQueryStringParameters() != null) { + request.getQueryStringParameters().keySet().stream().parallel().forEach(e -> { + List newValues = new ArrayList<>(); + if (output.containsKey(e)) { + String[] values = output.get(e); + newValues.addAll(Arrays.asList(values)); + } + newValues.add(getQueryParamValue(e, config.isQueryStringCaseSensitive())); + output.put(e, newValues.toArray(new String[0])); + }); + } return output; } @@ -649,13 +651,6 @@ public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse se return null; } - //------------------------------------------------------------- - // Methods - Protected - //------------------------------------------------------------- - - protected EncodingQueryStringParameterMap getQueryParametersMap() { - return queryStringParameters; - } //------------------------------------------------------------- // Methods - Private @@ -816,6 +811,21 @@ public static String decodeValueIfEncoded(String value) { } + private String getQueryParamValue(String key, boolean isCaseSensitive) { + if (isCaseSensitive) { + return request.getQueryStringParameters().get(key); + } + + for (String k : request.getQueryStringParameters().keySet()) { + if (k.toLowerCase(Locale.getDefault()).equals(key.toLowerCase(Locale.getDefault()))) { + return request.getQueryStringParameters().get(k); + } + } + + return null; + } + + public static class AwsServletInputStream extends ServletInputStream { private InputStream bodyStream; diff --git a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/EncodingQueryStringParameterMap.java b/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/EncodingQueryStringParameterMap.java deleted file mode 100644 index 31c0b65d0..000000000 --- a/aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/EncodingQueryStringParameterMap.java +++ /dev/null @@ -1,89 +0,0 @@ -package com.amazonaws.serverless.proxy.internal.servlet; - - -import com.amazonaws.serverless.proxy.internal.SecurityUtils; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.ws.rs.core.MultivaluedHashMap; - -import java.io.UnsupportedEncodingException; -import java.net.URLEncoder; -import java.util.ArrayList; -import java.util.List; -import java.util.Locale; -import java.util.Map; - - -/** - * query string parameters container. The main purpose of this object is to apply the required transformation to query - * string parameters coming from API Gateway. Specifically, parameters names and values need to be un-escaped and url-encoded - * again. - */ -public class EncodingQueryStringParameterMap extends MultivaluedHashMap { - - private boolean isCaseSensitive; - private String encoding; - private static Logger log = LoggerFactory.getLogger(EncodingQueryStringParameterMap.class); - - private static final long serialVersionUID = 42L; - - /** - * Creates a new instance of the parameters map. This allows the configuration to specify whether query string parameter - * names should be case sensitive or not. - * @param caseSensitive Whether parameters should be case sensitive. If the value is true, parameters names - * are automatically trnasformed to lower case as they are added to the map. - */ - public EncodingQueryStringParameterMap(final boolean caseSensitive, final String enc) { - isCaseSensitive = caseSensitive; - encoding = enc; - } - - public void putAllMapEncoding(final Map parametersMap) { - if (parametersMap == null) { - return; - } - parametersMap.entrySet().stream().forEach(e -> { - String key = e.getKey(); - if (!isCaseSensitive) { - key = key.toLowerCase(Locale.getDefault()); - } - key = unescapeAndEncode(key); - - String value = unescapeAndEncode(e.getValue()); - putSingle(key, value); - }); - } - - public void putAllMultiValuedMapEncoding(final MultivaluedHashMap parametersMap) { - if (parametersMap == null) { - return; - } - parametersMap.entrySet().stream().forEach(e -> { - String key = e.getKey(); - if (!isCaseSensitive) { - key = key.toLowerCase(Locale.getDefault()); - } - key = unescapeAndEncode(key); - - List newValueList = new ArrayList(); - // we don't expect the values to be many so we don't parallel() - e.getValue().stream().forEach(v -> { - newValueList.add(unescapeAndEncode(v)); - }); - - put(key, newValueList); - }); - } - - private String unescapeAndEncode(final String value) { - try { - return URLEncoder.encode(value, encoding); - } catch (UnsupportedEncodingException e) { - log.error("Could not url encode parameter value: " + SecurityUtils.crlf(value), e); - } - - return null; - } -} 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 976f8bfc8..4417f37b2 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 @@ -9,6 +9,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.Test; +import javax.servlet.ServletException; import javax.ws.rs.core.HttpHeaders; import static org.junit.Assert.*; @@ -77,7 +78,13 @@ public void headers_parseHeaderValue_complexAccept() { public void queryString_generateQueryString_validQuery() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(queryString, mockContext, null, config); - String parsedString = request.generateQueryString(request.getQueryParametersMap()); + String parsedString = null; + try { + parsedString = request.generateQueryString(request.getAwsProxyRequest().getQueryStringParameters(), true, config.getUriEncoding()); + } catch (ServletException e) { + e.printStackTrace(); + fail("Could not generate query string"); + } System.out.println(parsedString); assertTrue(parsedString.contains("one=two")); assertTrue(parsedString.contains("three=four")); @@ -88,7 +95,13 @@ public void queryString_generateQueryString_validQuery() { public void queryStringWithEncodedParams_generateQueryString_validQuery() { AwsProxyHttpServletRequest request = new AwsProxyHttpServletRequest(encodedQueryString, mockContext, null, config); - String parsedString = request.generateQueryString(request.getQueryParametersMap()); + String parsedString = null; + try { + parsedString = request.generateQueryString(request.getAwsProxyRequest().getQueryStringParameters(), true, 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()); diff --git a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringAwsProxyTest.java b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringAwsProxyTest.java index ca6844855..6e7280e93 100644 --- a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringAwsProxyTest.java +++ b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringAwsProxyTest.java @@ -116,6 +116,19 @@ public void queryString_uriInfo_echo() { validateMapResponseModel(output); } + @Test + public void queryString_listParameter_expectCorrectLength() { + AwsProxyRequest request = new AwsProxyRequestBuilder("/echo/list-query-string", "GET") + .json() + .queryString("list", "v1,v2,v3") + .build(); + + AwsProxyResponse output = handler.proxy(request, lambdaContext); + assertEquals(200, output.getStatusCode()); + + validateSingleValueModel(output, "3"); + } + @Test public void dateHeader_notModified_expect304() { AwsProxyRequest request = new AwsProxyRequestBuilder("/echo/last-modified", "GET") diff --git a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringBootAppTest.java b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringBootAppTest.java index c0efb0389..07388fa8c 100644 --- a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringBootAppTest.java +++ b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringBootAppTest.java @@ -66,6 +66,16 @@ public void requestUri_dotInPathParam_expectRoutingToMethod() { validateSingleValueModel(resp, "testdomain.com"); } + @Test + public void queryString_commaSeparatedList_expectUnmarshalAsList() { + AwsProxyRequest req = new AwsProxyRequestBuilder("/test/query-string", "GET") + .queryString("list", "v1,v2,v3").build(); + AwsProxyResponse resp = handler.handleRequest(req, context); + assertNotNull(resp); + assertEquals(200, resp.getStatusCode()); + validateSingleValueModel(resp, "3"); + } + private void validateSingleValueModel(AwsProxyResponse output, String value) { try { SingleValueModel response = mapper.readValue(output.getBody(), SingleValueModel.class); diff --git a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/echoapp/EchoResource.java b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/echoapp/EchoResource.java index 07817b37b..00b3360ae 100644 --- a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/echoapp/EchoResource.java +++ b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/echoapp/EchoResource.java @@ -17,6 +17,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Enumeration; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Random; @@ -67,6 +68,13 @@ public MapResponseModel echoQueryString(HttpServletRequest request, @RequestPara return queryStrings; } + @RequestMapping(path = "/list-query-string", method = RequestMethod.GET) + public SingleValueModel echoListQueryString(@RequestParam(value="list") List valueList) { + SingleValueModel value = new SingleValueModel(); + value.setValue(valueList.size() + ""); + return value; + } + @RequestMapping(path = "/authorizer-principal", method = RequestMethod.GET) public SingleValueModel echoAuthorizerPrincipal(HttpServletRequest context) { SingleValueModel valueModel = new SingleValueModel(); diff --git a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestApplication.java b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestApplication.java index d203a56e0..2ebe5f037 100644 --- a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestApplication.java +++ b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestApplication.java @@ -2,11 +2,15 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.boot.web.support.SpringBootServletInitializer; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; +import org.springframework.web.filter.CharacterEncodingFilter; @SpringBootApplication @ComponentScan(basePackages = "com.amazonaws.serverless.proxy.spring.springbootapp") public class TestApplication extends SpringBootServletInitializer { + } diff --git a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestController.java b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestController.java index 4d9f01b2e..3a87b64a7 100644 --- a/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestController.java +++ b/aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestController.java @@ -12,6 +12,7 @@ import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.servlet.config.annotation.ContentNegotiationConfigurer; import org.springframework.web.servlet.config.annotation.EnableWebMvc; @@ -19,6 +20,8 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; +import java.util.List; + @RestController @EnableWebSecurity @@ -53,6 +56,15 @@ public SingleValueModel testDomainInPath(@PathVariable("domain") String domainNa return value; } + @RequestMapping(path = "/test/query-string", method = { RequestMethod.GET }) + public SingleValueModel testQueryStringList(@RequestParam("list") List qsValues) { + assert qsValues != null; + System.out.println("QUERY_STRING_VALUES: " + qsValues); + SingleValueModel value = new SingleValueModel(); + value.setValue(qsValues.size() + ""); + return value; + } + @Override protected void configure(HttpSecurity http) throws Exception { http.sessionManagement().disable();