On Thu, Jun 4, 2026 at 11:01 AM Mark Thomas <[email protected]> wrote:
>
> On 04/06/2026 07:59, Rémy Maucherat wrote:
> > Hi,
> >
> > There are some discrepancies in URL encoding in the request dispatcher
> > for jakarta.servlet.include.context_path.
> > - Request.getContextPath is URL encoded, this is settled
> > - jakarta.servlet.forward.context_path saw a bug come in some time
> > ago, and it was switched to being URL encoded
> > - jakarta.servlet.include.context_path remains in the decoded camp,
> > but there's some mixed matching with a comparison with a
> > request.getContextPath, which is incorrect (lines 698, 701 and 703)
> >
> > The specification language is not very clear.
> > "These attributes are accessible from the included servlet via the
> > getAttribute method on the
> > request object and their values must be equal to the request URI,
> > context path, servlet path,
> > mapping, path info, and query string of the included servlet, respectively."
> >
> > I would think "jakarta.servlet.include.context_path" should be URL
> > encoded, rather than decoded as it is now, with changes to be made to
> > lines 387 and 703 to use context.getEncodedPath().
> >
> > Comments ?
>
> For Tomcat 11.0.x and earlier:
>
> - Request.getContextPath() is not decoded and not normalized.
> (/abc/../def is a valid return value)
>
> For Tomcat 12.0.x onwards:
> - Request.getContextPath() may be (partially) normalized and
> may be (partially) decoded.
>
>
> I feared this could get very messy. Having looked at it carefully, I
> think the fix is surprisingly simple:
>
> - line 387 should use this.context.getEncodedPath()
>
> - line 701 should use houterRequest.getServletContext().getContextPath()
>
> My reasoning for the 701 change is we want to identify if the contexts
> are the same. To do that we need the canonical context path not the
> request.getContextPath() form that may or may not be normalized and may
> or may not be encoded.
>
> Does that make sense?
On line 698 houterRequest.getAttribute(INCLUDE_CONTEXT_PATH) is
encoded now (with the change from line 387 to use encoded for the
attribute) so it won't match on line 703 then. Simply URL decoding the
attribute would be possible. Did I miss something ?
Since we have one encoded path and one decoded path. maybe do that ?
Object contextPath =
houterRequest.getAttribute(INCLUDE_CONTEXT_PATH);
if (contextPath == null) {
// Forward
contextPath =
houterRequest.getServletContext().getContextPath();
crossContext = !context.getPath().equals(contextPath);
} else {
crossContext =
!context.getEncodedPath().equals(contextPath);
}
But you're right about it being a mess, so let's forget about it and start over.
I think the problems started with change to set this in froward:
wrequest.setContextPath(context.getEncodedPath());
https://bz.apache.org/bugzilla/show_bug.cgi?id=63275
https://github.com/apache/tomcat/commit/4f1b667f15e6ed46d8704687dab5da73e7404552
Then it was not allowed by the spec, Request.getContextPath had to be
encoded then. But now I think we should revert this change in Tomcat
12 since the code is different and Tomcat has chosen to have
Request.getContextPath() return the decoded path (it is now allowed in
the specification).
The code review likes that solution (and pretty much hates all the
others since it then wants to change things everywhere for consistency
and correctness).
Then we can think about the previous Tomcat versions ...
Rémy
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
