Merge pull request #1078 from apache/fix/WW-5473-multipart-wraper

WW-5473 Fixes examining multiple HttpServletWrappers to find MultiPartRequestWrapper

core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java

@@ -22,6 +22,7 @@ import com.opensymphony.xwork2.ActionInvocation;
 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 class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor {
 
         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 class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor {
         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;
+    }
+
 }
 

core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java

@@ -25,6 +25,7 @@ import com.opensymphony.xwork2.ValidationAwareSupport;
 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 class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
         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 +

core/src/test/resources/log4j2.xml

@@ -29,6 +29,5 @@
         <Root level="info">
             <AppenderRef ref="STDOUT"/>
         </Root>
-        <Logger name="org.apache.struts2.dispatcher.multipart" level="debug"/>
     </Loggers>
 </Configuration>