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

Allowing all headers should be removed #23

Closed
neomerx opened this issue Aug 3, 2016 · 5 comments
Closed

Allowing all headers should be removed #23

neomerx opened this issue Aug 3, 2016 · 5 comments
Assignees

Comments

@neomerx
Copy link
Owner

neomerx commented Aug 3, 2016

Currently there is a config option Settings::VALUE_ALLOW_ALL_HEADERS which should allow all headers pass through CORS. It works fine for internal lib logic. No problem here. The problem is that this value * is actually sent to client in Access-Control-Allow-Headers and browser don't understand this value.

It looks the only possible way is listing all allowed headers and special * should be removed.

It was added mostly to make development easier. However since logging has been added to the lib this feature is not so important.

It is recommended avoid using Settings::VALUE_ALLOW_ALL_HEADERS and just list all allowed headers in Settings::KEY_ALLOWED_HEADERS

@neomerx
Copy link
Owner Author

neomerx commented Aug 3, 2016

Proposed solution: mark the constant as depreciated and do not sent it to client (it's not recognizable anyway).

@neomerx neomerx closed this as completed in cc0413d Aug 3, 2016
@neomerx
Copy link
Owner Author

neomerx commented Aug 3, 2016

link here original idea #15

@barryvdh
Copy link

Allow all headers could just return all requested headers, right?

@neomerx
Copy link
Owner Author

neomerx commented Feb 15, 2017

@barryvdh It's related to Access-Control-Allow-Headers header. On preflight request CORS should reply with a list of headers it allows browser to send in the actual request (which comes right after the preflight request). Initially I thought it would be a good idea to allow all headers (mostly for debugging purposes). Though the spec do not have any possibilities to 'allow all headers'. They must be listed one by one.
As far as I understand your question you're asking what if sending back all the headers CORS received in Access-Control-Request-Headers. IMHO it would look as a security hole because it undermines the whole idea about 'allowed headers'. I found logging Access-Control-Request-Headers headers which are not allowed to be a better solution for debugging.

@neomerx
Copy link
Owner Author

neomerx commented Feb 15, 2017

@barryvdh BTW if you think it would be better to send all them back you can easily override it here and here.

Also https://github.com/neomerx/cors-psr7#advanced-usage

If you have any further questions please don't hesitate to ask.

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

No branches or pull requests

2 participants