Skip to content

Add HEAD to CORS ALL_METHODS list#1112

Merged
JayH5 merged 4 commits intoKludex:masterfrom
jcwilson:cors-all-methods
Apr 6, 2021
Merged

Add HEAD to CORS ALL_METHODS list#1112
JayH5 merged 4 commits intoKludex:masterfrom
jcwilson:cors-all-methods

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 Kludex: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