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.

Sign up for GitHub

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: Add Cache-Control/max-age and ETag to IPNS requests #1921

Closed
wants to merge 6 commits into from
Closed

Gateway: Add Cache-Control/max-age and ETag to IPNS requests #1921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7e828de
sharness tests for caching and --ttl
ion1 Nov 5, 2015
52e567a
name publish: Forbid negative --ttl
ion1 Nov 5, 2015
9fee188
routing: Refactor EOL computation out of cacheSet; sanity check for EOL
ion1 Nov 2, 2015
9338b40
Make Resolver return TTL information
ion1 Oct 29, 2015
274c261
Gateway: Add Cache-Control/max-age to IPNS requests
ion1 Oct 31, 2015
e312125
Gateway: Add ETag to IPNS requests
ion1 Oct 31, 2015
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
4 changes: 4 additions & 0 deletions core/commands/publish.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ Publish an <ipfs-path> to another public key (not implemented):
res.SetError(err, cmds.ErrNormal)
return
}
if d < 0 {
res.SetError(fmt.Errorf("ttl can not be negative"), cmds.ErrNormal)
return
}

ctx = context.WithValue(ctx, "ipns-publish-ttl", d)
}
Expand Down
41 changes: 32 additions & 9 deletions core/corehttp/gateway_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ import (
path "github.com/ipfs/go-ipfs/path"
"github.com/ipfs/go-ipfs/routing"
uio "github.com/ipfs/go-ipfs/unixfs/io"
infd "github.com/ipfs/go-ipfs/util/infduration"
)

const (
ipfsPathPrefix = "/ipfs/"
ipnsPathPrefix = "/ipns/"

// HTTP needs a finite TTL.
ImmutableTTL = 10 * 365 * 24 * time.Hour
)

// gatewayHandler is a HTTP handler that serves IPFS objects (accessible by default at /ipfs/<path>)
Expand Down Expand Up @@ -111,13 +115,21 @@ 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.ResolveWithTTL(ctx, i.node, path.Path(urlPath))
if err != nil {
webError(w, "Path Resolve error", err, http.StatusBadRequest)
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
Expand Down Expand Up @@ -146,15 +158,14 @@ 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?
Copy link
Member

Choose a reason for hiding this comment

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

yes let's not cache 404s anytime soon.

Copy link
Author

Choose a reason for hiding this comment

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

It’s unclear to me where it would hurt, though. Given a successfully resolved /ipfs/QmFoo, the knowledge that “/ipfs/QmFoo/fileA exists” should be as cacheable as “/ipfs/QmFoo/fileB does not exist” for example. QmFoo is immutable after all.

The same applies to /ipns/QmBar. The record’s TTL dictates that the knowledge that “/ipns/QmBar/fileA exists” can be cached for just as long as that “/ipns/QmBar/fileB does not exist”.

Copy link
Member

Choose a reason for hiding this comment

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

i think @ion1 has a point here, but its going to be difficult to do it right. A 404 should only be cached if its for a link under a valid resolved hash. A 'could not find this hash on the network' 404 should never be cached.

Copy link
Author

Choose a reason for hiding this comment

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

The code should already return a TTL of 0 for all network failures.

setSuccessHeaders(w, ttl, etag)

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)
}
Expand Down Expand Up @@ -446,6 +457,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)
Copy link
Member

Choose a reason for hiding this comment

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

i really hope noone forgets to call this somewhere.

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)
Expand All @@ -455,3 +467,14 @@ 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 infd.Duration, etag string) {
maxAge := infd.GetFiniteDuration(ttl, ImmutableTTL)
w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(maxAge.Seconds())))
w.Header().Set("ETag", etag)
}

func unsetSuccessHeaders(w http.ResponseWriter) {
w.Header().Del("Cache-Control")
w.Header().Del("ETag")
}
109 changes: 95 additions & 14 deletions core/corehttp/gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package corehttp

import (
"errors"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
Expand All @@ -17,21 +18,37 @@ import (
path "github.com/ipfs/go-ipfs/path"
repo "github.com/ipfs/go-ipfs/repo"
config "github.com/ipfs/go-ipfs/repo/config"
infd "github.com/ipfs/go-ipfs/util/infduration"
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 infd.Duration
}

func (m mockNamesys) Resolve(ctx context.Context, name string) (value path.Path, err error) {
return m.ResolveN(ctx, name, namesys.DefaultDepthLimit)
p, _, err := m.ResolveWithTTL(ctx, name)
return p, err
}

func (m mockNamesys) ResolveN(ctx context.Context, name string, depth int) (value path.Path, err error) {
p, ok := m[name]
p, _, err := m.ResolveNWithTTL(ctx, name, depth)
return p, err
}

func (m mockNamesys) ResolveWithTTL(ctx context.Context, name string) (value path.Path, ttl infd.Duration, err error) {
return m.ResolveNWithTTL(ctx, name, namesys.DefaultDepthLimit)
}

func (m mockNamesys) ResolveNWithTTL(ctx context.Context, name string, depth int) (value path.Path, ttl infd.Duration, err error) {
entry, ok := m[name]
if !ok {
return "", namesys.ErrResolveFailed
return "", infd.FiniteDuration(0), namesys.ErrResolveFailed
}
return p, nil
return entry.path, entry.ttl, nil
}

func (m mockNamesys) Publish(ctx context.Context, name ci.PrivKey, value path.Path) error {
Expand Down Expand Up @@ -114,21 +131,28 @@ func TestGatewayGet(t *testing.T) {
if err != nil {
t.Fatal(err)
}
ns["/ipns/example.com"] = path.FromString("/ipfs/" + k)
kTTL := infd.FiniteDuration(10 * time.Minute)
ns["/ipns/example.com"] = mockEntry{
path: path.FromString("/ipfs/" + k),
Copy link
Member

Choose a reason for hiding this comment

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

completely out of scope of this PR, but I'm thinking we should probably make FromString do the parsing check in path.ParsePath, and panic if it fails.

ttl: kTTL,
}

t.Log(ts.URL)
infTTL := infd.InfiniteDuration()
for _, test := range []struct {
host string
path string
status int
ttl *infd.Duration
etag string
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, &infTTL, 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)
Expand All @@ -148,6 +172,8 @@ 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)
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)
Expand Down Expand Up @@ -188,8 +214,16 @@ 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)
ns["/ipns/example.net"] = path.FromString("/ipfs/" + k.String())
kTTL := infd.FiniteDuration(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)
Expand All @@ -213,6 +247,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) {
Expand Down Expand Up @@ -249,8 +286,20 @@ 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)
ns["/ipns/example.net"] = path.FromString("/ipfs/" + k.String())
kTTL := infd.FiniteDuration(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)
Expand Down Expand Up @@ -282,6 +331,9 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
t.Fatalf("expected file in directory listing")
}

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)
if err != nil {
Expand Down Expand Up @@ -312,6 +364,9 @@ func TestIPNSHostnameBacklinks(t *testing.T) {
t.Fatalf("expected file in directory listing")
}

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)
if err != nil {
Expand Down Expand Up @@ -341,4 +396,30 @@ 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)
checkETag(t, "http://example.net/foo/bar/", res, kBar.String())
}

func checkCacheControl(t *testing.T, urlstr string, resp *http.Response, ttl *infd.Duration) {
expCacheControl := ""
if ttl != nil {
maxAge := infd.GetFiniteDuration(*ttl, ImmutableTTL)
expCacheControl = fmt.Sprintf("public, max-age=%d", int(maxAge.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)
}
}

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)
}
}
29 changes: 21 additions & 8 deletions core/pathresolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

merkledag "github.com/ipfs/go-ipfs/merkledag"
path "github.com/ipfs/go-ipfs/path"
infd "github.com/ipfs/go-ipfs/util/infduration"
)

// ErrNoNamesys is an explicit error for when an IPFS node doesn't
Expand All @@ -20,38 +21,50 @@ var ErrNoNamesys = errors.New(
// entries and returning the final merkledag node. Effectively
// enables /ipns/, /dns/, etc. in commands.
func Resolve(ctx context.Context, n *IpfsNode, p path.Path) (*merkledag.Node, error) {
node, _, err := ResolveWithTTL(ctx, n, p)
return node, err
}

// ResolveWithTTL is like Resolve but also returns a time-to-live value which
// indicates the maximum amount of time the result (whether a success or an
// error) may be cached.
func ResolveWithTTL(ctx context.Context, n *IpfsNode, p path.Path) (*merkledag.Node, infd.Duration, error) {
ttl := infd.InfiniteDuration()

if strings.HasPrefix(p.String(), "/ipns/") {
// resolve ipns paths

// TODO(cryptix): we sould be able to query the local cache for the path
if n.Namesys == nil {
return nil, ErrNoNamesys
return nil, infd.FiniteDuration(0), ErrNoNamesys
}

seg := p.Segments()

if len(seg) < 2 || seg[1] == "" { // just "/<protocol/>" without further segments
return nil, path.ErrNoComponents
return nil, infd.InfiniteDuration(), path.ErrNoComponents
}

extensions := seg[2:]
resolvable, err := path.FromSegments("/", seg[0], seg[1])
if err != nil {
return nil, err
return nil, infd.InfiniteDuration(), err
}

respath, err := n.Namesys.Resolve(ctx, resolvable.String())
resPath, resTTL, err := n.Namesys.ResolveWithTTL(ctx, resolvable.String())
ttl = resTTL
if err != nil {
return nil, err
return nil, ttl, err
}

segments := append(respath.Segments(), extensions...)
segments := append(resPath.Segments(), extensions...)
p, err = path.FromSegments("/", segments...)
if err != nil {
return nil, err
return nil, ttl, err
}
}

// ok, we have an ipfs path now (or what we'll treat as one)
return n.Resolver.ResolvePath(ctx, p)
node, err := n.Resolver.ResolvePath(ctx, p)
return node, ttl, err
}
Loading