-
Notifications
You must be signed in to change notification settings - Fork 57
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
Handle OPTIONS cacheable #74
Conversation
So this would make sure that Preflight requests and OPTIONS requests are handled same as before, only the Vary header is added for the Methods, so preflight / non-preflight will be cached separately. Origin check is removed so it it doesn't create cache entries per origin. |
This looks perfect to me. |
src/CorsService.php
Outdated
@@ -166,7 +164,7 @@ private function configureAllowedMethods(Response $response, Request $request) | |||
if ($this->options['allowedMethods'] === true) { | |||
if ($this->options['supportsCredentials']) { | |||
$allowMethods = strtoupper($request->headers->get('Access-Control-Request-Method')); | |||
$this->varyHeader($response, 'Access-Control-Request-Method'); | |||
// Vary is always done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe more specific?
// Vary by Access-Control-Request-Method is added to every preflight request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweaked it a bit by just adding it and detect duplicates, in case the CorsService is used stand-alone or with a custom middleware.
FYI, I've tweaked it a bit in 03157d5 |
See #70 (comment) by @davidbarratt