WW-5276 Cleans up also wrapper request to avoid resource leak and pot…

…ential DoS attack
10088 · Jan 22, 2023 · 05d7196 · 05d7196
1 parent 46738c9
commit 05d7196
Showing 1 changed file with 38 additions and 21 deletions.
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareAndExecuteFilter.java
@@ -23,10 +23,10 @@
 import org.apache.struts2.RequestUtils;
 import org.apache.struts2.StrutsStatics;
 import org.apache.struts2.dispatcher.Dispatcher;
-import org.apache.struts2.dispatcher.mapper.ActionMapping;
 import org.apache.struts2.dispatcher.ExecuteOperations;
 import org.apache.struts2.dispatcher.InitOperations;
 import org.apache.struts2.dispatcher.PrepareOperations;
+import org.apache.struts2.dispatcher.mapper.ActionMapping;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
@@ -106,7 +106,7 @@ protected ExecuteOperations createExecuteOperations(Dispatcher dispatcher) {
     /**
      * Callback for post initialization
      *
-     * @param dispatcher the dispatcher
+     * @param dispatcher   the dispatcher
      * @param filterConfig the filter config
      */
     protected void postInit(Dispatcher dispatcher, FilterConfig filterConfig) {
@@ -119,33 +119,50 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
 
         try {
             String uri = RequestUtils.getUri(request);
-            if (excludedPatterns != null && prepare.isUrlExcluded(request, excludedPatterns)) {
-                LOG.trace("Request {} is excluded from handling by Struts, passing request to other filters", uri);
+            if (isRequestExcluded(request)) {
+                LOG.trace("Request: {} is excluded from handling by Struts, passing request to other filters", uri);
                 chain.doFilter(request, response);
             } else {
-                LOG.trace("Checking if {} is a static resource", uri);
-                boolean handled = execute.executeStaticResourceRequest(request, response);
-                if (!handled) {
-                    LOG.trace("Uri {} is not a static resource, assuming action", uri);
-                    prepare.setEncodingAndLocale(request, response);
-                    prepare.createActionContext(request, response);
-                    prepare.assignDispatcherToThread();
-                    HttpServletRequest wrappedRequest = prepare.wrapRequest(request);
-                    ActionMapping mapping = prepare.findActionMapping(wrappedRequest, response, true);
-                    if (mapping == null) {
-                        LOG.trace("Cannot find mapping for {}, passing to other filters", uri);
-                        chain.doFilter(request, response);
-                    } else {
-                        LOG.trace("Found mapping {} for {}", mapping, uri);
-                        execute.executeAction(wrappedRequest, response, mapping);
-                    }
-                }
+                tryHandleRequest(chain, request, response, uri);
             }
         } finally {
             prepare.cleanupRequest(request);
         }
     }
 
+    private void tryHandleRequest(FilterChain chain, HttpServletRequest request, HttpServletResponse response, String uri) throws IOException, ServletException {
+        LOG.trace("Checking if: {} is a static resource", uri);
+        boolean handled = execute.executeStaticResourceRequest(request, response);
+        if (!handled) {
+            LOG.trace("Uri: {} is not a static resource, assuming action", uri);
+            handleRequest(chain, request, response, uri);
+        }
+    }
+
+    private void handleRequest(FilterChain chain, HttpServletRequest request, HttpServletResponse response, String uri) throws ServletException, IOException {
+        prepare.setEncodingAndLocale(request, response);
+        prepare.createActionContext(request, response);
+        prepare.assignDispatcherToThread();
+
+        HttpServletRequest wrappedRequest = prepare.wrapRequest(request);
+        try {
+            ActionMapping mapping = prepare.findActionMapping(wrappedRequest, response, true);
+            if (mapping == null) {
+                LOG.trace("Cannot find mapping for: {}, passing to other filters", uri);
+                chain.doFilter(request, response);
+            } else {
+                LOG.trace("Found mapping: {} for: {}", mapping, uri);
+                execute.executeAction(wrappedRequest, response, mapping);
+            }
+        } finally {
+            prepare.cleanupRequest(wrappedRequest);
+        }
+    }
+
+    private boolean isRequestExcluded(HttpServletRequest request) {
+        return excludedPatterns != null && prepare.isUrlExcluded(request, excludedPatterns);
+    }
+
     public void destroy() {
         prepare.cleanupDispatcher();
     }