Mark,
I agree that using the return of getRequestURI as the target is a bad idea
for several reasons. Not only can it be encoded, the request can be
wrapped and the wrapper can return arbitrary rubbish!
If we look at the examples given in the javadoc I think it shows how it
really should work:
// REQUEST dispatch to /url/A
AsyncContext ac = request.startAsync();
...
ac.dispatch(); // ASYNC dispatch to /url/A
// REQUEST to /url/A
// FORWARD dispatch to /url/B
request.getRequestDispatcher("/url/B").forward(request,response);
// Start async operation from within the target of the FORWARD
// dispatch
ac = request.startAsync();
...
ac.dispatch(); // ASYNC dispatch to /url/A
// REQUEST to /url/A
// FORWARD dispatch to /url/B
request.getRequestDispatcher("/url/B").forward(request,response);
// Start async operation from within the target of the FORWARD
// dispatch
ac = request.startAsync(request,response);
...
ac.dispatch(); // ASYNC dispatch to /url/B
I think this is saying that if startAsync() is used, then the original
dispatch target is used: /usr/A. If startAsync(request,response) is used
then the last dispatch target is used. In this example it is /url/B
regardless of what the request wrapper says.
So how about we change the paragraph
If the asynchronous cycle was started with
ServletRequest.startAsync(ServletRequest,
ServletResponse)
<eclipse-javadoc:%E2%98%82=apache-jsp/%5C/home%5C/gregw%5C/.m2%5C/repository%5C/javax%5C/servlet%5C/javax.servlet-api%5C/3.1.0%5C/javax.servlet-api-3.1.0.jar%3Cjavax.servlet(AsyncContext.class%E2%98%83AsyncContext~dispatch%E2%98%82ServletRequest%E2%98%82startAsync%E2%98%82ServletRequest%E2%98%82ServletResponse>,
and the request passed is an instance of HttpServletRequest, then the
dispatch is to the URI returned by
javax.servlet.http.HttpServletRequest.getRequestURI
<eclipse-javadoc:%E2%98%82=apache-jsp/%5C/home%5C/gregw%5C/.m2%5C/repository%5C/javax%5C/servlet%5C/javax.servlet-api%5C/3.1.0%5C/javax.servlet-api-3.1.0.jar%3Cjavax.servlet(AsyncContext.class%E2%98%83AsyncContext~dispatch%E2%98%82javax.servlet.http.HttpServletRequest%E2%98%82getRequestURI>.
Otherwise, the dispatch is to the URI of the request when it was last
dispatched by the container.
to
If the asynchronous cycle was started with
ServletRequest.startAsync(ServletRequest,ServletResponse) then the dispatch
is to the URI of the last dispatch prior to the startAsync of type REQUEST,
ASYNC or FORWARD. Otherwise the dispatch is to the URI of the last
dispatch prior to the startAsync of type REQUEST or ASYNC.
Using the precise DispatcherType should avoid any confusion.
This is what jetty is doing, which I think it equivalent to what you
describe tomcat is doing in 99.999% of circumstances.
cheers
On 11 February 2015 at 09:53, Mark Thomas <markt_at_apache.org> wrote:
> The current definition of AsyncContext.dispatch() states:
>
> <quote>
> ... the dispatch is to the URI returned by
> HttpServletRequest.getRequestURI()
> </quote>
>
> This is problematic when implementing when the request URI is something
> like:
>
> "/foo/bar/../../abc/../f%24o/bar"
>
> It would be far simpler (and this is what Tomcat does) to map
> AsyncContext.dispatch() to AsyncContext.dispatch(servletPath + pathInfo)
>
> The main difference is that request.getRequestURI() returns a decoded
> and normalized value with Tomcat's approach as opposed to a undecoded,
> unnormalized value using the spec described approach.
>
> Note that the original undecoded, unnormalized URI is always available
> via a request attribute.
>
> What are other containers doing?
>
> Is Tomcat's approach acceptable?
>
> If yes, should the spec be changed to reflect this? (If we do we
> *really* need to address SERVLET_SPEC_18.)
>
> Mark
>
--
Greg Wilkins <gregw_at_intalio.com> @ Webtide - *an Intalio subsidiary*
http://eclipse.org/jetty HTTP, SPDY, Websocket server and client that scales
http://www.webtide.com advice and support for jetty and cometd.