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

CORS for websocket breaks existing CORS Filter [SPR-11443] #16069

Closed
spring-issuemaster opened this issue Feb 19, 2014 · 5 comments
Closed

CORS for websocket breaks existing CORS Filter [SPR-11443] #16069

spring-issuemaster opened this issue Feb 19, 2014 · 5 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Feb 19, 2014

Nils Rudolph opened SPR-11443 and commented

We have our own CORS Filter that applies CORS Headers to all responses.

Spring-websocket also sets the CORS Headers but for the Header "Access-Control-Allow-Origin" it adds the origin to the existing header again (e.g. "Access-Control-Allow-Origin" = "localhost:8080, localhost:8080").

The Browser does not accept these responses.
See http://www.w3.org/TR/cors/#access-control-allow-origin-response-header:
"In practice the origin-list-or-null production is more constrained. Rather than allowing a space-separated list of origins, it is either a single origin or the string "null"."

As Workaround we changed our CORSFilter and dont set the "Access-Control-Allow-Origin" for Websocket request and it works. But i think spring-websocket should either not set the "Access-Control-Allow-Origin" if it is already set or replace the existing value.


Affects: 4.0.1, 4.0.2

Issue Links:

  • #16063 Undocumented auto CORS for websocket endpoint breaking existing CORS code
  • #16308 Regression: AbstractHttpMessageConverter does not set ContentType
  • #16334 Improve Servlet 3 presence check in ServletServerHttpResponse

Referenced from: commits 49d7bda

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 5, 2014

Rossen Stoyanchev commented

I've made a change so that the SockJsService backs off and does not add any CORS headers if it detects the presence of "Access-Control-Allow-Origin" in the response. If you could please give this a try with 4.0.3.BUILD-SNAPSHOT to confirm it works.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 5, 2014

Prashant Deva commented

is Access-Control-Allow-Origin the only cors header that spring adds or does it add any other cors related header too?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 6, 2014

Rossen Stoyanchev commented

The SockJS service adds a few others as well. The idea however with the fix is that CORS headers are either managed centrally (e.g. through a Filter) or otherwise the SockJS service can provide them as well. This seems simpler than having CORS managed partly by a Filter and partly by the SockJS service. Furthermore in the case of HTTP OPTIONS those may not even get to the SockJS service as CORS filters typically handle them in full and don't delegate further to the filter chain. So we use the presence of Access-Control-Allow-Origin as a trigger to back off. You can see the logic here.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 6, 2014

Prashant Deva commented

can you document all the headers that are added and their values, so we can ensure our cors filter adds them too.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 6, 2014

Rossen Stoyanchev commented

Yes, that'll be included with #16063.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.