[cors] add OPTIONS status code + fix function typo#132
Merged
elithrar merged 1 commit intogorilla:masterfrom Sep 14, 2018
Merged
[cors] add OPTIONS status code + fix function typo#132elithrar merged 1 commit intogorilla:masterfrom
elithrar merged 1 commit intogorilla:masterfrom
Conversation
2b29c06 to
f2fe3ba
Compare
elithrar
suggested changes
Sep 11, 2018
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 |
Contributor
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.
Contributor
Author
There was a problem hiding this comment.
Totally agree we should keep this generic. I'm removing it.
629ee45 to
f49f2b7
Compare
elithrar
approved these changes
Sep 14, 2018
Contributor
|
Thanks @commit-master! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.