From 15fb28f2db71335b633972cb1fe5262493dfe04c Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 16 Oct 2024 10:06:45 +0200 Subject: [PATCH] WW-5473 Fixes examining multiple HttpServletWrappers to find MultiPartRequestWrapper --- .../ActionFileUploadInterceptor.java | 32 +++++++++- .../ActionFileUploadInterceptorTest.java | 62 +++++++++++++++++++ core/src/test/resources/log4j2.xml | 1 - 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java index a0197e6c39..3b6ef08aa0 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java @@ -22,6 +22,7 @@ import com.opensymphony.xwork2.ActionProxy; import com.opensymphony.xwork2.interceptor.ValidationAware; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.action.UploadedFilesAware; @@ -135,7 +136,11 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor { @Override public String intercept(ActionInvocation invocation) throws Exception { HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); - if (!(request instanceof MultiPartRequestWrapper multiWrapper)) { + MultiPartRequestWrapper multiWrapper = request instanceof HttpServletRequestWrapper wrapper + ? findMultipartRequestWrapper(wrapper) + : null; + + if (multiWrapper == null) { if (LOG.isDebugEnabled()) { ActionProxy proxy = invocation.getProxy(); LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, new String[]{proxy.getNamespace(), proxy.getActionName()})); @@ -145,8 +150,8 @@ public String intercept(ActionInvocation invocation) throws Exception { if (!(invocation.getAction() instanceof UploadedFilesAware action)) { LOG.debug("Action: {} doesn't implement: {}, ignoring file upload", - invocation.getProxy().getActionName(), - UploadedFilesAware.class.getSimpleName()); + invocation.getProxy().getActionName(), + UploadedFilesAware.class.getSimpleName()); return invocation.invoke(); } @@ -185,5 +190,26 @@ public String intercept(ActionInvocation invocation) throws Exception { return invocation.invoke(); } + /** + * Tries to find {@link MultiPartRequestWrapper} as the request can be already wrapped + * with another {@link HttpServletRequestWrapper}. + * If the {@link MultiPartRequestWrapper} cannot be found, null is returned instead. + * + * @param request current {@link HttpServletRequestWrapper} + * @return {@link MultiPartRequestWrapper} or null + * @since 7.0.0 + */ + protected MultiPartRequestWrapper findMultipartRequestWrapper(HttpServletRequestWrapper request) { + if (request instanceof MultiPartRequestWrapper multiPartRequestWrapper) { + LOG.debug("Found multipart request: {}", multiPartRequestWrapper.getClass().getSimpleName()); + return multiPartRequestWrapper; + } else if (request.getRequest() instanceof HttpServletRequestWrapper wrappedRequest) { + LOG.debug("Could not find multipart request wrapper, checking ancestor: {}", + wrappedRequest.getClass().getSimpleName()); + return findMultipartRequestWrapper(wrappedRequest); + } + return null; + } + } diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java index ec1195490f..178d6bfc02 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java @@ -25,6 +25,7 @@ import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.mock.MockActionProxy; import com.opensymphony.xwork2.util.ClassLoaderUtil; +import jakarta.servlet.http.HttpServletRequestWrapper; import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload; import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletFileUpload; import org.apache.struts2.StrutsInternalTestCase; @@ -514,6 +515,67 @@ public void testMultipartRequestLocalizedError() throws Exception { assertTrue(msg.startsWith("Der Request übertraf die maximal erlaubte Größe")); } + public void testSingleHttpRequestWrapper() throws Exception { + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("post"); + request.addHeader("Content-type", "multipart/form-data; boundary=---1234"); + + // inspired by the unit tests for jakarta commons fileupload + String content = (""" + -----1234\r + Content-Disposition: form-data; name="file"; filename="deleteme.txt"\r + Content-Type: text/html\r + \r + Unit test of ActionFileUploadInterceptor\r + -----1234--\r + """); + request.setContent(content.getBytes(StandardCharsets.UTF_8)); + MyFileUploadAction action = container.inject(MyFileUploadAction.class); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext() + .withServletRequest(new HttpServletRequestWrapper(createMultipartRequestMaxSize(1000))); + + String result = interceptor.intercept(mai); + + assertFalse(action.hasActionErrors()); + assertEquals("success", result); + } + + public void testMultipleHttpRequestWrappers() throws Exception { + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("post"); + request.addHeader("Content-type", "multipart/form-data; boundary=---1234"); + + // inspired by the unit tests for jakarta commons fileupload + String content = (""" + -----1234\r + Content-Disposition: form-data; name="file"; filename="deleteme.txt"\r + Content-Type: text/html\r + \r + Unit test of ActionFileUploadInterceptor\r + -----1234--\r + """); + request.setContent(content.getBytes(StandardCharsets.US_ASCII)); + + MyFileUploadAction action = container.inject(MyFileUploadAction.class); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext() + .withServletRequest(new HttpServletRequestWrapper(new HttpServletRequestWrapper(createMultipartRequestMaxSize(1000)))); + + String result = interceptor.intercept(mai); + + assertFalse(action.hasActionErrors()); + assertEquals("success", result); + } + private String encodeTextFile(String filename, String contentType, String content) { return endline + "--" + boundary + diff --git a/core/src/test/resources/log4j2.xml b/core/src/test/resources/log4j2.xml index 88dd6e0f6f..4c4061dbfa 100644 --- a/core/src/test/resources/log4j2.xml +++ b/core/src/test/resources/log4j2.xml @@ -29,6 +29,5 @@ - \ No newline at end of file