-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix: create options api for cors preflight request. #2598
Conversation
You can find the image built from this PR at
Built from dc496bb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!!! Thank you for this extension!
Should we have similar OPTION method handlers for all the enpoint paths?
@NagyZoltanPeter We can add OPTIONS for other endpoints when people requesting it, not sure if it's a best practice though. Prefer not adding too much boilerplate code. |
765a89a
to
0180095
Compare
@kaichaosun Do you think it worth to make some more generic solution for it? I think of, in case allow-origin is configured we should auto add OPTION to each installed endpoint? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
@kaichaosun - let's make sure all tests pass before merging :)
The only ones that we expect to fail are the js-waku
jobs, which will be fixed soon
@NagyZoltanPeter A generic and auto solution for all endpoints would be great. I will create an issue to track it. |
0180095
to
827a5d7
Compare
Force pushed for verified commit. @Ivansete-status |
Description
CORS request triggers an Options request to same endpoint. More context.
The changes here will fix send REST API request from web browser to local nwaku node.
Changes
Reason: header ‘content-type’ is not allowed according to header ‘Access-Control-Allow-Headers’