Bug 57559

Summary: Decoded Request URI used for Asynchronous dispatch
Product: Tomcat 7 Reporter: cg.throwaway
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED WONTFIX    
Severity: normal    
Priority: P2    
Version: 7.0.53   
Target Milestone: ---   
Hardware: PC   
OS: All   

Description cg.throwaway 2015-02-10 16:02:19 UTC
Overview:

In a scenario where an original request contains part of the URI percent-encoded (e.g. /context/foo/%24/bar), when the request is put into asynchronous mode using ServletRequest.startAsync() and then dispatched using the no-argument AsyncContext.dispatch() method, the dispatch is incorrectly made to a percent-decoded URI (e.g. /context/foo/$/bar), not the original request URI.

Steps to Reproduce:

Consider the following servlet: https://gist.github.com/anonymous/957122b45ae656a5e8f1
It puts a request into asynchronous mode using ServletRequest.startAsync() and immediately dispatches it using the no-argument AsyncContext.dispatch() method. When the asynchronous dispatch takes place, the request URI is written to the response.

1) Compile and deploy the servlet to http://localhost:8080/context
2) Visit http://localhost:8080/context/foo/%24/bar (or any similar percent-encoded URI) in your browser

Actual Results:

The printed-out request URI is /context/foo/$/bar (i.e. the request has been dispatched to /context/foo/$/bar, a percent-decoded version of the original URI)

Expected Results:

The printed-out request URI is /context/foo/%24/bar (i.e. the request should be dispatched to /context/foo/%24/bar, the original URI)

Additional Information:

The Java Servlet 3.0 Specification says: "The dispatch method takes no argument. It uses the original URI as the path. If the AsyncContext was initialized via the startAsync(ServletRequest, ServletResponse) and the request passed is an instance of HttpServletRequest, then the dispatch is to the URI returned by HttpServletRequest.getRequestURI(). Otherwise the dispatch is to the URI of the request when it was last dispatched by the container."

It seems clear that the dispatch should be to the original request URI as obtained from HttpServletRequest.getRequestURI(), and the Javadoc of HttpServletRequest.getRequestURI() states that "The web container does not decode this String." Therefore, it seems the asynchronous dispatch should not be made to a percent-decoded version of the original URI, but to the original URI as-is.
Comment 1 Mark Thomas 2015-02-10 22:54:01 UTC
The specification text you quoted skips over a few things. The main one is how tricky it is to split an undecoded URI into contextPath, servletPath and pathInfo. See the getContextPath() implementation in [1] for an idea of just how messy this could get. I have no desire to see that sort of code in something that is meant to be a convenience method.

Elsewhere in the Servlet spec (including for async) dispatches are handled in terms of decoded paths relative to a context root. The convenience dispatch() methods needs to be handled the same way to avoid a whole pile of unnecessary complexity.

If you need the original request URI it is available via the usual request attribute.

I'll raise this with the Servlet EG to see if the wording of this can be changed/improved but - as far as Tomcat is concerned - this is a WONTFIX. Note if the EG opt for the behavior you are asking for I'll re-open this issue.

I've added a test case that confirms the observed behavior.


[1] http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?view=annotate
Comment 2 Konstantin Kolinko 2015-02-11 00:04:04 UTC
Previous discussion of this issue, ~8 months ago (June 2014):

"Decoded URL set on asynchronous request"
http://tomcat.markmail.org/thread/e33tqu7ayp2fguh4
Comment 3 cg.throwaway 2015-02-18 21:17:47 UTC
(In reply to Konstantin Kolinko from comment #2)
> Previous discussion of this issue, ~8 months ago (June 2014):
> 
> "Decoded URL set on asynchronous request"
> http://tomcat.markmail.org/thread/e33tqu7ayp2fguh4

Yeah, I did find this discussion but it wasn't very clear what conclusion you guys came to. There was talk of opening a bug but I couldn't find any when I searched so I figured I would open this one.

(In reply to Mark Thomas from comment #1)
> I'll raise this with the Servlet EG to see if the wording of this can be
> changed/improved but - as far as Tomcat is concerned - this is a WONTFIX.
> Note if the EG opt for the behavior you are asking for I'll re-open this
> issue.

Thanks for the detailed reply. I'm still a bit confused about the outcome though... are you saying that this *is* a Tomcat bug but it won't be fixed (hence the WONTFIX resolution), or that it *isn't* a bug because you believe Tomcat's current behaviour is what the servlet spec intended, or something else entirely?
Comment 4 Mark Thomas 2015-02-19 08:55:30 UTC
I believe - and discussions with the Servlet EG have confirmed that other EG members agree - that Tomcat's behaviour is as the specification intended. I'd normally close bugs like this as invalid but went for won't fix because the intended spec behaviour at the time was unclear.
Comment 5 Mark Thomas 2016-04-22 20:01:25 UTC
Thread from EG.
Comment 6 Mark Thomas 2016-04-26 13:03:17 UTC
This time actually add the link.

https://java.net/projects/servlet-spec/lists/jsr369-experts/archive/2015-02/message/18