atmosphere icon indicating copy to clipboard operation
atmosphere copied to clipboard

Atmosphere does not follow Servlet Spec 3.1 - Section 3.12 (Lifetime of the Request Object)

Open joakime opened this issue 8 years ago • 1 comments

This came up because of Atmosphere/atmosphere#1998 and eclipse/jetty.project#660

Per the Servlet Spec 3.1 - Section 3.12 (page 3-31) ...

Each request object is valid only within the scope of a servlet’s service method, or within the scope of a filter’s doFilter method, unless the asynchronous processing is enabled for the component and the startAsync method is invoked on the request object. In the case where asynchronous processing occurs, the request object remains valid until complete is invoked on the AsyncContext . Containers commonly recycle request objects in order to avoid the performance overhead of request object creation. The developer must be aware that maintaining references to request objects for which startAsync has not been called outside the scope described above is not recommended as it may have indeterminate results.

In case of upgrade, the above is still true.

The fact that Atmosphere pulls out and uses the HttpServletRequest object outside the scope of the request is in violation of the servlet spec.

In Atmosphere/atmosphere#1998, atmosphere appears to access the request object after the upgrade phase is complete (this is invalid).

In the case of eclipse/jetty.project#660 it appears that some sort of atmosphere cleanup code is attempting to access the HttpServletRequest object after an AsyncContext.complete()

This behavior is dangerous to you, your stability, and your security.

If you happen to access a request object outside of the scopes defined in the Servlet spec, you may very well get data belonging to a different request, or NPEs, or be modifying Sessions not belonging to the request you think.

joakime avatar Jun 24 '16 17:06 joakime

@joakime Atmosphere pull out the HttpServletRequest because of historical reason (Servlet 3.0 and lower) but clone its content most of the time. the Jetty implementation may have some left over when native is used, but when jsr356/AsyncContext is set by default the issue shouldn't arise.

jfarcand avatar Jun 27 '16 22:06 jfarcand