Fix missing CORS headers on RPC response. #2029
Conversation
solonovamax
changed the title
|
What's preventing the CI pipeline from running for this PR? |
|
No idea - too old? But I have changes for this anyway so it'll run when I push later. |
Signed-off-by: solonovamax <solonovamax@12oclockpoint.com>
kingosticks
force-pushed
the
fix/fix-rpc-headers
branch
2 times, most recently
from
8a867f1
to
b997247
Compare
|
Why is codecov so useless? I am mostly ignoring it at this point |
Added basic cors tests for JsonRpcHandler and WebSocketHandlerTest. Moved CSRF testing from server into request handler tests. Better comments on how this all works. Fixed allowed_origins test bug which defined a list instead of a set. Fixed options response handler should not return Boolean.
kingosticks
force-pushed
the
fix/fix-rpc-headers
branch
from
b997247
to
8623a45
Compare
|
I was kind of hoping we could get this issue resolved before doing further work on the CORS code: The first step of my plan towards improving that situation was this PR, which has been waiting for review for a while now: It would be great to get that PR moving, as there are many other follow-up changes, some of which I've already made and some of which others have made. For example, this PR also adds a |
|
I'm just working through my long list of little things that personally bugged me. There are more CORS-related issues on the tracker, I'll visit yours next. |
|
Oh I actually didn't mean to squash that one. Oops |
|
And thanks @solonovamax for highlighting the issue and providing the original fix. I tweaked it a bit and added some comments explaining how the POST handler is designed to work w.r.t CORS because it wasn't totally obvious to me (after so long) and I wrote the thing! |
|
Perfect! |
Fixes #2028.
Add CORS headers to RPC response.
This fixes a CORS error on chrome.
See #2028 for more information.