Bug 59317

Summary: AsyncContextImpl breaks request URL containing spaces
Product: Tomcat 8 Reporter: Piotr Kubowicz <derbeth>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Severity: normal    
Priority: P2    
Version: 8.0.33   
Target Milestone: ----   
Hardware: PC   
OS: Linux   

Description Piotr Kubowicz 2016-04-13 14:56:57 UTC
The problem appears when performing async requests to URLs that contain a space in URI path. I have an application that performs an XHR POST to http://localhost:8080/api/alarms/:id. When I set id to 'foo bar' I get an error:

java.lang.IllegalStateException: Could not get HttpServletRequest URI: Illegal character in path at index 36: http://localhost:8080/api/alarms/foo bar
	at org.springframework.http.server.ServletServerHttpRequest.getURI(ServletServerHttpRequest.java:99)
	at org.springframework.web.util.UriComponentsBuilder.fromHttpRequest(UriComponentsBuilder.java:282)
	at org.springframework.web.util.WebUtils.isSameOrigin(WebUtils.java:814)
	at org.springframework.web.cors.DefaultCorsProcessor.processRequest(DefaultCorsProcessor.java:71)
	at org.springframework.web.servlet.handler.AbstractHandlerMapping$CorsInterceptor.preHandle(AbstractHandlerMapping.java:503)
	at org.springframework.web.servlet.HandlerExecutionChain.applyPreHandle(HandlerExecutionChain.java:134)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:954)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:893)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:968)
	at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:870)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:648)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:844)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:729)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:292)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:207)
	at org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:720)
	at org.apache.catalina.core.ApplicationDispatcher.doDispatch(ApplicationDispatcher.java:639)
	at org.apache.catalina.core.ApplicationDispatcher.dispatch(ApplicationDispatcher.java:605)
	at org.apache.catalina.core.AsyncContextImpl$1.run(AsyncContextImpl.java:229)
	at org.apache.catalina.core.AsyncContextImpl.doInternalDispatch(AsyncContextImpl.java:391)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:208)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:141)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
	at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:616)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
	at org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:392)
	at org.apache.coyote.http11.AbstractHttp11Processor.asyncDispatch(AbstractHttp11Processor.java:1715)
	at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:652)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1502)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1458)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.net.URISyntaxException: Illegal character in path at index 36: http://localhost:8080/api/alarms/foo bar
	at java.net.URI$Parser.fail(URI.java:2848)
	at java.net.URI$Parser.checkChars(URI.java:3021)
	at java.net.URI$Parser.parseHierarchical(URI.java:3105)
	at java.net.URI$Parser.parse(URI.java:3053)
	at java.net.URI.<init>(URI.java:588)
	at org.springframework.http.server.ServletServerHttpRequest.getURI(ServletServerHttpRequest.java:96)
	... 35 common frames omitted

Non-async requests don't cause any problem. I use Spring Framework 4.2.5.

Debugging revealed what happens:

URI constructor expects an escaped URI (/api/alarms/foo%20bar). When AsyncDispatcher runs, it passes a ServletRequest implementation which getRequestURI() returns /api/alarms/foo bar. Non-async request handling passes an implementation that returns the escaped URI.

AsyncContextImpl.dispatch() constructs a path from HttpServletRequest.getServletPath() and getPathInfo().
This path is used in ApplicationContext.getRequestDispatcher() where ApplicationDispatcher constructor is called with the path where it a request URI is expected.

Later ApplicationDispatcher.wrapRequest() creates an ApplicationHttpRequest with a correct request URI but ApplicationDispatcher.doDispatch() calls wrequest.setRequestURI() passing a path not a URI.

As result the servlet handling the request receives an ApplicationHttpRequest with an incorrect requestURI field ( identical to servletPath field), despite including a RequestFacade in the request field that returns the correct (escaped) value in getRequestURI().

I reproduced the bug on Tomcat 8.0.22, 8.0.30, 8.0.33 and 8.5.0 beta.
Comment 1 Mark Thomas 2016-04-22 20:02:39 UTC
See also bug 57559.

I'm close to marking this bug as a duplicate of that one but I do want to review the detail of exactly what is going on first.
Comment 2 Mark Thomas 2016-04-26 11:52:31 UTC
A few more notes as I investigate this.

Async and non-async behaviours are currently the same.
- Both expect the path used to obtain the dispatcher to be decoded. This behavior was confirmed with the Servlet EG.
- Both return the unencoded URI for req.getRequestURI(). That strikes me as wrong.
Comment 3 Mark Thomas 2016-04-26 13:41:30 UTC
The restriction the the request dispatcher (or the async dispatch) must be obtained with a decoded path has not changed. However, I have applied a fix that ensures that the result of the call to getRequestURI() after the dispatch returned an encoded URI.

This has been fixed in:
9.0.x for 9.0.0.M5 onwards
8.5.x for 8.5.1 onwards
8.0.x for 8.0.34 onwards
7.0.x for 7.0.70 onwards
Comment 4 Vincent Massol 2016-07-10 21:42:57 UTC

I believe this issue and the change brought with it are causing regressions in apps using Tomcat (such as XWiki).

See http://markmail.org/message/jwm5ip245empcghi

Do you think you could review this issue in light of this? 

Right now, in XWiki land we're starting to have more and more uses reporting issues with Tomcat 7.0.69+ and 8.0.33+ (see http://jira.xwiki.org/browse/XWIKI-13556). And I cannot unencode the path passed to the RequestDispatcher since that makes it fail with other servlet containers... We're a bit stuck.

Comment 5 seandawson2015 2016-08-22 16:05:26 UTC
So we ran into this too - with a customer who downloaded the latest rev of Tomcat 8.  Took us half the day yesterday and most of today to get to the bottom of it.  In troubleshooting, I tried using 7 instead but it was still failing there (and now I see that the change is in recent revs of all versions).

Is there a setting we can use to revert to the previous behavior?  Or what's the best way to workaround/fix this?

I'm not sure if it's in this particular set of changes, but if so, it looks like we might be able to turn it off...

Comment 6 Nitin Kamate 2017-02-08 18:12:04 UTC
The change done for this issue caused regression in the webapp and We wan into this of URL encoding. Few of the URLs that use : inside are getting encoded and the server is unable to locate the resource anymore.

e.g. /data/parent:children/id is getting encoded as /data/parent%3Achildren/id. Due to this the app does not locate the resources on v7.0.70 as it was doing earlier. Is it possible to allow turning-off this URL-encoding to retain previous behavior?
Comment 7 Mark Thomas 2017-02-08 18:41:41 UTC
Potential regressions should be followed up on the users list in the first instance and be accompanied by simple test cases (JSPs can be useful for this) that demonstrate the problem. That will enable us to identify where the problem lies.