[
https://issues.apache.org/jira/browse/WW-5548?focusedWorklogId=968907&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-968907
]
ASF GitHub Bot logged work on WW-5548:
--------------------------------------
Author: ASF GitHub Bot
Created on: 08/May/25 06:44
Start Date: 08/May/25 06:44
Worklog Time Spent: 10m
Work Description: MFAshby commented on code in PR #1265:
URL: https://github.com/apache/struts/pull/1265#discussion_r2078993654
##########
core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java:
##########
@@ -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:
Review Comment:
I think setting these is the responsibility of the servlet container e.g.
tomcat, inside the implementation of the `forward` method. We should not have
to do it here at all.
##########
core/src/main/java/org/apache/struts2/result/ServletDispatcherResult.java:
##########
@@ -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());
Review Comment:
I think this is missing a guard to set the attribute only if it is not
already set. There can be multiple forwards, and this attribute should contain
the _original_ request as far as I understand.
Per the other comment I think this is redundant, the servlet container
should be setting these properties and we should not touch them.
Issue Time Tracking
-------------------
Worklog Id: (was: 968907)
Time Spent: 50m (was: 40m)
> Request attribute jakarta.servlet.forward.servlet_path incorrectly set by
> ServletDispatcherResult breaking sitemesh integration
> -------------------------------------------------------------------------------------------------------------------------------
>
> Key: WW-5548
> URL: https://issues.apache.org/jira/browse/WW-5548
> Project: Struts 2
> Issue Type: Bug
> Components: Dispatch Filter
> Affects Versions: 7.0.0
> Reporter: Martin
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.1.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> The servlet request parameter jakarta.servlet.forward.servlet_path has the
> following documentation in tomcat:
> {noformat}
> The name of the request attribute that should be set by the container when
> the forward(ServletRequest, ServletResponse) method is called. It provides
> the original value of a path-related property of the request. See the chapter
> "Forwarded Request Parameters" in the Servlet Specification for details.
> {noformat}
> Strut's org.apache.struts2.result.ServletDispatcherResult#doExecute method is
> breaking this spec in a couple of ways:
> # it's setting the parameter to be the path that the request is about to be
> _forwarded_ to, not the _original_ path.
> # it's setting the parameter itself, when that should be the responsibility
> of the container (in this case, Tomcat) when forward() is invoked.
> Documentation for jakarta.servlet.forward.servlet_path
> https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#forwarded-request-parameters
> Tomcat container implementation of forward() which is responsible for setting
> the parameter
> https://github.com/apache/tomcat/blob/99ebd1e375297ad02846322e85d533c61077a7dc/java/org/apache/catalina/core/ApplicationDispatcher.java#L241
> This is breaking integration with other tools which expect the
> FORWARD_SERVLET_PATH to be set correctly, such as sitemesh (from version 3.1
> onwards)
> {code:xml}
> <sitemesh>
> ....
> <mapping path="/auth/*.action" decorator="/WEB-INF/decorators/main.jsp"/>
> ....
> </sitemesh>
> {code}
> Sitemesh now picks up the incorrectly set FORWARD_SERVLET_PATH which in my
> case is mapped to a JSP in struts.xml like so
> {code:xml}
> <struts>
> ...
> <package name="authoritative links" extends="pkb-default" namespace="/auth"
> strict-method-invocation="true">
> <action name="dashboard" class="com.pkb.action.DashboardAction">
> <result name="success">/WEB-INF/view/dashboard.jsp</result>
> </action>
> ...
> {code}
> and my decorator is no longer applied, when it was previously.
> This behaviour was introduced in WW-5463
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
