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

cors: Don't return an error for preflight requests with non-matching origin #224

Merged
merged 3 commits into from
Mar 9, 2022
Merged

cors: Don't return an error for preflight requests with non-matching origin #224

merged 3 commits into from
Mar 9, 2022

Conversation

jplatte
Copy link
Collaborator

@jplatte jplatte commented Feb 21, 2022

Depends on #219, will rebase after merge.

This PR contains behavioral changes.

This is the third and final preliminary refactoring for the implementation of the new CORS API I proposed in #194 (comment).

Motivation

As far as my understanding goes, there is no good reason to return an error status code when a preflight request is made from an origin that isn't configured to be allowed (since this whole thing is a browser-side security feature).

Solution

Just return the configured list of allowed origins or in case a closure is set, an empty list, on origin "mismatch". This should make the CORS middleware a lot more predictable and improve the debugging experience in cases where you haven't gotten the CORS rules correct yet.

@jplatte jplatte changed the title Cors refactor 3 cors: Don't return an error for preflight requests with non-matching origin Feb 21, 2022
… unless it's a closure, in which case this is of course impossible.
Moves the work of concatenating multiple origins to layer construction
time (from layer call time).
@jplatte jplatte added this to the 0.3 milestone Feb 22, 2022
@jplatte jplatte requested a review from davidpdrsn March 8, 2022 10:49
Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

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

Great! Thanks 😃

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

Successfully merging this pull request may close these issues.

2 participants