diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index 74c1581d1ea9..f5defe3ec5ae 100644 --- a/core/corehttp/gateway_handler.go +++ b/core/corehttp/gateway_handler.go @@ -117,7 +117,15 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request return } - etag := gopath.Base(urlPath) + ndHash, err := nd.Key() + if err != nil { + internalWebError(w, err) + return + } + + // ETag requires the quote marks: + // https://tools.ietf.org/html/rfc7232#section-2.3 + etag := "\"" + ndHash.String() + "\"" if r.Header.Get("If-None-Match") == etag { w.WriteHeader(http.StatusNotModified) return @@ -150,11 +158,10 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request // responses! The webError functions unset them automatically. // TODO: Consider letting clients cache 404s. Are there cases where // that would hurt? - setSuccessHeaders(w, ttl) + setSuccessHeaders(w, ttl, etag) modtime := time.Now() if strings.HasPrefix(urlPath, ipfsPathPrefix) { - w.Header().Set("Etag", etag) // set modtime to a really long time ago, since files are immutable and should stay cached modtime = time.Unix(1, 0) } @@ -457,8 +464,9 @@ func internalWebError(w http.ResponseWriter, err error) { webErrorWithCode(w, "internalWebError", err, http.StatusInternalServerError) } -func setSuccessHeaders(w http.ResponseWriter, ttl time.Duration) { +func setSuccessHeaders(w http.ResponseWriter, ttl time.Duration, etag string) { w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(ttl.Seconds()))) + w.Header().Set("ETag", etag) } func unsetSuccessHeaders(w http.ResponseWriter) { diff --git a/core/corehttp/gateway_test.go b/core/corehttp/gateway_test.go index 41dc837d38aa..1f9bc3a7810f 100644 --- a/core/corehttp/gateway_test.go +++ b/core/corehttp/gateway_test.go @@ -143,14 +143,15 @@ func TestGatewayGet(t *testing.T) { path string status int ttl *time.Duration + etag string text string }{ - {"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, &immutableTTL, "fnord"}, - {"localhost:5001", "/ipns/nxdomain.example.com", http.StatusBadRequest, nil, "Path Resolve error: " + namesys.ErrResolveFailed.Error()}, - {"localhost:5001", "/ipns/example.com", http.StatusOK, &kTTL, "fnord"}, - {"example.com", "/", http.StatusOK, &kTTL, "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, &immutableTTL, k, "fnord"}, + {"localhost:5001", "/ipns/nxdomain.example.com", http.StatusBadRequest, nil, "", "Path Resolve error: " + namesys.ErrResolveFailed.Error()}, + {"localhost:5001", "/ipns/example.com", http.StatusOK, &kTTL, k, "fnord"}, + {"example.com", "/", http.StatusOK, &kTTL, k, "fnord"}, } { var c http.Client r, err := http.NewRequest("GET", ts.URL+test.path, nil) @@ -171,6 +172,7 @@ func TestGatewayGet(t *testing.T) { continue } checkCacheControl(t, urlstr, resp, test.ttl) + checkETag(t, urlstr, resp, test.etag) body, err := ioutil.ReadAll(resp.Body) if err != nil { t.Fatalf("error reading response from %s: %s", urlstr, err) @@ -211,6 +213,10 @@ func TestIPNSHostnameRedirect(t *testing.T) { if err != nil { t.Fatal(err) } + kFoo, err := dagn2.Key() + if err != nil { + t.Fatal(err) + } t.Logf("k: %s\n", k) kTTL := 10 * time.Minute ns["/ipns/example.net"] = mockEntry{ @@ -240,7 +246,9 @@ 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, &kTTL) + checkETag(t, "http://example.net/foo", res, kFoo.String()) } func TestIPNSHostnameBacklinks(t *testing.T) { @@ -277,6 +285,14 @@ func TestIPNSHostnameBacklinks(t *testing.T) { if err != nil { t.Fatal(err) } + kFoo, err := dagn2.Key() + if err != nil { + t.Fatal(err) + } + kBar, err := dagn3.Key() + if err != nil { + t.Fatal(err) + } t.Logf("k: %s\n", k) kTTL := 10 * time.Minute ns["/ipns/example.net"] = mockEntry{ @@ -315,6 +331,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) { } checkCacheControl(t, "http://example.net/foo/", res, &kTTL) + checkETag(t, "http://example.net/foo/", res, kFoo.String()) // make request to directory listing req, err = http.NewRequest("GET", ts.URL, nil) @@ -347,6 +364,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) { } checkCacheControl(t, "http://example.net/", res, &kTTL) + checkETag(t, "http://example.net/", res, k.String()) // make request to directory listing req, err = http.NewRequest("GET", ts.URL+"/foo/bar/", nil) @@ -379,6 +397,7 @@ func TestIPNSHostnameBacklinks(t *testing.T) { } checkCacheControl(t, "http://example.net/foo/bar/", res, &kTTL) + checkETag(t, "http://example.net/foo/bar/", res, kBar.String()) } func checkCacheControl(t *testing.T, urlstr string, resp *http.Response, ttl *time.Duration) { @@ -391,3 +410,14 @@ func checkCacheControl(t *testing.T, urlstr string, resp *http.Response, ttl *ti t.Errorf("unexpected Cache-Control header from %s: expected %q; got %q", urlstr, expCacheControl, cacheControl) } } + +func checkETag(t *testing.T, urlstr string, resp *http.Response, expETagSansQuotes string) { + expETag := "" + if expETagSansQuotes != "" { + expETag = "\"" + expETagSansQuotes + "\"" + } + etag := resp.Header.Get("ETag") + if etag != expETag { + t.Errorf("unexpected ETag header from %s: expected %q; got %q", urlstr, expETag, etag) + } +} diff --git a/test/sharness/lib/test-lib.sh b/test/sharness/lib/test-lib.sh index 09a4ae57d238..d10172883f42 100644 --- a/test/sharness/lib/test-lib.sh +++ b/test/sharness/lib/test-lib.sh @@ -309,6 +309,16 @@ test_should_contain() { fi } +test_should_not_contain() { + test "$#" = 2 || error "bug in the test script: not 2 parameters to test_should_not_contain" + if grep -q "$1" "$2" + then + echo "'$2' should not contain '$1', but it does:" + cat "$2" + return 1 + fi +} + test_str_contains() { find=$1 shift diff --git a/test/sharness/t0110-gateway.sh b/test/sharness/t0110-gateway.sh index 9f54fb064961..bd0e4ed705fc 100755 --- a/test/sharness/t0110-gateway.sh +++ b/test/sharness/t0110-gateway.sh @@ -58,6 +58,7 @@ test_item() { _http_status="$3" _expected_output_file="$4" _max_age="$5" + _etag="$6" # Without the surrounding quote marks "$TEST_COMMAND" "$_item: responds with $_http_status" ' test_run_curl "$_url" && @@ -87,6 +88,21 @@ test_item() { ' ;; esac + + case "$_etag" in + NO_ETAG) + "$TEST_COMMAND" "$_item: has no ETag" ' + test_should_not_contain "^[Ee][Tt][Aa][Gg]: " actual.headers + ' + ;; + *) + # The go http package replaces ETag with Etag. Accept both. (HTTP + # header names are case-insensitive but ETag is the official name.) + "$TEST_COMMAND" "$_item: has ETag" ' + test_should_contain "^E[Tt]ag: \"$_etag\".\$" actual.headers + ' + ;; + esac } IMMUTABLE_TTL=$((10*365*24*60*60)) @@ -111,35 +127,38 @@ test_expect_success "Generating test environment" ' ' test_item "GET IPFS path" "http://127.0.0.1:$port/ipfs/$HASH1" \ - "200 OK" "$DATA1" "$IMMUTABLE_TTL" + "200 OK" "$DATA1" "$IMMUTABLE_TTL" "$HASH1" test_item "GET IPFS path on API" "http://127.0.0.1:$apiport/ipfs/$HASH1" \ - "403 Forbidden" IGNORE_OUTPUT NO_MAX_AGE + "403 Forbidden" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG ITEM="GET IPFS directory path" test_item "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2" \ - "200 OK" IGNORE_OUTPUT "$IMMUTABLE_TTL" + "200 OK" IGNORE_OUTPUT "$IMMUTABLE_TTL" "$HASH2" "$TEST_COMMAND" "$ITEM: output looks good" ' test_should_contain "Index of /ipfs/$HASH2" actual ' test_item "GET IPFS directory file" "http://127.0.0.1:$port/ipfs/$HASH2/test" \ - "200 OK" "$DATA2/test" "$IMMUTABLE_TTL" + "200 OK" "$DATA2/test" "$IMMUTABLE_TTL" \ + "$(ipfs add --only-hash -q "$DATA2/test")" ITEM="GET IPFS directory path with index.html" test_item "$ITEM" "http://127.0.0.1:$port/ipfs/$HASH2/has-index-html" \ - "302 Found" IGNORE_OUTPUT "$IMMUTABLE_TTL" + "302 Found" IGNORE_OUTPUT "$IMMUTABLE_TTL" \ + "$(ipfs add --only-hash -r -q "$DATA2/has-index-html" | tail -n 1)" "$TEST_COMMAND" "$ITEM: redirects to path/" ' test_should_contain "^Location: /ipfs/$HASH2/has-index-html/.\$" actual.headers ' test_item "GET IPFS directory path/ with index.html" \ "http://127.0.0.1:$port/ipfs/$HASH2/has-index-html/" \ - "200 OK" "$DATA2/has-index-html/index.html" "$IMMUTABLE_TTL" + "200 OK" "$DATA2/has-index-html/index.html" "$IMMUTABLE_TTL" \ + "$(ipfs add --only-hash -r -q "$DATA2/has-index-html" | tail -n 1)" test_item "GET IPFS non-existent file" \ "http://127.0.0.1:$port/ipfs/$HASH2/pleaseDontAddMe" \ - "404 Not Found" IGNORE_OUTPUT NO_MAX_AGE + "404 Not Found" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG # https://github.com/ipfs/go-ipfs/issues/1941 sharness suite: ipfs name # publish: “Error: failed to find any peer in table” @@ -152,7 +171,7 @@ ITEM="GET IPNS path" ipfs name publish --ttl="${TTL}s" "$HASH1" ' test_item "$ITEM" "http://127.0.0.1:$port/ipns/$PEERID" \ - "200 OK" "$DATA1" "$TTL" + "200 OK" "$DATA1" "$TTL" "$HASH1" test_timer_start EXPIRY_TIMER "${TTL}s" # The cache entry has expired when this finishes. ITEM="GET IPNS path again before cache expiry" @@ -163,7 +182,7 @@ ITEM="GET IPNS path again before cache expiry" ' go-sleep 1s # Ensure the following max-age is strictly less than TTL. test_item "$ITEM" "http://127.0.0.1:$port/ipns/$PEERID" \ - "200 OK" "$DATA1" IGNORE_MAX_AGE + "200 OK" "$DATA1" IGNORE_MAX_AGE "$HASH1" "$TEST_COMMAND" "$ITEM: has Cache-Control max-age between 0 and $TTL" ' max_age="$(test_get_max_age actual.headers)" && test "$max_age" -gt 0 && @@ -183,15 +202,16 @@ test_expect_success "$ITEM: previous version is no longer cached" ' test_item "GET IPNS path again after cache expiry" \ "http://127.0.0.1:$port/ipns/$PEERID/test" \ - "200 OK" "$DATA2/test" "$UNKNOWN_TTL" + "200 OK" "$DATA2/test" "$UNKNOWN_TTL" \ + "$(ipfs add --only-hash -q "$DATA2/test")" TEST_COMMAND=test_expect_success test_item "GET invalid IPFS path" "http://127.0.0.1:$port/ipfs/12345" \ - "400 Bad Request" IGNORE_OUTPUT NO_MAX_AGE + "400 Bad Request" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG test_item "GET invalid root path" "http://127.0.0.1:$port/12345" \ - "404 Not Found" IGNORE_OUTPUT NO_MAX_AGE + "404 Not Found" IGNORE_OUTPUT NO_MAX_AGE NO_ETAG test_expect_success "GET /webui returns code expected" ' test_curl_resp_http_code "http://127.0.0.1:$apiport/webui" "HTTP/1.1 302 Found" "HTTP/1.1 301 Moved Permanently"