Gateway: Add Cache-Control/max-age to IPNS requests

License: MIT
Signed-off-by: Johan Kiviniemi <[email protected]>

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))
+	nd, ttl, err := core.ResolveDetailed(ctx, i.node, path.Path(urlPath))
 	if err != nil {
 		webError(w, "Path Resolve error", err, http.StatusBadRequest)
 		return
@@ -146,15 +146,15 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
 		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.
+	// TODO: Consider letting clients cache 404s.  Are there cases where
+	// that would hurt?
+	setSuccessHeaders(w, ttl)
+
 	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)
 	}
@@ -446,6 +446,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 +456,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, ttl time.Duration) {
+	w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(ttl.Seconds())))
+}
+
+func unsetSuccessHeaders(w http.ResponseWriter) {
+	w.Header().Del("Cache-Control")
+	w.Header().Del("ETag")
+}

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) {
 	p, _, err := m.ResolveDetailed(ctx, name)
@@ -37,11 +43,11 @@ func (m mockNamesys) ResolveDetailed(ctx context.Context, name string) (path.Pat
 }
 
 func (m mockNamesys) ResolveNDetailed(ctx context.Context, name string, depth int) (path.Path, time.Duration, error) {
-	p, ok := m[name]
+	entry, ok := m[name]
 	if !ok {
 		return "", 0, namesys.ErrResolveFailed
 	}
-	return p, 0, nil
+	return entry.path, entry.ttl, nil
 }
 
 func (m mockNamesys) Publish(ctx context.Context, name ci.PrivKey, value path.Path) error {
@@ -124,21 +130,27 @@ 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)
+	immutableTTL := namesys.ImmutableTTL // Need a pointer to the constant
 	for _, test := range []struct {
 		host   string
 		path   string
 		status int
+		ttl    *time.Duration
 		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, &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"},
 	} {
 		var c http.Client
 		r, err := http.NewRequest("GET", ts.URL+test.path, nil)
@@ -158,6 +170,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 +212,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 +240,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, &kTTL)
 }
 
 func TestIPNSHostnameBacklinks(t *testing.T) {
@@ -260,7 +278,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 +314,8 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
 		t.Fatalf("expected file in directory listing")
 	}
 
+	checkCacheControl(t, "http://example.net/foo/", res, &kTTL)
+
 	// make request to directory listing
 	req, err = http.NewRequest("GET", ts.URL, nil)
 	if err != nil {
@@ -322,6 +346,8 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
 		t.Fatalf("expected file in directory listing")
 	}
 
+	checkCacheControl(t, "http://example.net/", res, &kTTL)
+
 	// make request to directory listing
 	req, err = http.NewRequest("GET", ts.URL+"/foo/bar/", nil)
 	if err != nil {
@@ -351,4 +377,17 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
 	if !strings.Contains(s, "<a href=\"/foo/bar/file.txt\">") {
 		t.Fatalf("expected file in directory listing")
 	}
+
+	checkCacheControl(t, "http://example.net/foo/bar/", res, &kTTL)
+}
+
+func checkCacheControl(t *testing.T, urlstr string, resp *http.Response, ttl *time.Duration) {
+	expCacheControl := ""
+	if ttl != nil {
+		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)
+	}
 }