Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
joakime opened this issue Jun 24, 2016 · 1 comment

Comments

@joakime
Copy link

joakime commented Jun 24, 2016

This came up because of #1998 and jetty/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 #1998, atmosphere appears to access the request object after the upgrade phase is complete (this is invalid).

In the case of jetty/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.

@jfarcand
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants