Skip to content

Commit

Permalink
Changes to query string handling to address #162. parameter values co…
Browse files Browse the repository at this point in the history 
…me from request decoded. the getQueryString return an encoded string.
  • Loading branch information
@sapessi
sapessi committed Jun 22, 2018
1 parent dfe6e4f commit 5cf97f9
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 123 deletions.
29 changes: 26 additions & 3 deletions .../src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, String> parameters, boolean encode, String encodeCharset)
throws ServletException {
if (parameters == null || parameters.size() == 0) {
return null;
}
Expand All @@ -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<String, String> 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
Expand Down
68 changes: 39 additions & 29 deletions ...main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ public class AwsProxyHttpServletRequest extends AwsHttpServletRequest {
private Map<String, List<String>> urlEncodedFormParameters;
private Map<String, Part> multipartFormParameters;
private Map<String, String> caseInsensitiveHeaders;
private EncodingQueryStringParameterMap queryStringParameters;
private static Logger log = LoggerFactory.getLogger(AwsProxyHttpServletRequest.class);
private ContainerConfig config;

Expand All @@ -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());
}
Expand Down Expand Up @@ -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;
}
}


Expand Down Expand Up @@ -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;
}
Expand All @@ -462,7 +459,9 @@ public String getParameter(String s) {
@Override
public Enumeration<String> getParameterNames() {
List<String> paramNames = new ArrayList<>();
paramNames.addAll(queryStringParameters.keySet());
if (request.getQueryStringParameters() != null) {
paramNames.addAll(request.getQueryStringParameters().keySet());
}
paramNames.addAll(getFormUrlEncodedParametersMap().keySet());
return Collections.enumeration(paramNames);
}
Expand All @@ -471,16 +470,11 @@ public Enumeration<String> 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<String> values = new ArrayList<>();
List<String> 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) {
Expand All @@ -504,9 +498,17 @@ public Map<String, String[]> 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<String> 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;
}
Expand Down Expand Up @@ -649,13 +651,6 @@ public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse se
return null;
}

//-------------------------------------------------------------
// Methods - Protected
//-------------------------------------------------------------

protected EncodingQueryStringParameterMap getQueryParametersMap() {
return queryStringParameters;
}

//-------------------------------------------------------------
// Methods - Private
Expand Down Expand Up @@ -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;
Expand Down
89 changes: 0 additions & 89 deletions ...java/com/amazonaws/serverless/proxy/internal/servlet/EncodingQueryStringParameterMap.java
View file

This file was deleted.

17 changes: 15 additions & 2 deletions .../test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletRequestTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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"));
Expand All @@ -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());
Expand Down
13 changes: 13 additions & 0 deletions ...tainer-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringAwsProxyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
10 changes: 10 additions & 0 deletions ...ntainer-spring/src/test/java/com/amazonaws/serverless/proxy/spring/SpringBootAppTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions ...iner-spring/src/test/java/com/amazonaws/serverless/proxy/spring/echoapp/EchoResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> 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();
Expand Down
4 changes: 4 additions & 0 deletions ...ng/src/test/java/com/amazonaws/serverless/proxy/spring/springbootapp/TestApplication.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

}
Loading

0 comments on commit 5cf97f9

Please sign in to comment.