-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
CORS breaking change #2767
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
Comments
@aldas If I submit a fix PR, would you be willing to consider merging it? |
In which situation/use-case this change broke existing/expected behavior? I can not decide without knowing what we have broken. |
@aldas Previously, the handler that runs after the CORSMiddleware was still executed, but with this change it is skipped and the server now returns 401. |
@aldas Although this isn't specified by the Fetch standard, I would argue that no CORS middleware should block non-preflight requests; such requests should be let through. One reason is that not all requests that carry an fetch("//example.org", {mode:"no-cors", method:"POST"}) // carries an Origin header but isn't a CORS request There may be good reasons to block such a request, but a CORS middleware shouldn't be expected to do that. If the request's origin isn't allowed by the CORS configuration, the middleware should simply omit the relevant CORS headers from the response and let the browser fail the CORS check (if any). @y-kawawa That is what github.com/jub0bs/cors does, and you can make it work with Echo. |
Related: #2534 |
Thank you for adding echo support.
After pulling the latest master, I noticed the following:
#2732 introduces a breaking change.
Since CORS checks are ultimately enforced by the browser, I think it would be helpful to provide an option that preserves backward compatibility.
One more point. I’d like to work with a release cut from the latest master.
Under Semantic Versioning, any breaking change should trigger a major‑version bump.
However, this change doesn’t feel large enough to warrant a new major version, so—if we can keep backward compatibility—it would be preferable to ship it as a minor version update instead.
The text was updated successfully, but these errors were encountered: