spring-projects / spring-session

Spring Session
https://spring.io/projects/spring-session
Apache License 2.0
1.86k stars 1.12k forks source link

Issue with CookieHttpSessionStrategy and Undertow #552

Closed ssi-paulsideleau closed 3 years ago

ssi-paulsideleau commented 8 years ago

Spring Session 1.2.0

If you do an include to a JSP page and the content of the JSP page exceeds the response buffer size, SpringSession will not be able to write the set-cookie response header if the app is running on Undertow. That's b/c undertow ignores the response.addCookie when called within an include. See http://lists.jboss.org/pipermail/undertow-dev/2015-June/001265.html. You may want to eagerly write the cookie as soon as a new session id is created to minimize this and/or reconsider supporting URL encoding.

rwinch commented 8 years ago

Thanks for the report @ssi-paulsideleau!

This does seem like a problem.

I'm not sure there is a lot we can do about it. If we eagerly create the session, then users will be mad that we created a session before one was requested (we don't know that they are going to create the session in the first place).

We do not want to use URL encoding because this leads to security vulnerabilities like session fixation attacks.

I'll have to give this some thought, but I think the answer for now is don't create the session inside the include (you must create it before hand). I'm certainly open to ideas, but we need to keep in mind the constraints I mentioned above.

ssi-paulsideleau commented 8 years ago

The problem is the session is created before the the sevlet does an include. However, spring session doesn't try to write the cookie until the finally block of the filter by which time it's too late. The buffer has already been flushed. What I did as a workaround was to write the cookie in a httpservletrequestwrapper and overrode the get session methods. If a session is created, then immediately write the cookie. I could try to submit my changes as a pull request if u approve of the idea.

rwinch commented 8 years ago

@ssi-paulsideleau Thanks for the clarification. I wasn't aware you were creating the session before the include.

I'm not sure we want to write the cookie to the response immediately because it may cause problems if someone does something like this:

request.getSession();
request.changeSessionId();

If we write the cookie immediately, then the response would have two session cookies in it. We can explore if this is a viable option, but I have a feeling this may cause problems.

Another option is that we can add a hook in HttpServletRequest that would ensure the cookie is written just before someone invokes:

request.getRequestDispatcher("/foo").include(request, response);

I'm leaning towards adding a hook in the RequestDispatcher at the moment. What are your thoughts?

ssi-paulsideleau commented 8 years ago

I agree with you. I think that is a safer approach

ssi-paulsideleau commented 8 years ago

Just an FYI that I implemented the approach you recommended and it works perfectly. Thank you.

rwinch commented 8 years ago

@ssi-paulsideleau Awesome! Thanks for the update? Would you be interested in sending a PR?

spring-projects-issues commented 3 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

spring-projects-issues commented 3 years ago

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.