WW-5199 Allow forwarding from/to actions

10088 · Dec 8, 2022 · afd04ea · afd04ea
1 parent 97691a1
commit afd04ea
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 22 deletions.
diff --git a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DispatcherResultTest.java b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DispatcherResultTest.java
@@ -43,13 +43,8 @@ public void testDispatchingToAction() throws Exception {
             webClient.getOptions().setThrowExceptionOnFailingStatusCode(false);
             final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/dispatcher/forward.action");
 
-            //DomElement div = page.getElementById("dispatcher-result");
-            //Assert.assertEquals("This page is a result of \"dispatching\" to it from an action", div.asNormalizedText());
-            // support for forwarding to another action is broken on StrutsPrepareFilter/StrutsExecuteFilter
-            // it only works in StrutsPrepareAndExecuteFilter
-            // this will be fixed in Struts 6.1.x
-
-            Assert.assertEquals(404, page.getWebResponse().getStatusCode());
+            DomElement div = page.getElementById("dispatcher-result");
+            Assert.assertEquals("This page is a result of \"dispatching\" to it from an action", div.asNormalizedText());
         }
     }
 

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java
@@ -52,6 +52,7 @@ public class PrepareOperations {
 
     private Dispatcher dispatcher;
     private static final String STRUTS_ACTION_MAPPING_KEY = "struts.actionMapping";
+    private static final String NO_ACTION_MAPPING = "noActionMapping";
     public static final String CLEANUP_RECURSION_COUNTER = "__cleanup_recursion_counter";
 
     public PrepareOperations(Dispatcher dispatcher) {
@@ -73,7 +74,7 @@ public ActionContext createActionContext(HttpServletRequest request, HttpServlet
         if (oldCounter != null) {
             counter = oldCounter + 1;
         }
-        
+
         ActionContext oldContext = ActionContext.getContext();
         if (oldContext != null) {
             // detected existing context, so we are probably in a forward
@@ -172,26 +173,32 @@ public ActionMapping findActionMapping(HttpServletRequest request, HttpServletRe
      * has already been found, otherwise, it creates it and stores it in the request.  No mapping will be created in the
      * case of static resource requests or unidentifiable requests for other servlets, for example.
      * @param forceLookup if true, the action mapping will be looked up from the ActionMapper instance, ignoring if there is one
-     * in the request or not 
+     * in the request or not
      *
      * @param request servlet request
      * @param response servlet response
      *
      * @return the action mapping
      */
     public ActionMapping findActionMapping(HttpServletRequest request, HttpServletResponse response, boolean forceLookup) {
-        ActionMapping mapping = (ActionMapping) request.getAttribute(STRUTS_ACTION_MAPPING_KEY);
-        if (mapping == null || forceLookup) {
+        ActionMapping mapping = null;
+
+        Object mappingAttr = request.getAttribute(STRUTS_ACTION_MAPPING_KEY);
+        if (mappingAttr == null || forceLookup) {
             try {
                 mapping = dispatcher.getContainer().getInstance(ActionMapper.class).getMapping(request, dispatcher.getConfigurationManager());
                 if (mapping != null) {
                     request.setAttribute(STRUTS_ACTION_MAPPING_KEY, mapping);
+                } else {
+                    request.setAttribute(STRUTS_ACTION_MAPPING_KEY, NO_ACTION_MAPPING);
                 }
             } catch (Exception ex) {
                 if (dispatcher.isHandleException() || dispatcher.isDevMode()) {
                     dispatcher.sendError(request, response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex);
                 }
             }
+        } else if (!NO_ACTION_MAPPING.equals(mappingAttr)) {
+            mapping = (ActionMapping) mappingAttr;
         }
 
         return mapping;

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsExecuteFilter.java b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsExecuteFilter.java
@@ -73,17 +73,10 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
 
         ActionMapping mapping = prepare.findActionMapping(request, response);
 
-        //if recursion counter is > 1, it means we are in a "forward", in that case a mapping will still be
-        //in the request, if we handle it, it will lead to an infinite loop, see WW-3077
-        Integer recursionCounter = (Integer) request.getAttribute(PrepareOperations.CLEANUP_RECURSION_COUNTER);
-
-        if (mapping == null || recursionCounter > 1) {
-            boolean handled = execute.executeStaticResourceRequest(request, response);
-            if (!handled) {
-                chain.doFilter(request, response);
-            }
-        } else {
+        if (mapping != null) {
             execute.executeAction(request, response, mapping);
+        } else if (!execute.executeStaticResourceRequest(request, response)) {
+            chain.doFilter(request, response);
         }
     }
 

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java b/core/src/main/java/org/apache/struts2/dispatcher/filter/StrutsPrepareFilter.java
@@ -86,7 +86,7 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
                 prepare.createActionContext(request, response);
                 prepare.assignDispatcherToThread();
                 request = prepare.wrapRequest(request);
-                prepare.findActionMapping(request, response);
+                prepare.findActionMapping(request, response, true);
             }
             chain.doFilter(request, response);
         } finally {