diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index d9864c051465..bec8189e1dcc 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -111,7 +111,7 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request return } - nd, err := core.Resolve(ctx, i.node, path.Path(urlPath)) + res, err := core.ResolveFull(ctx, i.node, path.Path(urlPath)) if err != nil { webError(w, "Path Resolve error", err, http.StatusBadRequest) return @@ -139,22 +139,20 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request w.Header().Set("Suborigin", pathRoot) } - dr, err := uio.NewDagReader(ctx, nd, i.node.DAG) + dr, err := uio.NewDagReader(ctx, res.Node, i.node.DAG) if err != nil && err != uio.ErrIsDir { // not a directory and still an error internalWebError(w, err) return } - // set these headers _after_ the error, for we may just not have it - // and dont want the client to cache a 500 response... - // and only if it's /ipfs! - // TODO: break this out when we split /ipfs /ipns routes. + // Remember to unset the Cache-Control/ETag headers before error + // responses! The webError functions unset them automatically. + setSuccessHeaders(w, res) + modtime := time.Now() if strings.HasPrefix(urlPath, ipfsPathPrefix) { w.Header().Set("Etag", etag) - w.Header().Set("Cache-Control", "public, max-age=29030400") - // set modtime to a really long time ago, since files are immutable and should stay cached modtime = time.Unix(1, 0) } @@ -170,7 +168,7 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request var dirListing []directoryItem // loop through files foundIndex := false - for _, link := range nd.Links { + for _, link := range res.Node.Links { if link.Name == "index.html" { log.Debugf("found index.html link for %s", urlPath) foundIndex = true @@ -446,6 +444,7 @@ func webError(w http.ResponseWriter, message string, err error, defaultCode int) } func webErrorWithCode(w http.ResponseWriter, message string, err error, code int) { + unsetSuccessHeaders(w) w.WriteHeader(code) log.Errorf("%s: %s", message, err) // TODO(cryptix): log errors until we have a better way to expose these (counter metrics maybe) fmt.Fprintf(w, "%s: %s", message, err) @@ -455,3 +454,12 @@ func webErrorWithCode(w http.ResponseWriter, message string, err error, code int func internalWebError(w http.ResponseWriter, err error) { webErrorWithCode(w, "internalWebError", err, http.StatusInternalServerError) } + +func setSuccessHeaders(w http.ResponseWriter, res core.ResolveResult) { + w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(res.TTL.Seconds()))) +} + +func unsetSuccessHeaders(w http.ResponseWriter) { + w.Header().Del("Cache-Control") + w.Header().Del("ETag") +} diff --git a/core/corehttp/gateway_test.go b/core/corehttp/gateway_test.go index c6448c22f75b..b987cb30155c 100644 --- a/core/corehttp/gateway_test.go +++ b/core/corehttp/gateway_test.go @@ -2,6 +2,7 @@ package corehttp import ( "errors" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -20,7 +21,12 @@ import ( testutil "github.com/ipfs/go-ipfs/util/testutil" ) -type mockNamesys map[string]path.Path +type mockNamesys map[string]mockEntry + +type mockEntry struct { + path path.Path + ttl time.Duration +} func (m mockNamesys) Resolve(ctx context.Context, name string) (path.Path, error) { res, err := m.ResolveFull(ctx, name) @@ -37,11 +43,11 @@ func (m mockNamesys) ResolveFull(ctx context.Context, name string) (namesys.Reso } func (m mockNamesys) ResolveNFull(ctx context.Context, name string, depth int) (namesys.ResolveResult, error) { - p, ok := m[name] + entry, ok := m[name] if !ok { return namesys.ResolveResult{}, namesys.ErrResolveFailed } - return namesys.ResolveResult{p, 0}, nil + return namesys.ResolveResult{entry.path, entry.ttl}, nil } func (m mockNamesys) Publish(ctx context.Context, name ci.PrivKey, value path.Path) error { @@ -124,21 +130,26 @@ func TestGatewayGet(t *testing.T) { if err != nil { t.Fatal(err) } - ns["/ipns/example.com"] = path.FromString("/ipfs/" + k) + kTTL := 10 * time.Minute + ns["/ipns/example.com"] = mockEntry{ + path: path.FromString("/ipfs/" + k), + ttl: kTTL, + } t.Log(ts.URL) for _, test := range []struct { host string path string status int + ttl []time.Duration // an approximation for a Maybe/Option sum type text string }{ - {"localhost:5001", "/", http.StatusNotFound, "404 page not found\n"}, - {"localhost:5001", "/" + k, http.StatusNotFound, "404 page not found\n"}, - {"localhost:5001", "/ipfs/" + k, http.StatusOK, "fnord"}, - {"localhost:5001", "/ipns/nxdomain.example.com", http.StatusBadRequest, "Path Resolve error: " + namesys.ErrResolveFailed.Error()}, - {"localhost:5001", "/ipns/example.com", http.StatusOK, "fnord"}, - {"example.com", "/", http.StatusOK, "fnord"}, + {"localhost:5001", "/", http.StatusNotFound, nil, "404 page not found\n"}, + {"localhost:5001", "/" + k, http.StatusNotFound, nil, "404 page not found\n"}, + {"localhost:5001", "/ipfs/" + k, http.StatusOK, []time.Duration{namesys.ImmutableTTL}, "fnord"}, + {"localhost:5001", "/ipns/nxdomain.example.com", http.StatusBadRequest, nil, "Path Resolve error: " + namesys.ErrResolveFailed.Error()}, + {"localhost:5001", "/ipns/example.com", http.StatusOK, []time.Duration{kTTL}, "fnord"}, + {"example.com", "/", http.StatusOK, []time.Duration{kTTL}, "fnord"}, } { var c http.Client r, err := http.NewRequest("GET", ts.URL+test.path, nil) @@ -158,6 +169,7 @@ func TestGatewayGet(t *testing.T) { t.Errorf("got %d, expected %d from %s", resp.StatusCode, test.status, urlstr) continue } + checkCacheControl(t, urlstr, resp, test.ttl) body, err := ioutil.ReadAll(resp.Body) if err != nil { t.Fatalf("error reading response from %s: %s", urlstr, err) @@ -199,7 +211,11 @@ func TestIPNSHostnameRedirect(t *testing.T) { t.Fatal(err) } t.Logf("k: %s\n", k) - ns["/ipns/example.net"] = path.FromString("/ipfs/" + k.String()) + kTTL := 10 * time.Minute + ns["/ipns/example.net"] = mockEntry{ + path: path.FromString("/ipfs/" + k.String()), + ttl: kTTL, + } // make request to directory containing index.html req, err := http.NewRequest("GET", ts.URL+"/foo", nil) @@ -223,6 +239,7 @@ func TestIPNSHostnameRedirect(t *testing.T) { } else if hdr[0] != "/foo/" { t.Errorf("location header is %v, expected /foo/", hdr[0]) } + checkCacheControl(t, "http://example.net/foo", res, []time.Duration{kTTL}) } func TestIPNSHostnameBacklinks(t *testing.T) { @@ -260,7 +277,11 @@ func TestIPNSHostnameBacklinks(t *testing.T) { t.Fatal(err) } t.Logf("k: %s\n", k) - ns["/ipns/example.net"] = path.FromString("/ipfs/" + k.String()) + kTTL := 10 * time.Minute + ns["/ipns/example.net"] = mockEntry{ + path: path.FromString("/ipfs/" + k.String()), + ttl: kTTL, + } // make request to directory listing req, err := http.NewRequest("GET", ts.URL+"/foo/", nil) @@ -292,6 +313,8 @@ func TestIPNSHostnameBacklinks(t *testing.T) { t.Fatalf("expected file in directory listing") } + checkCacheControl(t, "http://example.net/foo/", res, []time.Duration{kTTL}) + // make request to directory listing req, err = http.NewRequest("GET", ts.URL, nil) if err != nil { @@ -322,6 +345,8 @@ func TestIPNSHostnameBacklinks(t *testing.T) { t.Fatalf("expected file in directory listing") } + checkCacheControl(t, "http://example.net/", res, []time.Duration{kTTL}) + // make request to directory listing req, err = http.NewRequest("GET", ts.URL+"/foo/bar/", nil) if err != nil { @@ -351,4 +376,17 @@ func TestIPNSHostnameBacklinks(t *testing.T) { if !strings.Contains(s, "") { t.Fatalf("expected file in directory listing") } + + checkCacheControl(t, "http://example.net/foo/bar/", res, []time.Duration{kTTL}) +} + +func checkCacheControl(t *testing.T, urlstr string, resp *http.Response, ttlMaybe []time.Duration) { + expCacheControl := "" + for _, ttl := range ttlMaybe { + expCacheControl = fmt.Sprintf("public, max-age=%d", int(ttl.Seconds())) + } + cacheControl := resp.Header.Get("Cache-Control") + if cacheControl != expCacheControl { + t.Errorf("unexpected Cache-Control header from %s: expected %q; got %q", urlstr, expCacheControl, cacheControl) + } } diff --git a/test/sharness/t0110-gateway.sh b/test/sharness/t0110-gateway.sh index c15832088484..765e99f2295f 100755 --- a/test/sharness/t0110-gateway.sh +++ b/test/sharness/t0110-gateway.sh @@ -22,58 +22,257 @@ apiport=$PORT_API # define a function to test a gateway, and do so for each port. # for now we check 5001 here as 5002 will be checked in gateway-writable. -test_expect_success "GET IPFS path succeeds" ' - echo "Hello Worlds!" >expected && - HASH=$(ipfs add -q expected) && - curl -sfo actual "http://127.0.0.1:$port/ipfs/$HASH" +test_curl_headers() { + _headers="$1"; shift + _output="$1"; shift + curl -s -D "$_headers" -o "$_output" "$@" +} + +test_get_max_age() { + perl -nwe 's/^Cache-Control:.* max-age=([0-9]+).*$/$1/ && print' "$1" +} + +# Overwrites actual, expected. +test_expect_max_age() { + test_get_max_age "$1" >actual + printf "%s\n" "$2" >expected + test_cmp expected actual || test_fsh cat actual.headers +} + +# Overwrites actual, expected. +test_expect_no_max_age() { + test_get_max_age "$1" >actual + : >expected + test_cmp expected actual || test_fsh cat actual.headers +} + +immutable_ttl=$((10*365*24*60*60)) +unknown_ttl=60 + +# DATA1, HASH1: a file +# DATA2, HASH2: a directory +# PEERID: the node ID +test_expect_success "Generating test environment" ' + DATA1=data1 && + echo "Hello Worlds!" >"$DATA1" && + HASH1=$(ipfs add -q "$DATA1") && + DATA2=data2 && + mkdir "$DATA2" && + echo "12345" >"$DATA2"/test && + mkdir "$DATA2"/has-index-html && + echo "index" >"$DATA2"/has-index-html/index.html && + ipfs add -r -q "$DATA2" >actual && + HASH2=$(tail -n 1 actual) && + PEERID=$(ipfs config Identity.PeerID) && + test_check_peerid "$PEERID" ' -test_expect_success "GET IPFS path output looks good" ' - test_cmp expected actual && - rm actual +ITEM="GET IPFS path" + +test_expect_success "$ITEM: succeeds" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipfs/$HASH1" && + test_should_contain "^HTTP/1.1 200 OK.\$" actual.headers ' -test_expect_success "GET IPFS path on API forbidden" ' - test_curl_resp_http_code "http://127.0.0.1:$apiport/ipfs/$HASH" "HTTP/1.1 403 Forbidden" +test_expect_success "$ITEM: output looks good" ' + test_cmp "$DATA1" actual ' -test_expect_success "GET IPFS directory path succeeds" ' - mkdir dir && - echo "12345" >dir/test && - ipfs add -r -q dir >actual && - HASH2=$(tail -n 1 actual) && - curl -sf "http://127.0.0.1:$port/ipfs/$HASH2" +test_expect_success "$ITEM: has long Cache-Control max-age" ' + test_expect_max_age actual.headers "$immutable_ttl" +' + +ITEM="GET IPFS path on API" + +test_expect_success "$ITEM: forbidden (403)" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$apiport/ipfs/$HASH1" && + test_should_contain "^HTTP/1.1 403 Forbidden.\$" actual.headers ' -test_expect_success "GET IPFS directory file succeeds" ' - curl -sfo actual "http://127.0.0.1:$port/ipfs/$HASH2/test" +test_expect_success "$ITEM: has no Cache-Control max-age" ' + test_expect_no_max_age actual.headers ' -test_expect_success "GET IPFS directory file output looks good" ' - test_cmp dir/test actual +ITEM="GET IPFS directory path" + +test_expect_success "$ITEM: succeeds" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipfs/$HASH2" && + test_should_contain "^HTTP/1.1 200 OK.\$" actual.headers ' -test_expect_success "GET IPFS non existent file returns code expected (404)" ' - test_curl_resp_http_code "http://127.0.0.1:$port/ipfs/$HASH2/pleaseDontAddMe" "HTTP/1.1 404 Not Found" +test_expect_success "$ITEM: output looks good" ' + test_should_contain "Index of /ipfs/$HASH2" actual ' -test_expect_failure "GET IPNS path succeeds" ' - ipfs name publish "$HASH" && - PEERID=$(ipfs config Identity.PeerID) && - test_check_peerid "$PEERID" && - curl -sfo actual "http://127.0.0.1:$port/ipns/$PEERID" +test_expect_success "$ITEM: has long Cache-Control max-age" ' + test_expect_max_age actual.headers "$immutable_ttl" ' -test_expect_failure "GET IPNS path output looks good" ' - test_cmp expected actual +ITEM="GET IPFS directory file" + +test_expect_success "$ITEM: succeeds" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipfs/$HASH2/test" && + test_should_contain "^HTTP/1.1 200 OK.\$" actual.headers ' -test_expect_success "GET invalid IPFS path errors" ' - test_must_fail curl -sf "http://127.0.0.1:$port/ipfs/12345" +test_expect_success "$ITEM: output looks good" ' + test_cmp "$DATA2"/test actual +' + +test_expect_success "$ITEM: has long Cache-Control max-age" ' + test_expect_max_age actual.headers "$immutable_ttl" +' + +ITEM="GET IPFS directory path with index.html" + +test_expect_success "$ITEM: succeeds" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipfs/$HASH2/has-index-html" && + test_should_contain "^HTTP/1.1 302 Found.\$" actual.headers +' + +test_expect_success "$ITEM: redirects to path/" ' + test_should_contain "^Location: /ipfs/$HASH2/has-index-html/.\$" actual.headers +' + +test_expect_success "$ITEM: has long Cache-Control max-age" ' + test_expect_max_age actual.headers "$immutable_ttl" +' + +ITEM="GET IPFS directory path/ with index.html" + +test_expect_success "$ITEM: succeeds" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipfs/$HASH2/has-index-html/" && + test_should_contain "^HTTP/1.1 200 OK.\$" actual.headers +' + +test_expect_success "$ITEM: output looks good" ' + test_cmp "$DATA2"/has-index-html/index.html actual +' + +test_expect_success "$ITEM: has long Cache-Control max-age" ' + test_expect_max_age actual.headers "$immutable_ttl" +' + +ITEM="GET IPFS non-existent file" + +test_expect_success "$ITEM: not found (404)" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipfs/$HASH2/pleaseDontAddMe" && + test_should_contain "^HTTP/1.1 404 Not Found.\$" actual.headers +' + +# TODO Consider letting clients cache 404s. Are there cases where that would +# hurt? +test_expect_success "$ITEM: has no Cache-Control max-age" ' + test_expect_no_max_age actual.headers +' + +ITEM="GET IPNS path" + +TTL=10 + +# https://github.com/ipfs/go-ipfs/issues/1941 sharness suite: ipfs name +# publish: “Error: failed to find any peer in table” +test_expect_failure "$ITEM: IPNS publish with TTL $TTL succeeds" ' + ipfs name publish --ttl="${TTL}s" "$HASH1" +' + +test_expect_failure "$ITEM: succeeds" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipns/$PEERID" && + test_should_contain "^HTTP/1.1 200 OK.\$" actual.headers +' + +# The cache entry has expired when this finishes. +go-sleep "${TTL}s" & EXPIRY_PID="$!" + +test_expect_failure "$ITEM: output looks good" ' + test_cmp "$DATA1" actual +' + +test_expect_failure "$ITEM: has Cache-Control max-age=$TTL" ' + test_expect_max_age actual.headers "$TTL" +' + +ITEM="GET IPNS path again before cache expiry" + +# Publish a new version now, the previous version should still be cached. The +# next item will resolve this version. +test_expect_failure "$ITEM: IPNS publish with default TTL succeeds" ' + ipfs name publish "$HASH2" +' + +test_expect_failure "$ITEM: succeeds" ' + go-sleep 1s && + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipns/$PEERID" && + test_should_contain "^HTTP/1.1 200 OK.\$" actual.headers +' + +# Make sure the expiry timer is still running, otherwise the result might be +# wrong. If this fails, we will need to increase TTL above to give enough time +# for the tests. +test_expect_success "$ITEM: tests did not take too long" ' + kill -0 "$EXPIRY_PID" +' + +test_expect_failure "$ITEM: output looks good (previous resolve was cached)" ' + test_cmp "$DATA1" actual +' + +test_expect_failure "$ITEM: has Cache-Control max-age between 0 and $TTL" ' + max_age="$(test_get_max_age actual.headers)" && + test "$max_age" -gt 0 && + test "$max_age" -lt "$TTL" || + test_fsh cat actual.headers +' + +ITEM="GET IPNS path again after cache expiry" + +# Wait for a while, GET should return the new result now. +test_expect_failure "$ITEM: succeeds" ' + wait "$EXPIRY_PID" && + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipns/$PEERID/test" && + test_should_contain "^HTTP/1.1 200 OK.\$" actual.headers +' + +test_expect_failure "$ITEM: output looks good (new result)" ' + test_cmp "$DATA2"/test actual +' + +test_expect_failure "$ITEM: has default Cache-Control max-age" ' + test_expect_max_age actual.headers "$unknown_ttl" +' + +ITEM="GET invalid IPFS path" + +test_expect_success "$ITEM: bad request (400)" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/ipfs/12345" && + test_should_contain "^HTTP/1.1 400 Bad Request.\$" actual.headers +' + +test_expect_success "$ITEM: has no Cache-Control max-age" ' + test_expect_no_max_age actual.headers +' + +ITEM="GET invalid root path" + +test_expect_success "$ITEM: not found (404)" ' + rm -f actual.headers actual && + test_curl_headers actual.headers actual "http://127.0.0.1:$port/12345" && + test_should_contain "^HTTP/1.1 404 Not Found.\$" actual.headers ' -test_expect_success "GET invalid path errors" ' - test_must_fail curl -sf "http://127.0.0.1:$port/12345" +test_expect_success "$ITEM: has no Cache-Control max-age" ' + test_expect_no_max_age actual.headers ' test_expect_success "GET /webui returns code expected" ' @@ -103,7 +302,7 @@ test_expect_success "get IPFS directory file through readonly API succeeds" ' ' test_expect_success "get IPFS directory file through readonly API output looks good" ' - test_cmp dir/test actual + test_cmp "$DATA2"/test actual ' test_expect_success "refs IPFS directory file through readonly API succeeds" '