-
Notifications
You must be signed in to change notification settings - Fork 32
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
v1 - Bring us up to spec 🎉 #51
Comments
This comment has been minimized.
This comment has been minimized.
@tim-phillips Seems like ya'll lost steam on this. I'd like to help. I've recently started building a Next project and will need to handle some CORS stuff in my API Routes (which use If I start working on some PRs, will you be available to review and help clarify anything I'm confused about? |
@metamas Yes! We lost steam because the steps are not clear right now. We need to review the spec and create actionable issues to bring this lib to spec. It can be useful before you create a PR to create an Issue to discuss what you're planning, or we can chat about it in this thread if you think that's a better place for it. |
@tim-phillips Disregard what I wrote before editing this comment. I initially misunderstood your message. So, what needs to happen is a thorough review of how the current state of the lib compares to the spec. Is that correct? |
@metamas Yes, precisely. This lib does work fine, it just doesn’t follow the spec exactly. |
I would propose the following:
that partially covers part of the spec |
@tim-phillips I'll fork and approach this by simply reading through the spec. As I do so, I'll check that the project has tests for specified behavior. If it does, I'll check their validity. If it doesn't, I'll add tests and work on getting them to pass (if they don't already). I'll also return here to add to this list. Ok. So that's my intention. Now, I just need to manage enough free-time to get it done. 😅 Any suggestions on my approach? |
I opened up a PR with some changes to the Appreciate anyone taking the time to look it through. Keen on getting |
In order to bring this library up to the CORS spec, we need to accomplish the following tasks. Please review the spec and add to this list!!
Simple Cross-Origin Request, Actual Request, and Redirects
Origin
header in the request is not present, stop adding headers and run handlerOrigin
header in the request does not match exactly, stop adding headers and run handlerallowCredentials
is true, SetAccess-Control-Allow-Origin
to the value of theOrigin
header (is the client responsible for rejecting ifOrigin
is*
?)Preflight Request
204
as empty response? Allow user to change response code?Origin
header in the request is not present, stop adding headers and return emptyOrigin
header in the request does not match exactly, stop adding headers and return emptyDynamic
Access-Control-Allow-Origin
Vary
header Correctly use Vary header #58Specification
Supplementary reading
cc @bukinoshita @infernalmaster @lemol
The text was updated successfully, but these errors were encountered: