Skip to content
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

gateway: fix CORs headers #5893

Merged
merged 4 commits into from
Jan 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions core/corehttp/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,18 @@ func addHeadersFromConfig(c *cmdsHttp.ServerConfig, nc *config.Config) {
}
}

c.Headers = make(map[string][]string, len(nc.API.HTTPHeaders))
c.Headers = make(map[string][]string, len(nc.API.HTTPHeaders)+1)

// Copy these because the config is shared and this function is called
// in multiple places concurrently. Updating these in-place *is* racy.
for h, v := range nc.API.HTTPHeaders {
c.Headers[h] = v
h = http.CanonicalHeaderKey(h)
switch h {
case cmdsHttp.ACAOrigin, cmdsHttp.ACAMethods, cmdsHttp.ACACredentials:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already skip these in the commands lib so this probably isn't necessary.

// these are handled by the CORs library.
default:
c.Headers[h] = v
}
}
c.Headers["Server"] = []string{"go-ipfs/" + version.CurrentVersionNumber}
}
Expand Down
58 changes: 57 additions & 1 deletion core/corehttp/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"net"
"net/http"
"sort"

version "github.com/ipfs/go-ipfs"
core "github.com/ipfs/go-ipfs/core"
Expand All @@ -18,6 +19,26 @@ type GatewayConfig struct {
PathPrefixes []string
}

// A helper function to clean up a set of headers:
// 1. Canonicalizes.
// 2. Deduplicates.
// 3. Sorts.
func cleanHeaderSet(headers []string) []string {
// Deduplicate and canonicalize.
m := make(map[string]struct{}, len(headers))
for _, h := range headers {
m[http.CanonicalHeaderKey(h)] = struct{}{}
}
result := make([]string, 0, len(m))
for k := range m {
result = append(result, k)
}

// Sort
sort.Strings(result)
return result
}

func GatewayOption(writable bool, paths ...string) ServeOption {
return func(n *core.IpfsNode, _ net.Listener, mux *http.ServeMux) (*http.ServeMux, error) {
cfg, err := n.Repo.Config()
Expand All @@ -30,8 +51,43 @@ func GatewayOption(writable bool, paths ...string) ServeOption {
return nil, err
}

headers := make(map[string][]string, len(cfg.Gateway.HTTPHeaders))
for h, v := range cfg.Gateway.HTTPHeaders {
headers[http.CanonicalHeaderKey(h)] = v
}

// Hard-coded headers.
const ACAHeadersName = "Access-Control-Allow-Headers"
const ACEHeadersName = "Access-Control-Expose-Headers"
const ACAOriginName = "Access-Control-Allow-Origin"
const ACAMethodsName = "Access-Control-Allow-Methods"

if _, ok := headers[ACAOriginName]; !ok {
// Default to *all*
headers[ACAOriginName] = []string{"*"}
}
if _, ok := headers[ACAMethodsName]; !ok {
// Default to GET
headers[ACAMethodsName] = []string{"GET"}
}

headers[ACAHeadersName] = cleanHeaderSet(
append([]string{
"Content-Type",
"User-Agent",
"Range",
"X-Requested-With",
}, headers[ACAHeadersName]...))

headers[ACEHeadersName] = cleanHeaderSet(
append([]string{
"Content-Range",
"X-Chunked-Output",
"X-Stream-Output",
}, headers[ACEHeadersName]...))

gateway := newGatewayHandler(n, GatewayConfig{
Headers: cfg.Gateway.HTTPHeaders,
Headers: headers,
Writable: writable,
PathPrefixes: cfg.Gateway.PathPrefixes,
}, api)
Expand Down
13 changes: 0 additions & 13 deletions core/corehttp/gateway_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,19 +184,6 @@ func (i *gatewayHandler) getOrHeadHandler(ctx context.Context, w http.ResponseWr
w.Header().Set("X-IPFS-Path", urlPath)
w.Header().Set("Etag", etag)

// set 'allowed' headers
// & expose those headers
var allowedHeadersArr = []string{
"Content-Range",
"X-Chunked-Output",
"X-Stream-Output",
}

var allowedHeaders = strings.Join(allowedHeadersArr, ", ")

w.Header().Set("Access-Control-Allow-Headers", allowedHeaders)
w.Header().Set("Access-Control-Expose-Headers", allowedHeaders)

// Suborigin header, sandboxes apps from each other in the browser (even
// though they are served from the same gateway domain).
//
Expand Down
55 changes: 42 additions & 13 deletions test/sharness/t0112-gateway-cors.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,70 @@ test_init_ipfs
test_config_ipfs_cors_headers
test_launch_ipfs_daemon

gwport=$GWAY_PORT
apiport=$API_PORT
thash='QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn'

# Gateway

# HTTP GET Request
test_expect_success "GET to Gateway succeeds" '
curl -svX GET "http://127.0.0.1:$gwport/ipfs/$thash" 2>curl_output
curl -svX GET "http://127.0.0.1:$GWAY_PORT/ipfs/$thash" >/dev/null 2>curl_output &&
cat curl_output
'

cat curl_output
# GET Response from Gateway should contain CORS headers
test_expect_success "GET response for Gateway resource looks good" '
grep "Access-Control-Allow-Origin:" curl_output | grep "\*" &&
grep "Access-Control-Allow-Methods:" curl_output | grep " GET\b" &&
grep "Access-Control-Allow-Headers:" curl_output
grep "< Access-Control-Allow-Origin: \*" curl_output &&
grep "< Access-Control-Allow-Methods: GET" curl_output &&
grep "< Access-Control-Allow-Headers: Range" curl_output &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access-Control-Allow-Headers is relevant only during OPTIONS request, should not be returned with GET response:

Suggested change
grep "< Access-Control-Allow-Headers: Range" curl_output &&
grep -vL "< Access-Control-Allow-Headers" curl_output &&

Rationale: ipfs/in-web-browsers#132 (comment)

grep "< Access-Control-Expose-Headers: Content-Range" curl_output
'

# HTTP OPTIONS Request
test_expect_success "OPTIONS to Gateway succeeds" '
curl -svX OPTIONS "http://127.0.0.1:$gwport/ipfs/$thash" 2>curl_output
curl -svX OPTIONS "http://127.0.0.1:$GWAY_PORT/ipfs/$thash" 2>curl_output &&
cat curl_output
'

# OPTION Response from Gateway should contain CORS headers
test_expect_success "OPTIONS response for Gateway resource looks good" '
grep "Access-Control-Allow-Origin:" curl_output | grep "\*" &&
grep "Access-Control-Allow-Methods:" curl_output | grep " GET\b" &&
grep "Access-Control-Allow-Headers:" curl_output
grep "< Access-Control-Allow-Origin: \*" curl_output &&
grep "< Access-Control-Allow-Methods: GET" curl_output &&
grep "< Access-Control-Allow-Headers: Range" curl_output &&
grep "< Access-Control-Expose-Headers: Content-Range" curl_output
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access-Control-Expose-Headers is relevant only during non-OPTIONS request (GET, PUT etc), and should not be returned with OPTIONS response:

Suggested change
grep "< Access-Control-Expose-Headers: Content-Range" curl_output
grep -vL "< Access-Control-Expose-Headers" curl_output

Rationale: ipfs/in-web-browsers#132 (comment)

'

test_kill_ipfs_daemon

# Change headers
test_expect_success "Can configure gateway headers" '
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Headers "[\"X-Custom1\"]" &&
ipfs config --json Gateway.HTTPHeaders.Access-Control-Expose-Headers "[\"X-Custom2\"]" &&
ipfs config --json Gateway.HTTPHeaders.Access-Control-Allow-Origin "[\"localhost\"]"
'

test_launch_ipfs_daemon

test_expect_success "OPTIONS to Gateway succeeds" '
curl -svX OPTIONS "http://127.0.0.1:$GWAY_PORT/ipfs/$thash" 2>curl_output &&
cat curl_output
'

test_expect_success "Access-Control-Allow-Headers extends" '
grep "< Access-Control-Allow-Headers: Range" curl_output &&
grep "< Access-Control-Allow-Headers: X-Custom1" curl_output &&
grep "< Access-Control-Expose-Headers: Content-Range" curl_output &&
grep "< Access-Control-Expose-Headers: X-Custom2" curl_output
'

test_expect_success "Access-Control-Allow-Origin replaces" '
grep "< Access-Control-Allow-Origin: localhost" curl_output
'

# Read-Only API (at the Gateway Port)

# HTTP GET Request
test_expect_success "GET to API succeeds" '
curl -svX GET "http://127.0.0.1:$gwport/api/v0/cat?arg=$thash" 2>curl_output
curl -svX GET "http://127.0.0.1:$GWAY_PORT/api/v0/cat?arg=$thash" >/dev/null 2>curl_output
'
# GET Response from the API should NOT contain CORS headers
# Blacklisting: https://git.io/vzaj2
Expand All @@ -63,7 +92,7 @@ test_expect_success "OPTIONS response for API looks good" '

# HTTP OPTIONS Request
test_expect_success "OPTIONS to API succeeds" '
curl -svX OPTIONS "http://127.0.0.1:$gwport/api/v0/cat?arg=$thash" 2>curl_output
curl -svX OPTIONS "http://127.0.0.1:$GWAY_PORT/api/v0/cat?arg=$thash" 2>curl_output
'
# OPTIONS Response from the API should NOT contain CORS headers
test_expect_success "OPTIONS response for API looks good" '
Expand Down