Skip to content

Commit 19d0309

Browse files
sapessisapessi
authored
Merge fixes for patch release 1.5.1 (#364)
* 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
1 parent 79b8073 commit 19d0309

File tree

12 files changed

+282
-27
lines changed

12 files changed

+282
-27
lines changed

‎.github/ISSUE_TEMPLATE.md

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,30 @@
1-
* Framework version: XX
2-
* Implementations: Jersey / Spring / Spring Boot / Spark
1+
*To help us debug your issue fill in the basic information below using the options provided*
2+
3+
*Serverless Java Container version*: `eg. 1.5`
4+
5+
*Implementations:* `Jersey / Spring / Spring Boot / Spring Boot 2 / Spark`
6+
7+
*Framework version:* `eg SpringBoot 2.2.6.RELEASE`
8+
9+
*Frontend service:* `REST API / HTTP API / ALB`
10+
11+
*Deployment method:* `eg SAM, Serverless Framework, Console`
312

413
## Scenario
14+
*Describe what you are trying to accomplish*
515

616
## Expected behavior
17+
*Describe how you would expect the application to behave*
718

819
## Actual behavior
20+
*Describe what you are seeing instead*
921

1022
## Steps to reproduce
23+
*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*
1124

1225
## Full log output
26+
*Paste the full log output from the Lambda function's CloudWatch logs*
27+
28+
```
29+
logs
30+
```

‎.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
*Issue #, if available:*
2+
3+
*Description of changes:*
4+
5+
6+
By submitting this pull request
7+
8+
- [ ] I confirm that my contribution is made under the terms of the Apache 2.0 license.
9+
- [ ] I confirm that I've made a best effort attempt to update all relevant documentation.

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpApiV2ProxyHttpServletRequest.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -482,16 +482,19 @@ private MultiValuedTreeMap<String, String> parseRawQueryString(String qs) {
482482

483483
MultiValuedTreeMap<String, String> qsMap = new MultiValuedTreeMap<>();
484484
for (String value : qs.split(QUERY_STRING_SEPARATOR)) {
485-
if (!value.contains(QUERY_STRING_KEY_VALUE_SEPARATOR)) {
486-
log.warn("Invalid query string parameter: " + SecurityUtils.crlf(value));
487-
continue;
488-
}
489-
490-
String[] kv = value.split(QUERY_STRING_KEY_VALUE_SEPARATOR);
491485
try {
492-
qsMap.add(URLDecoder.decode(kv[0], LambdaContainerHandler.getContainerConfig().getUriEncoding()), kv[1]);
486+
if (!value.contains(QUERY_STRING_KEY_VALUE_SEPARATOR)) {
487+
qsMap.add(URLDecoder.decode(value, LambdaContainerHandler.getContainerConfig().getUriEncoding()), null);
488+
log.warn("Query string parameter with empty value and no =: " + SecurityUtils.crlf(value));
489+
continue;
490+
}
491+
492+
String[] kv = value.split(QUERY_STRING_KEY_VALUE_SEPARATOR);
493+
String key = URLDecoder.decode(kv[0], LambdaContainerHandler.getContainerConfig().getUriEncoding());
494+
String val = kv.length == 2 ? kv[1] : null;
495+
qsMap.add(key, val);
493496
} catch (UnsupportedEncodingException e) {
494-
log.error("Unsupported encoding in query string key: " + SecurityUtils.crlf(kv[0]), e);
497+
log.error("Unsupported encoding in query string key: " + SecurityUtils.crlf(value), e);
495498
}
496499
}
497500
return qsMap;

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponse.java

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import javax.servlet.http.HttpServletRequest;
2929
import javax.servlet.http.HttpServletResponse;
3030
import javax.ws.rs.core.HttpHeaders;
31+
import javax.ws.rs.core.MediaType;
3132
import java.io.ByteArrayOutputStream;
3233
import java.io.IOException;
3334
import java.io.OutputStreamWriter;
@@ -387,18 +388,26 @@ public void setContentType(String s) {
387388
if (s == null) {
388389
return;
389390
}
390-
391-
// we have no forced character encoding
392-
if (characterEncoding == null) {
393-
setHeader(HttpHeaders.CONTENT_TYPE, s, true);
394-
return;
391+
String contentType = s;
392+
String charEncoding = characterEncoding;
393+
394+
// TODO: Make the utilities to parse header values from the request object generic and reuse them here
395+
if (s.contains("charset=")) { // we have a forced charset
396+
int charsetIndex = s.indexOf("charset=") + 8;
397+
int endCharsetIndex = s.indexOf(" ", charsetIndex);
398+
if (endCharsetIndex == -1) {
399+
endCharsetIndex = s.length();
400+
}
401+
charEncoding = s.substring(charsetIndex, endCharsetIndex).toUpperCase(Locale.getDefault());
402+
contentType = s.split(";")[0];
395403
}
396404

397-
if (s.contains(";")) { // we have a forced charset
398-
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s; charset=%s", s.split(";")[0], characterEncoding), true);
399-
} else {
400-
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s; charset=%s", s, characterEncoding), true);
405+
if (charEncoding == null) {
406+
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s", contentType), true);
407+
return;
401408
}
409+
characterEncoding = charEncoding;
410+
setHeader(HttpHeaders.CONTENT_TYPE, String.format("%s; charset=%s", contentType, charEncoding), true);
402411
}
403412

404413

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

424-
if(charset == null) {
433+
byte[] respBody = bodyOutputStream.toByteArray();
434+
435+
// The content type is json but we have no encoding specified, according to the RFC (https://tools.ietf.org/html/rfc4627#section-3)
436+
// we should attempt to detect the encoding. However, since we are running in Lambda we shouldn't even consider
437+
// big endian systems and it's highly unlikely we'll have apps using UTF-16/32 we simply force UTF-8
438+
if (headers != null && headers.getFirst(HttpHeaders.CONTENT_TYPE) != null &&
439+
headers.getFirst(HttpHeaders.CONTENT_TYPE).toLowerCase(Locale.getDefault()).trim().equals(MediaType.APPLICATION_JSON) &&
440+
charset == null) {
441+
charset = "UTF-8";
442+
}
443+
444+
// if at this point we are still null, we set the default
445+
if (charset == null) {
425446
charset = LambdaContainerHandler.getContainerConfig().getDefaultContentCharset();
426447
}
427448

428-
responseBody = new String(bodyOutputStream.toByteArray(), charset);
449+
responseBody = new String(respBody, charset);
429450
log.debug("Response buffer flushed with {} bytes, latch={}", responseBody.length(), writersCountDownLatch.getCount());
430451
isCommitted = true;
431452
writersCountDownLatch.countDown();

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/testutils/AwsProxyRequestBuilder.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,6 @@ public HttpApiV2ProxyRequest toHttpApiV2Request() {
482482
// we do not encode it
483483
rawQueryString.append(URLEncoder.encode(s, "UTF-8").replaceAll("%2C", ","));
484484
} catch (UnsupportedEncodingException e) {
485-
System.out.println("Ex!");
486485
throw new RuntimeException(e);
487486
}
488487
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/model/HttpApiV2ProxyRequest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class HttpApiV2ProxyRequest {
2626
private Map<String, String> headers;
2727
private Map<String, String> queryStringParameters;
2828
private String body;
29+
private Map<String, String> pathParameters;
2930
private boolean isBase64Encoded;
3031
private Map<String, String> stageVariables;
3132
private HttpApiV2ProxyRequestContext requestContext;
@@ -90,6 +91,14 @@ public String getBody() {
9091
return body;
9192
}
9293

94+
public Map<String, String> getPathParameters() {
95+
return pathParameters;
96+
}
97+
98+
public void setPathParameters(Map<String, String> pathParameters) {
99+
this.pathParameters = pathParameters;
100+
}
101+
93102
public void setBody(String body) {
94103
this.body = body;
95104
}

‎aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsHttpServletResponseTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.amazonaws.serverless.proxy.internal.servlet;
22

33

4+
import com.amazonaws.serverless.proxy.model.ContainerConfig;
45
import com.amazonaws.serverless.proxy.model.Headers;
56

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

40+
private static final String CONTENT_TYPE_WITH_CHARSET = "application/json; charset=UTF-8";
41+
3942
@Test
4043
public void cookie_addCookie_verifyPath() {
4144
AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null);
@@ -301,6 +304,27 @@ public void characterEncoding_setCharacterEncodingAndsetContentType() {
301304
assertEquals("UTF-8", resp.getCharacterEncoding());
302305
}
303306

307+
@Test
308+
public void characterEncoding_setCharacterEncodingInContentType_characterEncodingPopulatedCorrectly() {
309+
AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null);
310+
resp.setContentType(CONTENT_TYPE_WITH_CHARSET);
311+
312+
assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getContentType());
313+
assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getHeader("Content-Type"));
314+
assertEquals("UTF-8", resp.getCharacterEncoding());
315+
}
316+
317+
@Test
318+
public void characterEncoding_setCharacterEncodingInContentType_overridesDefault() {
319+
AwsHttpServletResponse resp = new AwsHttpServletResponse(null, null);
320+
resp.setCharacterEncoding(ContainerConfig.DEFAULT_CONTENT_CHARSET);
321+
resp.setContentType(CONTENT_TYPE_WITH_CHARSET);
322+
323+
assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getContentType());
324+
assertEquals(CONTENT_TYPE_WITH_CHARSET, resp.getHeader("Content-Type"));
325+
assertEquals("UTF-8", resp.getCharacterEncoding());
326+
}
327+
304328
private int getMaxAge(String header) {
305329
Matcher ageMatcher = MAX_AGE_PATTERN.matcher(header);
306330
assertTrue(ageMatcher.find());

‎aws-serverless-java-container-spring/src/test/java/com/amazonaws/serverless/proxy/spring/echoapp/EchoResource.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
public class EchoResource {
3939
public static final String TEST_GENERATE_URI = "test";
4040
public static final String STRING_BODY = "Hello";
41+
public static final String EX_MESSAGE = "404 exception message";
4142

4243
@Bean
4344
public MultipartResolver multipartResolver() {

‎aws-serverless-java-container-springboot2/src/test/java/com/amazonaws/serverless/proxy/spring/ServletAppTest.java

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@
33
import com.amazonaws.serverless.proxy.internal.LambdaContainerHandler;
44
import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder;
55
import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext;
6+
import com.amazonaws.serverless.proxy.model.AwsProxyRequest;
67
import com.amazonaws.serverless.proxy.model.AwsProxyResponse;
7-
import com.amazonaws.serverless.proxy.spring.servletapp.LambdaHandler;
8-
import com.amazonaws.serverless.proxy.spring.servletapp.MessageController;
9-
import com.amazonaws.serverless.proxy.spring.servletapp.MessageData;
10-
import com.amazonaws.serverless.proxy.spring.servletapp.UserData;
8+
import com.amazonaws.serverless.proxy.model.ContainerConfig;
9+
import com.amazonaws.serverless.proxy.spring.servletapp.*;
1110
import com.fasterxml.jackson.core.JsonProcessingException;
1211
import org.junit.Assert;
1312
import org.junit.Test;
@@ -17,8 +16,12 @@
1716
import javax.ws.rs.core.HttpHeaders;
1817
import javax.ws.rs.core.MediaType;
1918

19+
import java.io.ByteArrayOutputStream;
20+
import java.io.IOException;
21+
import java.io.InputStream;
2022
import java.util.Arrays;
2123
import java.util.Collection;
24+
import java.util.stream.Collectors;
2225

2326
import static org.junit.Assert.assertEquals;
2427
import static org.junit.Assert.assertNotNull;
@@ -108,4 +111,85 @@ public void echoMessage_fileNameLikeParameter_returnsMessage() {
108111
assertEquals(200, resp.getStatusCode());
109112
assertEquals("test.test.test", resp.getBody());
110113
}
114+
115+
@Test
116+
public void getUtf8String_returnsValidUtf8String() {
117+
// We expect strings to come back as UTF-8 correctly because Spring itself will call the setCharacterEncoding
118+
// method on the response to set it to UTF-
119+
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
120+
AwsProxyRequestBuilder req = new AwsProxyRequestBuilder("/content-type/utf8", "GET")
121+
.header(HttpHeaders.ACCEPT, MediaType.TEXT_PLAIN);
122+
AwsProxyResponse resp = handler.handleRequest(req, lambdaContext);
123+
assertNotNull(resp);
124+
assertEquals(200, resp.getStatusCode());
125+
assertEquals("text/plain; charset=UTF-8", resp.getMultiValueHeaders().get(HttpHeaders.CONTENT_TYPE).stream().collect(Collectors.joining(",")));
126+
assertEquals(MessageController.UTF8_RESPONSE, resp.getBody());
127+
}
128+
129+
@Test
130+
public void getUtf8Json_returnsValidUtf8String() {
131+
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
132+
AwsProxyRequestBuilder req = new AwsProxyRequestBuilder("/content-type/jsonutf8", "GET");
133+
AwsProxyResponse resp = handler.handleRequest(req, lambdaContext);
134+
assertNotNull(resp);
135+
assertEquals(200, resp.getStatusCode());
136+
assertEquals("{\"s\":\""+MessageController.UTF8_RESPONSE+"\"}", resp.getBody());
137+
}
138+
139+
@Test
140+
public void stream_getUtf8String_returnsValidUtf8String() throws IOException {
141+
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
142+
LambdaStreamHandler streamHandler = new LambdaStreamHandler(type);
143+
AwsProxyRequestBuilder reqBuilder = new AwsProxyRequestBuilder("/content-type/utf8", "GET")
144+
.header(HttpHeaders.ACCEPT, MediaType.TEXT_PLAIN);
145+
InputStream req = null;
146+
switch (type) {
147+
case "ALB":
148+
req = reqBuilder.alb().buildStream();
149+
break;
150+
case "API_GW":
151+
req = reqBuilder.buildStream();
152+
break;
153+
case "HTTP_API":
154+
req = reqBuilder.toHttpApiV2RequestStream();
155+
}
156+
ByteArrayOutputStream out = new ByteArrayOutputStream();
157+
streamHandler.handleRequest(req, out, lambdaContext);
158+
AwsProxyResponse resp = LambdaContainerHandler.getObjectMapper().readValue(out.toByteArray(), AwsProxyResponse.class);
159+
assertNotNull(resp);
160+
assertEquals(200, resp.getStatusCode());
161+
assertEquals(MessageController.UTF8_RESPONSE, resp.getBody());
162+
}
163+
164+
@Test
165+
public void stream_getUtf8Json_returnsValidUtf8String() throws IOException {
166+
LambdaContainerHandler.getContainerConfig().setDefaultContentCharset(ContainerConfig.DEFAULT_CONTENT_CHARSET);
167+
LambdaStreamHandler streamHandler = new LambdaStreamHandler(type);
168+
AwsProxyRequestBuilder reqBuilder = new AwsProxyRequestBuilder("/content-type/jsonutf8", "GET");
169+
InputStream req = null;
170+
switch (type) {
171+
case "ALB":
172+
req = reqBuilder.alb().buildStream();
173+
break;
174+
case "API_GW":
175+
req = reqBuilder.buildStream();
176+
break;
177+
case "HTTP_API":
178+
req = reqBuilder.toHttpApiV2RequestStream();
179+
}
180+
ByteArrayOutputStream out = new ByteArrayOutputStream();
181+
streamHandler.handleRequest(req, out, lambdaContext);
182+
AwsProxyResponse resp = LambdaContainerHandler.getObjectMapper().readValue(out.toByteArray(), AwsProxyResponse.class);
183+
assertNotNull(resp);
184+
assertEquals(200, resp.getStatusCode());
185+
assertEquals("{\"s\":\""+MessageController.UTF8_RESPONSE+"\"}", resp.getBody());
186+
}
187+
188+
@Test
189+
public void springExceptionMapping_throw404Ex_expectMappedTo404() {
190+
AwsProxyRequestBuilder req = new AwsProxyRequestBuilder("/ex/customstatus", "GET");
191+
AwsProxyResponse resp = handler.handleRequest(req, lambdaContext);
192+
assertNotNull(resp);
193+
assertEquals(404, resp.getStatusCode());
194+
}
111195
}

0 commit comments

Comments
 (0)