From 397cd9dc45565b30de5963046f2f085869f61400 Mon Sep 17 00:00:00 2001 From: mbfreder Date: Thu, 2 Nov 2023 17:27:41 -0700 Subject: [PATCH 1/2] fix: Multipart files processing when Array of files with same fieldName is sent in request --- .../AwsHttpApiV2ProxyHttpServletRequest.java | 14 ++++++- .../servlet/AwsHttpServletRequest.java | 14 +++++-- .../servlet/AwsProxyHttpServletRequest.java | 13 ++++++- .../AwsProxyHttpServletRequestFormTest.java | 37 +++++++++++++++++-- 4 files changed, 67 insertions(+), 11 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 bdf11b819..3229fda2d 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 @@ -37,6 +37,7 @@ import java.time.ZonedDateTime; import java.time.format.DateTimeParseException; import java.util.*; +import java.util.stream.Collectors; import java.util.stream.Stream; public class AwsHttpApiV2ProxyHttpServletRequest extends AwsHttpServletRequest { @@ -236,12 +237,21 @@ public void logout() throws ServletException { @Override public Collection getParts() throws IOException, ServletException { - return getMultipartFormParametersMap().values(); + List partList = + getMultipartFormParametersMap().values().stream() + .flatMap(List::stream) + .collect(Collectors.toList()); + return partList; } @Override public Part getPart(String s) throws IOException, ServletException { - return getMultipartFormParametersMap().get(s); + // In case there's multiple files with the same fieldName, we return the first one in the list + List values = getMultipartFormParametersMap().get(s); + if (Objects.isNull(values)) { + return null; + } + return getMultipartFormParametersMap().get(s).get(0); } @Override 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 7a44fa95e..10db7defe 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 @@ -86,7 +86,7 @@ public abstract class AwsHttpServletRequest implements HttpServletRequest { private ServletContext servletContext; private AwsHttpSession session; private String queryString; - private Map multipartFormParameters; + private Map> multipartFormParameters; private Map> urlEncodedFormParameters; protected AwsHttpServletResponse response; @@ -511,7 +511,7 @@ protected Map> getFormUrlEncodedParametersMap() { } @SuppressFBWarnings({"FILE_UPLOAD_FILENAME", "WEAK_FILENAMEUTILS"}) - protected Map getMultipartFormParametersMap() { + protected Map> getMultipartFormParametersMap() { if (multipartFormParameters != null) { return multipartFormParameters; } @@ -537,7 +537,7 @@ protected Map getMultipartFormParametersMap() { newPart.addHeader(h, item.getHeaders().getHeader(h)); }); - multipartFormParameters.put(item.getFieldName(), newPart); + addPart(multipartFormParameters, item.getFieldName(), newPart); } } catch (FileUploadException e) { Timer.stop("SERVLET_REQUEST_GET_MULTIPART_PARAMS"); @@ -546,6 +546,14 @@ protected Map getMultipartFormParametersMap() { Timer.stop("SERVLET_REQUEST_GET_MULTIPART_PARAMS"); return multipartFormParameters; } + private void addPart(Map> params, String fieldName, Part newPart) { + List partList = params.get(fieldName); + if (Objects.isNull(partList)) { + partList = new ArrayList<>(); + params.put(fieldName, partList); + } + partList.add(newPart); + } protected String[] getQueryParamValues(MultiValuedTreeMap qs, String key, boolean isCaseSensitive) { List value = getQueryParamValuesAsList(qs, key, isCaseSensitive); 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 a4ee15150..08ffa6d16 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 @@ -259,14 +259,23 @@ public void logout() @Override public Collection getParts() throws IOException, ServletException { - return getMultipartFormParametersMap().values(); + List partList = + getMultipartFormParametersMap().values().stream() + .flatMap(List::stream) + .collect(Collectors.toList()); + return partList; } @Override public Part getPart(String s) throws IOException, ServletException { - return getMultipartFormParametersMap().get(s); + // In case there's multiple files with the same fieldName, we return the first one in the list + List values = getMultipartFormParametersMap().get(s); + if (Objects.isNull(values)) { + return null; + } + return getMultipartFormParametersMap().get(s).get(0); } diff --git a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestFormTest.java b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestFormTest.java index 989066328..67b147620 100644 --- a/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestFormTest.java +++ b/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestFormTest.java @@ -4,7 +4,9 @@ import com.amazonaws.serverless.proxy.model.AwsProxyRequest; import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder; +import jakarta.servlet.http.Part; import org.apache.commons.io.IOUtils; +import org.apache.hc.client5.http.entity.mime.MultipartPartBuilder; import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder; @@ -17,10 +19,7 @@ import java.io.IOException; import java.nio.charset.Charset; -import java.util.Base64; -import java.util.Collections; -import java.util.Map; -import java.util.Random; +import java.util.*; import static org.junit.jupiter.api.Assertions.*; @@ -31,6 +30,7 @@ public class AwsProxyHttpServletRequestFormTest { private static final String PART_KEY_2 = "test2"; private static final String PART_VALUE_2 = "value2"; private static final String FILE_KEY = "file_upload_1"; + private static final String FILE_KEY_2 = "file_upload_2"; private static final String FILE_NAME = "testImage.jpg"; private static final String ENCODED_VALUE = "test123a%3D1%262@3"; @@ -41,6 +41,7 @@ public class AwsProxyHttpServletRequestFormTest { .build(); private static final int FILE_SIZE = 512; private static byte[] FILE_BYTES = new byte[FILE_SIZE]; + private static byte[] FILE_BYTES_2 = new byte[FILE_SIZE]; static { new Random().nextBytes(FILE_BYTES); } @@ -49,6 +50,10 @@ public class AwsProxyHttpServletRequestFormTest { .addTextBody(PART_KEY_2, PART_VALUE_2) .addBinaryBody(FILE_KEY, FILE_BYTES, ContentType.IMAGE_JPEG, FILE_NAME) .build(); + private static final HttpEntity MULTIPART_BINARY_DATA_2 = MultipartEntityBuilder.create() + .addBinaryBody(FILE_KEY, FILE_BYTES, ContentType.IMAGE_JPEG, FILE_NAME) + .addBinaryBody(FILE_KEY, FILE_BYTES_2, ContentType.IMAGE_JPEG, FILE_NAME) + .build(); private static final String ENCODED_FORM_ENTITY = PART_KEY_1 + "=" + ENCODED_VALUE + "&" + PART_KEY_2 + "=" + PART_VALUE_2; @Test @@ -109,6 +114,30 @@ void multipart_getParts_binary() { } } + @Test + void multipart_getParts_returnsMultiplePartsWithSameFieldName() { + try { + AwsProxyRequest proxyRequest = new AwsProxyRequestBuilder("/form", "POST") + .header(HttpHeaders.CONTENT_TYPE, MULTIPART_BINARY_DATA_2.getContentType()) + .header(HttpHeaders.CONTENT_LENGTH, MULTIPART_BINARY_DATA_2.getContentLength() + "") + .binaryBody(MULTIPART_BINARY_DATA_2.getContent()) + .build(); + + HttpServletRequest request = new AwsProxyHttpServletRequest(proxyRequest, null, null); + assertNotNull(request.getParts()); + assertEquals(2, request.getParts().size()); + assertNotNull(request.getPart(FILE_KEY)); + List partList = new ArrayList<>(request.getParts()); + assertEquals(partList.get(0).getSubmittedFileName(), partList.get(1).getSubmittedFileName()); + assertEquals(partList.get(0).getName(), partList.get(1).getName()); + assertEquals(FILE_SIZE, request.getPart(FILE_KEY).getSize()); + assertEquals(FILE_KEY, request.getPart(FILE_KEY).getName()); + assertEquals(FILE_NAME, request.getPart(FILE_KEY).getSubmittedFileName()); + } catch (IOException | ServletException e) { + fail(e.getMessage()); + } + } + @Test void postForm_getParamsBase64Encoded_expectAllParams() { AwsProxyRequest proxyRequest = new AwsProxyRequestBuilder("/form", "POST") From 3b00e72f215059a2aefc2744a52b78ad845e0948 Mon Sep 17 00:00:00 2001 From: mbfreder Date: Thu, 2 Nov 2023 22:51:16 -0700 Subject: [PATCH 2/2] moved getParts and getPart to AwsHttpServletRequest --- .../AwsHttpApiV2ProxyHttpServletRequest.java | 19 --------------- .../servlet/AwsHttpServletRequest.java | 21 ++++++++++++++++ .../servlet/AwsProxyHttpServletRequest.java | 24 ------------------- 3 files changed, 21 insertions(+), 43 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 3229fda2d..8a06003fd 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 @@ -235,25 +235,6 @@ public void logout() throws ServletException { throw new UnsupportedOperationException(); } - @Override - public Collection getParts() throws IOException, ServletException { - List partList = - getMultipartFormParametersMap().values().stream() - .flatMap(List::stream) - .collect(Collectors.toList()); - return partList; - } - - @Override - public Part getPart(String s) throws IOException, ServletException { - // In case there's multiple files with the same fieldName, we return the first one in the list - List values = getMultipartFormParametersMap().get(s); - if (Objects.isNull(values)) { - return null; - } - return getMultipartFormParametersMap().get(s).get(0); - } - @Override public T upgrade(Class aClass) throws IOException, ServletException { throw new UnsupportedOperationException(); 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 10db7defe..ca5d163ec 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 @@ -510,6 +510,27 @@ protected Map> getFormUrlEncodedParametersMap() { return urlEncodedFormParameters; } + @Override + public Collection getParts() + throws IOException, ServletException { + List partList = + getMultipartFormParametersMap().values().stream() + .flatMap(List::stream) + .collect(Collectors.toList()); + return partList; + } + + @Override + public Part getPart(String s) + throws IOException, ServletException { + // In case there's multiple files with the same fieldName, we return the first one in the list + List values = getMultipartFormParametersMap().get(s); + if (Objects.isNull(values)) { + return null; + } + return getMultipartFormParametersMap().get(s).get(0); + } + @SuppressFBWarnings({"FILE_UPLOAD_FILENAME", "WEAK_FILENAMEUTILS"}) protected Map> getMultipartFormParametersMap() { if (multipartFormParameters != null) { 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 08ffa6d16..a93a00b45 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 @@ -255,30 +255,6 @@ public void logout() throw new UnsupportedOperationException(); } - - @Override - public Collection getParts() - throws IOException, ServletException { - List partList = - getMultipartFormParametersMap().values().stream() - .flatMap(List::stream) - .collect(Collectors.toList()); - return partList; - } - - - @Override - public Part getPart(String s) - throws IOException, ServletException { - // In case there's multiple files with the same fieldName, we return the first one in the list - List values = getMultipartFormParametersMap().get(s); - if (Objects.isNull(values)) { - return null; - } - return getMultipartFormParametersMap().get(s).get(0); - } - - @Override public T upgrade(Class aClass) throws IOException, ServletException {