From ab727f3d16b38953dd35219f3e7d4e0abd594e18 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 8 May 2025 07:58:19 +0200 Subject: [PATCH 1/3] WW-5548 Defines proper request attributes when forwarding or including final path --- .../result/ServletDispatcherResult.java | 30 ++++++++++++----- .../result/ServletDispatcherResultTest.java | 33 ++++++++++++++++--- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java b/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java index feced67bb7..3d8aeb8f34 100644 --- a/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java +++ b/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java @@ -18,8 +18,6 @@ */ package org.apache.struts2.result; -import org.apache.struts2.ActionInvocation; -import org.apache.struts2.inject.Inject; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -28,9 +26,11 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.ActionInvocation; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsStatics; import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.inject.Inject; import org.apache.struts2.url.QueryStringParser; import java.io.Serial; @@ -158,13 +158,6 @@ public void doExecute(String finalLocation, ActionInvocation invocation) throws //if we are inside an action tag, we always need to do an include boolean insideActionTag = (Boolean) ObjectUtils.defaultIfNull(request.getAttribute(StrutsStatics.STRUTS_ACTION_TAG_INVOCATION), Boolean.FALSE); - // this should allow integration with third-party view related frameworks - if (finalLocation.contains("?")) { - request.setAttribute(RequestDispatcher.FORWARD_SERVLET_PATH, finalLocation.substring(0, finalLocation.indexOf('?'))); - } else { - request.setAttribute(RequestDispatcher.FORWARD_SERVLET_PATH, finalLocation); - } - // If we're included, then include the view // Otherwise do forward // This allow the page to, for example, set content type @@ -173,9 +166,28 @@ public void doExecute(String finalLocation, ActionInvocation invocation) throws request.setAttribute("struts.view_uri", finalLocation); request.setAttribute("struts.request_uri", request.getRequestURI()); + // These attributes are required when forwarding to another servlet, see specification: + // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#forwarded-request-parameters + request.setAttribute(RequestDispatcher.FORWARD_MAPPING, request.getHttpServletMapping()); + request.setAttribute(RequestDispatcher.FORWARD_REQUEST_URI, request.getRequestURI()); + request.setAttribute(RequestDispatcher.FORWARD_CONTEXT_PATH, request.getContextPath()); + request.setAttribute(RequestDispatcher.FORWARD_SERVLET_PATH, request.getServletPath()); + request.setAttribute(RequestDispatcher.FORWARD_PATH_INFO, request.getPathInfo()); + request.setAttribute(RequestDispatcher.FORWARD_QUERY_STRING, request.getQueryString()); + dispatcher.forward(request, response); } else { LOG.debug("Including location: {}", finalLocation); + + // These attributes are required when forwarding to another servlet, see specification: + // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#included-request-parameters + request.setAttribute(RequestDispatcher.INCLUDE_MAPPING, request.getHttpServletMapping()); + request.setAttribute(RequestDispatcher.INCLUDE_REQUEST_URI, request.getRequestURI()); + request.setAttribute(RequestDispatcher.INCLUDE_CONTEXT_PATH, request.getContextPath()); + request.setAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH, request.getServletPath()); + request.setAttribute(RequestDispatcher.INCLUDE_PATH_INFO, request.getPathInfo()); + request.setAttribute(RequestDispatcher.INCLUDE_QUERY_STRING, request.getQueryString()); + dispatcher.include(request, response); } } diff --git a/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java b/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java index c64c9d76ad..c143326654 100644 --- a/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java @@ -39,30 +39,54 @@ public void testForward() throws Exception { ServletDispatcherResult view = new ServletDispatcherResult(); view.setLocation("foo.jsp"); + request.setRequestURI("/app/namespace/my.action"); + request.setContextPath("/app"); + request.setServletPath("/namespace/my.action"); + request.setPathInfo(null); + request.setQueryString("a=1&b=2"); + request.setAttribute("struts.actiontag.invocation", null); request.setAttribute("jakarta.servlet.include.servlet_path", null); - request.setRequestURI("foo.jsp"); response.setCommitted(Boolean.FALSE); view.execute(invocation); assertEquals("foo.jsp", response.getForwardedUrl()); - assertEquals("foo.jsp", request.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH)); + + // Attributes required by Specification when forwarding to another resource + // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#forwarded-request-parameters + assertEquals("/app/namespace/my.action", request.getAttribute(RequestDispatcher.FORWARD_REQUEST_URI)); + assertEquals("/app", request.getAttribute(RequestDispatcher.FORWARD_CONTEXT_PATH)); + assertEquals("/namespace/my.action", request.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH)); + assertNull(request.getAttribute(RequestDispatcher.FORWARD_PATH_INFO)); + assertEquals("a=1&b=2", request.getAttribute(RequestDispatcher.FORWARD_QUERY_STRING)); } public void testInclude() throws Exception { ServletDispatcherResult view = new ServletDispatcherResult(); view.setLocation("foo.jsp"); + request.setRequestURI("/app/namespace/my.action"); + request.setContextPath("/app"); + request.setServletPath("/namespace/my.action"); + request.setPathInfo(null); + request.setQueryString("a=1&b=2"); + request.setAttribute("struts.actiontag.invocation", null); response.setCommitted(Boolean.TRUE); - request.setRequestURI("foo.jsp"); view.execute(invocation); assertEquals("foo.jsp", response.getIncludedUrl()); - assertEquals("foo.jsp", request.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH)); + + // These attributes must be set when including another resource + // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#included-request-parameters + assertEquals("/app/namespace/my.action", request.getAttribute(RequestDispatcher.INCLUDE_REQUEST_URI)); + assertEquals("/app", request.getAttribute(RequestDispatcher.INCLUDE_CONTEXT_PATH)); + assertEquals("/namespace/my.action", request.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH)); + assertNull(request.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO)); + assertEquals("a=1&b=2", request.getAttribute(RequestDispatcher.INCLUDE_QUERY_STRING)); } public void testWithParameter() throws Exception { @@ -76,7 +100,6 @@ public void testWithParameter() throws Exception { // See https://issues.apache.org/jira/browse/WW-5486 assertEquals("1", stack.findString("#parameters.bar")); - assertEquals("foo.jsp", request.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH)); } @Override From 95183cddfb457783354df2c9bcba95757175bea6 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 8 May 2025 08:02:13 +0200 Subject: [PATCH 2/3] Fies typo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../java/org/apache/struts2/result/ServletDispatcherResult.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java b/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java index 3d8aeb8f34..26c3852a2f 100644 --- a/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java +++ b/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java @@ -179,7 +179,7 @@ public void doExecute(String finalLocation, ActionInvocation invocation) throws } else { LOG.debug("Including location: {}", finalLocation); - // These attributes are required when forwarding to another servlet, see specification: + // These attributes are required when including another resource, see specification: // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#included-request-parameters request.setAttribute(RequestDispatcher.INCLUDE_MAPPING, request.getHttpServletMapping()); request.setAttribute(RequestDispatcher.INCLUDE_REQUEST_URI, request.getRequestURI()); From 7013118ff6cbaa51783b89608f313be67b148a22 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 8 May 2025 07:58:19 +0200 Subject: [PATCH 3/3] WW-5548 Drops RequestDispatcher parameters as they should be defined by Servlet container --- .../result/ServletDispatcherResult.java | 18 ------------------ .../result/ServletDispatcherResultTest.java | 17 ----------------- 2 files changed, 35 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java b/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java index 26c3852a2f..a57fd7a9a6 100644 --- a/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java +++ b/core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java @@ -166,28 +166,10 @@ public void doExecute(String finalLocation, ActionInvocation invocation) throws request.setAttribute("struts.view_uri", finalLocation); request.setAttribute("struts.request_uri", request.getRequestURI()); - // These attributes are required when forwarding to another servlet, see specification: - // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#forwarded-request-parameters - request.setAttribute(RequestDispatcher.FORWARD_MAPPING, request.getHttpServletMapping()); - request.setAttribute(RequestDispatcher.FORWARD_REQUEST_URI, request.getRequestURI()); - request.setAttribute(RequestDispatcher.FORWARD_CONTEXT_PATH, request.getContextPath()); - request.setAttribute(RequestDispatcher.FORWARD_SERVLET_PATH, request.getServletPath()); - request.setAttribute(RequestDispatcher.FORWARD_PATH_INFO, request.getPathInfo()); - request.setAttribute(RequestDispatcher.FORWARD_QUERY_STRING, request.getQueryString()); - dispatcher.forward(request, response); } else { LOG.debug("Including location: {}", finalLocation); - // These attributes are required when including another resource, see specification: - // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#included-request-parameters - request.setAttribute(RequestDispatcher.INCLUDE_MAPPING, request.getHttpServletMapping()); - request.setAttribute(RequestDispatcher.INCLUDE_REQUEST_URI, request.getRequestURI()); - request.setAttribute(RequestDispatcher.INCLUDE_CONTEXT_PATH, request.getContextPath()); - request.setAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH, request.getServletPath()); - request.setAttribute(RequestDispatcher.INCLUDE_PATH_INFO, request.getPathInfo()); - request.setAttribute(RequestDispatcher.INCLUDE_QUERY_STRING, request.getQueryString()); - dispatcher.include(request, response); } } diff --git a/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java b/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java index c143326654..12a4796b1e 100644 --- a/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java +++ b/core/src/test/java/org/apache/struts2/result/ServletDispatcherResultTest.java @@ -18,7 +18,6 @@ */ package org.apache.struts2.result; -import jakarta.servlet.RequestDispatcher; import org.apache.struts2.ActionContext; import org.apache.struts2.StrutsInternalTestCase; import org.apache.struts2.StrutsStatics; @@ -53,14 +52,6 @@ public void testForward() throws Exception { view.execute(invocation); assertEquals("foo.jsp", response.getForwardedUrl()); - - // Attributes required by Specification when forwarding to another resource - // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#forwarded-request-parameters - assertEquals("/app/namespace/my.action", request.getAttribute(RequestDispatcher.FORWARD_REQUEST_URI)); - assertEquals("/app", request.getAttribute(RequestDispatcher.FORWARD_CONTEXT_PATH)); - assertEquals("/namespace/my.action", request.getAttribute(RequestDispatcher.FORWARD_SERVLET_PATH)); - assertNull(request.getAttribute(RequestDispatcher.FORWARD_PATH_INFO)); - assertEquals("a=1&b=2", request.getAttribute(RequestDispatcher.FORWARD_QUERY_STRING)); } public void testInclude() throws Exception { @@ -79,14 +70,6 @@ public void testInclude() throws Exception { view.execute(invocation); assertEquals("foo.jsp", response.getIncludedUrl()); - - // These attributes must be set when including another resource - // https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#included-request-parameters - assertEquals("/app/namespace/my.action", request.getAttribute(RequestDispatcher.INCLUDE_REQUEST_URI)); - assertEquals("/app", request.getAttribute(RequestDispatcher.INCLUDE_CONTEXT_PATH)); - assertEquals("/namespace/my.action", request.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH)); - assertNull(request.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO)); - assertEquals("a=1&b=2", request.getAttribute(RequestDispatcher.INCLUDE_QUERY_STRING)); } public void testWithParameter() throws Exception {