[cors] add OPTIONS status code + fix function typo #132
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit-master
force-pushed
the
fix-options-status-code
branch
4 times, most recently
from
2b29c06 to
f2fe3ba
Compare
elithrar
suggested changes
Sep 11, 2018
commit-master
force-pushed
the
fix-options-status-code
branch
from
f2fe3ba to
629ee45
Compare
elithrar
suggested changes
Sep 14, 2018
cors.go
Outdated
| // ExposeHeaders can be used to specify headers that are available | ||
| // OptionStatusCode sets a custom status code on the OPTIONS requests. | ||
| // Default behaviour sets it to 200 to reflect best practices. This is option is not mandatory | ||
| // and can be used if you need a custom status code (i.e 204) or have a monitoring |
There was a problem hiding this comment.
Remove the piece about monitoring middleware - as per my other comment, there are more robust ways to get the real status code in middleware when logging.
There was a problem hiding this comment.
Totally agree we should keep this generic. I'm removing it.
commit-master
force-pushed
the
fix-options-status-code
branch
from
629ee45 to
f49f2b7
Compare
elithrar
approved these changes
Sep 14, 2018
|
Thanks @commit-master! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I believe we should handle the StatusCode when receiving OPTIONS requests. Best practice is to respond with a
200or204code.200code seems to be the most used since it seems to work across any browser:https://stackoverflow.com/questions/46026409/what-are-proper-status-codes-for-cors-preflight-requests
We should let the user override this status code this is why I exposed the
OptionStatusCodemethod that take an int.