Gateway: add support for HTTP OPTIONS request type#2232
Gateway: add support for HTTP OPTIONS request type#2232jbenet merged 2 commits intoipfs:masterfrom lidel:cors-preflight-fix
Conversation
OPTIONS is a noop request that is used by the browsers to check if server will accept cross-site XMLHttpRequest (indicated by the presence of CORS headers) Before this fix user could enable CORS headers in the Gateway config, but XHR failed due to the lack of support for OPTIONS request type (as described in https://git.io/vzgGe) License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
|
Thanks! -- could you add a test which makes sure this works for both the gateway and the readonly api at |
|
@lgierth where should I add OPTION tests? BTW: while OPTIONS itself works with API, CORS headers such as
In short: cross-site XHR currently does not work for API resources. |
There was a problem hiding this comment.
Is this enabled in both ro and writable case? Shouldn't it be enabled for ro only, ref: #934 (comment).
There was a problem hiding this comment.
I feel we should always produce OPTIONS response.
Gateway owner can decide what types of requests are accepted by customizing Access-Control-Allow-Method header (GET is ro, POST, PUT, DELETE are rw).
All major browsers respect this header and refuse to do XHR requests if method is not whitelisted.
|
@lidel I think the test is to be put in test/sharness/t0110-gateway.sh. To speed-up this PR, here is a quick test template: test_gateway_options_request() {
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Origin '["*"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Methods '["PUT", "GET", "POST"]'
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Headers '["X-Requested-With"]'
test_expect_success "OPTIONS to gateway succeeds" '
curl -svX OPTIONS "http://localhost:$port/ipfs/" 2>curl_output
'
test_expect_success "OPTIONS to gateway output looks good" '
grep "Origin:" curl_output &&
grep "Access-Control-Request-Method:" curl_output &&
grep "Access-Control-Request-Headers:" curl_output
'
} |
|
I figured that the cors wrapper in https://github.com/ipfs/go-ipfs/blob/a2b0287a5f6fb5c0f35b63688ad5ac81c451afce/commands/http/handler.go#L102 was meant to handle preflight (see https://github.com/rs/cors/blob/45d446e551b0020074e771dee17d2bb2fd2e9b44/cors.go#L245). Somehow it doesn't get enabled. |
|
|
Just updated the test. It should be runnable. |
- Implements #2232 (comment) - Separate test suite: - we don't want to pollute other gateway tests with CORS headers - (as of now) changing headers requires daemon restart anyway License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
|
Thank you @rht, I decided to add CORS headers and OPTIONS tests as a separate file in 15d717c
I feel that the goal of this PR is to add missing support for Let me know if I should add anything else for this to be merged. |
|
There could be test for gateway ACL as well. |
|
LGTM! |
|
thanks @lidel |
Gateway: add support for HTTP OPTIONS request type
This potentially closes #934
OPTIONSis a noop request that is used by the browsers to check if server explicitly accepts cross-siteXMLHttpRequest(XHR). The acceptance is indicated by the presence of CORS headers.Before this fix user could enable CORS headers in the Gateway config (note that this disabled by default):
..but requests sent from JavaScript failed due to the lack of support for
OPTIONSrequest type.I described details of the problem with XHR in ipfs/ipfs-companion#45 (comment).
With this fix sending cross-site XHR to the local Gateway works as expected.
Reference: