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

Header not set when response is bigger than bufferSize #26

Closed
ralscha opened this issue Jul 26, 2014 · 5 comments
Closed

Header not set when response is bigger than bufferSize #26

ralscha opened this issue Jul 26, 2014 · 5 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@ralscha
Copy link

ralscha commented Jul 26, 2014

Hi

I'm fiddling with M1 and found two issues. I created a spring boot application that demonstrates the problem: https://github.com/ralscha/session

The application uses the HeaderHttpSessionStrategy and the response contains the x-auth-token header when the response is json and smaller than the default response buffer size of 8192. (http://localhost:8080/big)

The response is missing the x-auth-token header when the response is bigger than 8192 (http://localhost:8080/bigger)

The same happens when the StringHttpMessageConverter is used.
(http://localhost:8080/string). In this case the size does not matter. Even a small response does not contain the x-auth-token

As a workaround for the first problem I can simply increase the buffer

response.setBufferSize(9000);

Found no workaround for the String problem.

Ralph

@rwinch rwinch added this to the 1.0.0 M2 milestone Jul 30, 2014
@rwinch rwinch added the bug label Jul 30, 2014
@rwinch rwinch self-assigned this Jul 30, 2014
@rwinch rwinch closed this as completed in 0d217c6 Jul 31, 2014
@rwinch
Copy link
Member

rwinch commented Jul 31, 2014

Thanks for reporting this issue. The problem was related to the fact that the OnCommitedResponseWrapper was not triggering when the response length was reached. Specifically the servlet specification states that the response should be considered committed if:

The amount of content specified in the setContentLength method of the response has been greater than zero and has been written to the response.

This means that the session header was not written until after the response was committed. Since the response was committed, the header was not actually included.

I have fixed this in master which should resolve your issue. I have verified that the fix resolves your sample application. If you get a chance, "Can you give the latest snapshot a try?"

@ralscha
Copy link
Author

ralscha commented Aug 1, 2014

Hi. Thanks for the fix. The StringHttpMessageConverter url (http://localhost:8080/string) does work now, but a request to http://localhost:8080/bigger still does not contain the x-auth-token response header.
I use this snapshot: spring-session-1.0.0.BUILD-20140801.000037-56.jar

@rwinch
Copy link
Member

rwinch commented Aug 1, 2014

@ralscha Thanks for the feedback. Locally I had tested this using Jetty as well (which all the URLs work on). It appears that somehow the URL for bigger has since broke again on Tomcat. I will look into resolving this. Thanks again for the report and double checking my work!

@rwinch rwinch reopened this Aug 1, 2014
@rwinch rwinch closed this as completed in 4eb1ac0 Aug 1, 2014
rwinch pushed a commit that referenced this issue Aug 1, 2014
@rwinch
Copy link
Member

rwinch commented Aug 1, 2014

@ralscha The second problem was related to the fact that the OnCommitedResponseWrapper was not triggering when the buffer size was reached. As alluded to, this only impacted Tomcat and not Jetty.

I have fixed this in master which should resolve your issue. I have verified the URLs on both Tomcat and Jetty now work. If you get a chance, "Can you give the latest snapshot a try?"

Thanks again for trying Spring Session and providing critical feedback!

@ralscha
Copy link
Author

ralscha commented Aug 3, 2014

Hi. Looks good, the response always contains the header.
Thanks for your work.

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

No branches or pull requests

2 participants