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

Add HEAD to CORS ALL_METHODS list #1112

Merged
merged 4 commits into from
Apr 6, 2021
Merged

Conversation

jcwilson
Copy link
Contributor

The HEAD method is conspicuously absent from the allowed methods list when allow_methods="*" is
used. This doesn't really affect CORS preflight requests, as HEAD requests aren't preflighted by the
browser, but it does prevent the actual cross-origin HEAD response from being read by the calling
app.
This can catch people off-guard.

This simply adds HEAD to the ALL_METHODS list in the CORS middleware module and includes some
additional tests to validate the new behavior.

The HEAD method is conspicuously absent from the allowed methods list when `allow_methods="*"` is
used. This doesn't really affect CORS preflight requests, as HEAD requests aren't preflighted by the
browser, but it does prevent the actual cross-origin HEAD response from being read by the calling
app.
[This can catch people off-guard.](https://discuss.encode.io/t/for-cors-middleware-why-is-head-not-included-in-all-methods/939)

This simply adds HEAD to the `ALL_METHODS` list in the CORS middleware module and includes some
additional tests to validate the new behavior.
@falkben
Copy link
Contributor

falkben commented Dec 15, 2020

It does seem to be specified in the spec. https://fetch.spec.whatwg.org/ Do you know of any reason it might have been missing?

@jcwilson
Copy link
Contributor Author

Nope, I just discovered it when I found that the Access-Control-* response headers weren't present on HEAD requests. I'm not sure why it was omitted in the first place.

… check

Co-authored-by: euri10 <euri10@users.noreply.github.com>
@jcwilson
Copy link
Contributor Author

Hmm, this one's failing in the same way as #1111 with I think errors in the database code.

@JayH5 JayH5 added the cors Cross-Origin Resource Sharing label Feb 4, 2021
@jcwilson
Copy link
Contributor Author

jcwilson commented Apr 3, 2021

@euri10 I forgot about this one. Looks like tests are passing now.

@JayH5 JayH5 merged commit f5ecb53 into encode:master Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cors Cross-Origin Resource Sharing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants