-
Notifications
You must be signed in to change notification settings - Fork 13
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
give simpleHeaders
to simpleCorsResourcePolicy
#26
base: master
Are you sure you want to change the base?
Conversation
It makes more sense to give `simpleHeaders` explicitely to the `simpleCorsResourcePolicy`, and by extension to `simpleCors`. The reason for this is that the preflight request chrome and firefox make vie OPTIONS includes `content-type`, resulting in a 400. As the documentation states, all `simpleHeaders` _except_ `content-type` are implicit when passing the empty list to `corsRequestHeaders`. I hope this is ok, let me know if I misunderstood the intention regarding `simpleCors`.
I had the same problems, and the fix above solved it. In my opinions this gives a better out-of-the-box experience. |
I hesitate to take this change without carefully reading the related sections in the CORS standard. The behavior of the server guides the browser to reject unsafe scenarios. Thus, a more liberal policy is always more convenient, but, generally, not safer. I am happy to merge this when I am convinced that it is in accordance with the standard, doesn't affect security for other users of the library, and doesn't break existing uses. In case of doubt, I prefer some inconvenience for some use cases. |
Unfortunately, I am not working with CORS on a daily basis and re-familiarizing myself with the standard always takes some time, which is the reason why this package is maintained rather conservatively. |
I appreciate that people use this package and file issues and submit PRs. And am sorry, that I am slow in keeping up with responding. |
In concrete, I am concerned that the change in the PR may conflict with the following paragraph from section 6.2 of the CORS standard:
|
No need to apologise! I for one appreciate that you are careful and methodical about these changes, I agree with your points about being conservative instead of convenient. I have to admit I have to spend more time parsing that section 6.2 to understand it :) |
It makes more sense to give
simpleHeaders
explicitely to thesimpleCorsResourcePolicy
, and by extension tosimpleCors
.The reason for this is that the preflight request chrome and firefox make vie OPTIONS includes
content-type
, resulting in a 400. As the documentation states, allsimpleHeaders
exceptcontent-type
are implicit when passing the empty list tocorsRequestHeaders
.I hope this is ok, let me know if I misunderstood the intention regarding
simpleCors
.