From ba27679629716feba4733fd4f06191e08afb904c Mon Sep 17 00:00:00 2001 From: Johan Kiviniemi Date: Sat, 31 Oct 2015 08:08:47 +0200 Subject: [PATCH] Gateway: Add ETag to IPNS requests Closes ipfs/go-ipfs#1818. License: MIT Signed-off-by: Johan Kiviniemi --- core/corehttp/gateway_handler.go | 16 ++++++--- core/corehttp/gateway_test.go | 42 +++++++++++++++++++---- test/sharness/lib/test-lib.sh | 10 ++++++ test/sharness/t0110-gateway.sh | 58 ++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/core/corehttp/gateway_handler.go b/core/corehttp/gateway_handler.go index 1c0929d1aacb..e3ac2d70b0c1 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 := res.Node.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 @@ -148,11 +156,10 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request // Remember to unset the Cache-Control/ETag headers before error // responses! The webError functions unset them automatically. - setSuccessHeaders(w, res) + setSuccessHeaders(w, res, 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) } @@ -455,8 +462,9 @@ func internalWebError(w http.ResponseWriter, err error) { webErrorWithCode(w, "internalWebError", err, http.StatusInternalServerError) } -func setSuccessHeaders(w http.ResponseWriter, res core.ResolveResult) { +func setSuccessHeaders(w http.ResponseWriter, res core.ResolveResult, etag string) { w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(res.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 10cbf22cd87c..87f0b5b9fbce 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 b9a25ae878f6..1bd4ee815f8f 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 765e99f2295f..8f77ebc756cf 100755 --- a/test/sharness/t0110-gateway.sh +++ b/test/sharness/t0110-gateway.sh @@ -83,6 +83,12 @@ test_expect_success "$ITEM: has long Cache-Control max-age" ' test_expect_max_age actual.headers "$immutable_ttl" ' +# The go http package replaces ETag with Etag. Accept both. (HTTP header +# names are case-insensitive but ETag is the official name.) +test_expect_success "$ITEM: has ETag" ' + test_should_contain "^E[Tt]ag: \"$HASH1\".\$" actual.headers +' + ITEM="GET IPFS path on API" test_expect_success "$ITEM: forbidden (403)" ' @@ -95,6 +101,10 @@ test_expect_success "$ITEM: has no Cache-Control max-age" ' test_expect_no_max_age actual.headers ' +test_expect_success "$ITEM: has no ETag" ' + test_should_not_contain "^ETag: " actual.headers +' + ITEM="GET IPFS directory path" test_expect_success "$ITEM: succeeds" ' @@ -111,6 +121,10 @@ test_expect_success "$ITEM: has long Cache-Control max-age" ' test_expect_max_age actual.headers "$immutable_ttl" ' +test_expect_success "$ITEM: has ETag" ' + test_should_contain "^E[Tt]ag: \"$HASH2\".\$" actual.headers +' + ITEM="GET IPFS directory file" test_expect_success "$ITEM: succeeds" ' @@ -127,6 +141,11 @@ test_expect_success "$ITEM: has long Cache-Control max-age" ' test_expect_max_age actual.headers "$immutable_ttl" ' +test_expect_success "$ITEM: has ETag" ' + hash=$(ipfs add -q "$DATA2"/test) && + test_should_contain "^E[Tt]ag: \"$hash\".\$" actual.headers +' + ITEM="GET IPFS directory path with index.html" test_expect_success "$ITEM: succeeds" ' @@ -143,6 +162,13 @@ test_expect_success "$ITEM: has long Cache-Control max-age" ' test_expect_max_age actual.headers "$immutable_ttl" ' +# The ETag is based on the hash of the path, the index.html magic does not +# affect it. +test_expect_success "$ITEM: has ETag" ' + hash=$(ipfs add -r -q "$DATA2"/has-index-html | tail -n 1) && + test_should_contain "^E[Tt]ag: \"$hash\".\$" actual.headers +' + ITEM="GET IPFS directory path/ with index.html" test_expect_success "$ITEM: succeeds" ' @@ -159,6 +185,13 @@ test_expect_success "$ITEM: has long Cache-Control max-age" ' test_expect_max_age actual.headers "$immutable_ttl" ' +# The ETag is based on the hash of the path, the index.html magic does not +# affect it. +test_expect_success "$ITEM: has ETag" ' + hash=$(ipfs add -r -q "$DATA2"/has-index-html | tail -n 1) && + test_should_contain "^E[Tt]ag: \"$hash\".\$" actual.headers +' + ITEM="GET IPFS non-existent file" test_expect_success "$ITEM: not found (404)" ' @@ -173,6 +206,10 @@ test_expect_success "$ITEM: has no Cache-Control max-age" ' test_expect_no_max_age actual.headers ' +test_expect_success "$ITEM: has no ETag" ' + test_should_not_contain "^ETag: " actual.headers +' + ITEM="GET IPNS path" TTL=10 @@ -200,6 +237,10 @@ test_expect_failure "$ITEM: has Cache-Control max-age=$TTL" ' test_expect_max_age actual.headers "$TTL" ' +test_expect_failure "$ITEM: has ETag" ' + test_should_contain "^E[Tt]ag: \"$HASH1\".\$" actual.headers +' + ITEM="GET IPNS path again before cache expiry" # Publish a new version now, the previous version should still be cached. The @@ -233,6 +274,10 @@ test_expect_failure "$ITEM: has Cache-Control max-age between 0 and $TTL" ' test_fsh cat actual.headers ' +test_expect_failure "$ITEM: has ETag" ' + test_should_contain "^E[Tt]ag: \"$HASH1\".\$" actual.headers +' + ITEM="GET IPNS path again after cache expiry" # Wait for a while, GET should return the new result now. @@ -251,6 +296,11 @@ test_expect_failure "$ITEM: has default Cache-Control max-age" ' test_expect_max_age actual.headers "$unknown_ttl" ' +test_expect_failure "$ITEM: has ETag" ' + hash=$(ipfs add -q "$DATA2"/test) && + test_should_contain "^E[Tt]ag: \"$hash\".\$" actual.headers +' + ITEM="GET invalid IPFS path" test_expect_success "$ITEM: bad request (400)" ' @@ -263,6 +313,10 @@ test_expect_success "$ITEM: has no Cache-Control max-age" ' test_expect_no_max_age actual.headers ' +test_expect_success "$ITEM: has no ETag" ' + test_should_not_contain "^ETag: " actual.headers +' + ITEM="GET invalid root path" test_expect_success "$ITEM: not found (404)" ' @@ -275,6 +329,10 @@ test_expect_success "$ITEM: has no Cache-Control max-age" ' test_expect_no_max_age actual.headers ' +test_expect_success "$ITEM: has no ETag" ' + test_should_not_contain "^ETag: " actual.headers +' + 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" '