Skip to content

Commit

Permalink
Merge fixes for patch release 1.5.1 (#364)
Browse files Browse the repository at this point in the history
* fix: Adding pathParameters to v2 proxy event as reported in #358.

* fix: Address JSON content type issue reported in #352 and #344

* fix: Fixed bug caught by integration tests for #352

* fix: Fix struts tests for the changes made for #352

* test: Attempting to replicate the issue reported in #342

* test: Reverting exception test in Spring package since it's only available in Spring5, not Spring4

* fix: Sigh, forgot to remove the import for the class that doesn't exist from the previous commit

* fix: Addresses bug reported in query string parsing (#363) for HTTP API support where we have a query string key, followed by a value declarator (=), but then no value

* chore: Update GitHub issue and PR templates

* fix: Fixed issue reported by SpotBugs with the exception logging of the HTTP API query string parsing
  • Loading branch information
sapessi authored Jul 15, 2020
1 parent 79b8073 commit 19d0309
Show file tree
Hide file tree
Showing 12 changed files with 282 additions and 27 deletions.
22 changes: 20 additions & 2 deletions .github/ISSUE_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
* Framework version: XX
* Implementations: Jersey / Spring / Spring Boot / Spark
*To help us debug your issue fill in the basic information below using the options provided*

*Serverless Java Container version*: `eg. 1.5`

*Implementations:* `Jersey / Spring / Spring Boot / Spring Boot 2 / Spark`

*Framework version:* `eg SpringBoot 2.2.6.RELEASE`

*Frontend service:* `REST API / HTTP API / ALB`

*Deployment method:* `eg SAM, Serverless Framework, Console`

## Scenario
*Describe what you are trying to accomplish*

## Expected behavior
*Describe how you would expect the application to behave*

## Actual behavior
*Describe what you are seeing instead*

## Steps to reproduce
*Provide code samples we can use to reproduce the issue as part of our integration tests. If there is a public repository for the misbehaving application link to it here*

## Full log output
*Paste the full log output from the Lambda function's CloudWatch logs*

```
logs
```
9 changes: 9 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
*Issue #, if available:*

*Description of changes:*


By submitting this pull request

- [ ] I confirm that my contribution is made under the terms of the Apache 2.0 license.
- [ ] I confirm that I've made a best effort attempt to update all relevant documentation.
Original file line number Diff line number Diff line change
Expand Up @@ -482,16 +482,19 @@ private MultiValuedTreeMap<String, String> parseRawQueryString(String qs) {

MultiValuedTreeMap<String, String> qsMap = new MultiValuedTreeMap<>();
for (String value : qs.split(QUERY_STRING_SEPARATOR)) {
if (!value.contains(QUERY_STRING_KEY_VALUE_SEPARATOR)) {
log.warn("Invalid query string parameter: " + SecurityUtils.crlf(value));
continue;
}

String[] kv = value.split(QUERY_STRING_KEY_VALUE_SEPARATOR);
try {
qsMap.add(URLDecoder.decode(kv[0], LambdaContainerHandler.getContainerConfig().getUriEncoding()), kv[1]);
if (!value.contains(QUERY_STRING_KEY_VALUE_SEPARATOR)) {
qsMap.add(URLDecoder.decode(value, LambdaContainerHandler.getContainerConfig().getUriEncoding()), null);
log.warn("Query string parameter with empty value and no =: " + SecurityUtils.crlf(value));
continue;
}

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] : null;
qsMap.add(key, val);
} catch (UnsupportedEncodingException e) {
log.error("Unsupported encoding in query string key: " + SecurityUtils.crlf(kv[0]), e);
log.error("Unsupported encoding in query string key: " + SecurityUtils.crlf(value), e);
}
}
return qsMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
Expand Down Expand Up @@ -387,18 +388,26 @@ public void setContentType(String s) {
if (s == null) {
return;
}

// we have no forced character encoding
if (characterEncoding == null) {
setHeader(HttpHeaders.CONTENT_TYPE, s, true);
return;
String contentType = s;
String charEncoding = characterEncoding;

// TODO: Make the utilities to parse header values from the request object generic and reuse them here
if (s.contains("charset=")) { // we have a forced charset
int charsetIndex = s.indexOf("charset=") + 8;
int endCharsetIndex = s.indexOf(" ", charsetIndex);
if (endCharsetIndex == -1) {
endCharsetIndex = s.length();
}
charEncoding = s.substring(charsetIndex, endCharsetIndex).toUpperCase(Locale.getDefault());
contentType = s.split(";")[0];
}

if (s.contains(";")) { // we have a forced charset
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s; charset=%s", s.split(";")[0], characterEncoding), true);
} else {
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s; charset=%s", s, characterEncoding), true);
if (charEncoding == null) {
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s", contentType), true);
return;
}
characterEncoding = charEncoding;
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s; charset=%s", contentType, charEncoding), true);
}


Expand All @@ -421,11 +430,23 @@ public void flushBuffer() throws IOException {
}
String charset = characterEncoding;

if(charset == null) {
byte[] respBody = bodyOutputStream.toByteArray();

// The content type is json but we have no encoding specified, according to the RFC (https://tools.ietf.org/html/rfc4627#section-3)
// we should attempt to detect the encoding. However, since we are running in Lambda we shouldn't even consider
// big endian systems and it's highly unlikely we'll have apps using UTF-16/32 we simply force UTF-8
if (headers != null && headers.getFirst(HttpHeaders.CONTENT_TYPE) != null &&
headers.getFirst(HttpHeaders.CONTENT_TYPE).toLowerCase(Locale.getDefault()).trim().equals(MediaType.APPLICATION_JSON) &&
charset == null) {
charset = "UTF-8";
}

// if at this point we are still null, we set the default
if (charset == null) {
charset = LambdaContainerHandler.getContainerConfig().getDefaultContentCharset();
}

responseBody = new String(bodyOutputStream.toByteArray(), charset);
responseBody = new String(respBody, charset);
log.debug("Response buffer flushed with {} bytes, latch={}", responseBody.length(), writersCountDownLatch.getCount());
isCommitted = true;
writersCountDownLatch.countDown();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,6 @@ public HttpApiV2ProxyRequest toHttpApiV2Request() {
// we do not encode it
rawQueryString.append(URLEncoder.encode(s, "UTF-8").replaceAll("%2C", ","));
} catch (UnsupportedEncodingException e) {
System.out.println("Ex!");
throw new RuntimeException(e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class HttpApiV2ProxyRequest {
private Map<String, String> headers;
private Map<String, String> queryStringParameters;
private String body;
private Map<String, String> pathParameters;
private boolean isBase64Encoded;
private Map<String, String> stageVariables;
private HttpApiV2ProxyRequestContext requestContext;
Expand Down Expand Up @@ -90,6 +91,14 @@ public String getBody() {
return body;
}

public Map<String, String> getPathParameters() {
return pathParameters;
}

public void setPathParameters(Map<String, String> pathParameters) {
this.pathParameters = pathParameters;
}

public void setBody(String body) {
this.body = body;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.amazonaws.serverless.proxy.internal.servlet;


import com.amazonaws.serverless.proxy.model.ContainerConfig;
import com.amazonaws.serverless.proxy.model.Headers;

import org.junit.Test;
Expand Down Expand Up @@ -36,6 +37,8 @@ public class AwsHttpServletResponseTest {
private static final Pattern MAX_AGE_PATTERN = Pattern.compile("Max-Age=(-?[0-9]+)");
private static final Pattern EXPIRES_PATTERN = Pattern.compile("Expires=(.*)$");

private static final String CONTENT_TYPE_WITH_CHARSET = "application/json; charset=UTF-8";

@Test
public void cookie_addCookie_verifyPath() {
AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null);
Expand Down Expand Up @@ -301,6 +304,27 @@ public void characterEncoding_setCharacterEncodingAndsetContentType() {
assertEquals("UTF-8", resp.getCharacterEncoding());
}

@Test
public void characterEncoding_setCharacterEncodingInContentType_characterEncodingPopulatedCorrectly() {
AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null);
resp.setContentType(CONTENT_TYPE_WITH_CHARSET);

assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getContentType());
assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getHeader("Content-Type"));
assertEquals("UTF-8", resp.getCharacterEncoding());
}

@Test
public void characterEncoding_setCharacterEncodingInContentType_overridesDefault() {
AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null);
resp.setCharacterEncoding(ContainerConfig.DEFAULT_CONTENT_CHARSET);
resp.setContentType(CONTENT_TYPE_WITH_CHARSET);

assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getContentType());
assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getHeader("Content-Type"));
assertEquals("UTF-8", resp.getCharacterEncoding());
}

private int getMaxAge(String header) {
Matcher ageMatcher = MAX_AGE_PATTERN.matcher(header);
assertTrue(ageMatcher.find());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
public class EchoResource {
public static final String TEST_GENERATE_URI = "test";
public static final String STRING_BODY = "Hello";
public static final String EX_MESSAGE = "404 exception message";

@Bean
public MultipartResolver multipartResolver() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
import com.amazonaws.serverless.proxy.internal.LambdaContainerHandler;
import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder;
import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext;
import com.amazonaws.serverless.proxy.model.AwsProxyRequest;
import com.amazonaws.serverless.proxy.model.AwsProxyResponse;
import com.amazonaws.serverless.proxy.spring.servletapp.LambdaHandler;
import com.amazonaws.serverless.proxy.spring.servletapp.MessageController;
import com.amazonaws.serverless.proxy.spring.servletapp.MessageData;
import com.amazonaws.serverless.proxy.spring.servletapp.UserData;
import com.amazonaws.serverless.proxy.model.ContainerConfig;
import com.amazonaws.serverless.proxy.spring.servletapp.*;
import com.fasterxml.jackson.core.JsonProcessingException;
import org.junit.Assert;
import org.junit.Test;
Expand All @@ -17,8 +16,12 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
import java.util.Collection;
import java.util.stream.Collectors;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -108,4 +111,85 @@ public void echoMessage_fileNameLikeParameter_returnsMessage() {
assertEquals(200, resp.getStatusCode());
assertEquals("test.test.test", resp.getBody());
}

@Test
public void getUtf8String_returnsValidUtf8String() {
// We expect strings to come back as UTF-8 correctly because Spring itself will call the setCharacterEncoding
// method on the response to set it to UTF-
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
AwsProxyRequestBuilder req = new AwsProxyRequestBuilder("/content-type/utf8", "GET")
.header(HttpHeaders.ACCEPT, MediaType.TEXT_PLAIN);
AwsProxyResponse resp = handler.handleRequest(req, lambdaContext);
assertNotNull(resp);
assertEquals(200, resp.getStatusCode());
assertEquals("text/plain; charset=UTF-8", resp.getMultiValueHeaders().get(HttpHeaders.CONTENT_TYPE).stream().collect(Collectors.joining(",")));
assertEquals(MessageController.UTF8_RESPONSE, resp.getBody());
}

@Test
public void getUtf8Json_returnsValidUtf8String() {
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
AwsProxyRequestBuilder req = new AwsProxyRequestBuilder("/content-type/jsonutf8", "GET");
AwsProxyResponse resp = handler.handleRequest(req, lambdaContext);
assertNotNull(resp);
assertEquals(200, resp.getStatusCode());
assertEquals("{\"s\":\""+MessageController.UTF8_RESPONSE+"\"}", resp.getBody());
}

@Test
public void stream_getUtf8String_returnsValidUtf8String() throws IOException {
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
LambdaStreamHandler streamHandler = new LambdaStreamHandler(type);
AwsProxyRequestBuilder reqBuilder = new AwsProxyRequestBuilder("/content-type/utf8", "GET")
.header(HttpHeaders.ACCEPT, MediaType.TEXT_PLAIN);
InputStream req = null;
switch (type) {
case "ALB":
req = reqBuilder.alb().buildStream();
break;
case "API_GW":
req = reqBuilder.buildStream();
break;
case "HTTP_API":
req = reqBuilder.toHttpApiV2RequestStream();
}
ByteArrayOutputStream out = new ByteArrayOutputStream();
streamHandler.handleRequest(req, out, lambdaContext);
AwsProxyResponse resp = LambdaContainerHandler.getObjectMapper().readValue(out.toByteArray(), AwsProxyResponse.class);
assertNotNull(resp);
assertEquals(200, resp.getStatusCode());
assertEquals(MessageController.UTF8_RESPONSE, resp.getBody());
}

@Test
public void stream_getUtf8Json_returnsValidUtf8String() throws IOException {
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
LambdaStreamHandler streamHandler = new LambdaStreamHandler(type);
AwsProxyRequestBuilder reqBuilder = new AwsProxyRequestBuilder("/content-type/jsonutf8", "GET");
InputStream req = null;
switch (type) {
case "ALB":
req = reqBuilder.alb().buildStream();
break;
case "API_GW":
req = reqBuilder.buildStream();
break;
case "HTTP_API":
req = reqBuilder.toHttpApiV2RequestStream();
}
ByteArrayOutputStream out = new ByteArrayOutputStream();
streamHandler.handleRequest(req, out, lambdaContext);
AwsProxyResponse resp = LambdaContainerHandler.getObjectMapper().readValue(out.toByteArray(), AwsProxyResponse.class);
assertNotNull(resp);
assertEquals(200, resp.getStatusCode());
assertEquals("{\"s\":\""+MessageController.UTF8_RESPONSE+"\"}", resp.getBody());
}

@Test
public void springExceptionMapping_throw404Ex_expectMappedTo404() {
AwsProxyRequestBuilder req = new AwsProxyRequestBuilder("/ex/customstatus", "GET");
AwsProxyResponse resp = handler.handleRequest(req, lambdaContext);
assertNotNull(resp);
assertEquals(404, resp.getStatusCode());
}
}
Loading

0 comments on commit 19d0309

Please sign in to comment.