Bug 52586 - When requests are forwarded, the request attribute javax.servlet.forward.path_info is assigned an incorrect value.
When requests are forwarded, the request attribute javax.servlet.forward.path...
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Servlet & JSP API
7.0.23
PC Linux
: P2 major (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2012-02-02 18:49 UTC by diego.rivera.cr@gmail.com
Modified: 2012-02-05 21:58 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description diego.rivera.cr@gmail.com 2012-02-02 18:49:59 UTC
As per the servlet spec, the values for:

javax.servlet.forward.request_uri
javax.servlet.forward.context
javax.servlet.forward.servlet_path
javax.servlet.forward.path_info
javax.servlet.forward.query_string

MUST be set to the values from the very first servlet that serviced the current request.  However, it seems that Tomcat is populating path_info with the value of the LAST path_info serviced in the request (i.e. the final servlet that serviced the request).  It's possible the other value assignments are afflicted by similar defects, I've not explicitly tested for this.

Because of this issue, it's impossible to correctly reconstruct the original request's URI piecemeal by concatenating context+servlet_path+path_info+?+query_string, since path_info will contain the path to another servlet and thus this concatenation will produce a probably invalid URI target.

Therefore, creating "click here to retry" links is impossible without workaround code.

The critical issue here is the deviation from the servlet spec, which spells out the behavior of those attributes.
Comment 1 diego.rivera.cr@gmail.com 2012-02-02 21:48:48 UTC
The problem seems to be in StandardHostValve, on (or about) line 438:

    private boolean custom(Request request, Response response,
                             ErrorPage errorPage) {

        if (container.getLogger().isDebugEnabled())
            container.getLogger().debug("Processing " + errorPage);

        request.setPathInfo(errorPage.getLocation());

        try {
            // Forward control to the specified location
            ServletContext servletContext =
                request.getContext().getServletContext();

Here, evidently the pathinfo for the request being serviced is set to the location of the error page being forwarded to:

        request.setPathInfo(errorPage.getLocation());

This is evidently incorrect: pathInfo should be set to the path info of the topmost request on the stack, not the location of the error page being forwarded to (this explicitly contradicts the Servlet spec).

In my mind, this line should simply not exist.  I'll leave it up to you Tomcat experts to determine what the impact of that is.
Comment 2 Mark Thomas 2012-02-05 21:58:04 UTC
The second time this evening I have had to dig back more than 5 years into the svn history to figure out why some code is the way it is. It must be code archaeology week and but no-one bothered to tell me.

That line was added in r301993 which was a modification to 301883 which was an ugly hack to fix bug 20018.

With the current Tomcat code, there is no need for the hack (I didn't dig into when the hack became unnecessary) so the line can be removed.

For the record, only the path info was affected and then, only during error handling.

This has been fixed in trunk and 7.0.x and will be included in 7.0.26 onwards.