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

[HttpFoundation] Add an helper method to check if a request is a CORS preflight request #34391

Closed
wants to merge 2 commits into from

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Nov 15, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets n/a
License MIT
Doc PR n/a

Add a new method to check if a request is a CORS preflight request according to the CORS spec.
It's useful in many cases (see api-platform/core#3265), and will allow to remove code from API Platform and NelmioCorsBundle.

@chalasr chalasr added this to the 4.4 milestone Nov 15, 2019
@teohhanhui
Copy link
Contributor

There is no such distinction between CORS (preflight) / non-CORS OPTIONS requests: nelmio/NelmioCorsBundle#54 (comment)

Any distinction we try to make is arbitrary and contrary to the spec.

@nicolas-grekas nicolas-grekas modified the milestones: 4.4, next Nov 17, 2019
@dunglas
Copy link
Member Author

dunglas commented Nov 17, 2019

@teohhanhui that’s wrong. Preflight requests are explicitly specified in the WHATWG’s CORS standard: https://fetch.spec.whatwg.org/#http-requests

See also https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request

*/
public function isCorsPreflightRequest(): bool
{
return $this->isMethod('OPTIONS') && $this->headers->has('Access-Control-Request-Method');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to also check for Origin and Access-Control-Request-Headers?

@GromNaN
Copy link
Member

GromNaN commented Dec 16, 2019

Maybe the whole CORS spec should be implemented in a separate class. The Request class contains already a lot of methods and I think it becomes complex.

@fabpot
Copy link
Member

fabpot commented Feb 3, 2020

I'm 👎 on adding yet another specific method on Request.

@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants