Skip to content

Commit

Permalink
refactor: futher cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
hacdias committed Sep 6, 2023
1 parent bb397e5 commit 17cd5dd
Show file tree
Hide file tree
Showing 31 changed files with 477 additions and 457 deletions.
18 changes: 17 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ The following emojis are used to highlight certain changes:
### Added

* ✨ The `routing/http` implements Delegated Peer Routing introduced in [IPIP-417](https://github.com/ipfs/specs/pull/417).
* The gateway now sets a `Cache-Control` header for requests under the `/ipns/` namespace
if the TTL for the corresponding IPNS Records or DNSLink entities is known.

### Changed

Expand All @@ -31,14 +33,28 @@ The following emojis are used to highlight certain changes:
condensed the different path-related packages under a single one. Therefore, there
are many breaking changes. Please consult the [documentation](https://pkg.go.dev/github.com/ipfs/boxo/path)
for more details on how to use the new package.

* 🛠 The `namesys` package has been refactored. The following are the largest modifications:
* The options in `coreiface/options/namesys` have been moved to `namesys` and their names
have been made more consistent.
* Many of the exported structs and functions have been renamed in order to be consistent with
the remaining packages.
* `namesys.Resolver.Resolve` now returns a TTL, in addition to the resolved path. If the
TTL is unknown, 0 is returned. `IPNSResolver` is able to resolve a TTL, while `DNSResolver`
is not.
* `namesys/resolver.ResolveIPNS` has been moved to `namesys.ResolveIPNS` and now returns a TTL
in addition to the resolved path.
* 🛠 The `gateway`'s `IPFSBackend.ResolveMutable` is now expected to return a TTL in addition to
the resolved path. If the TTL is unknown, 0 should be returned.

### Removed

* 🛠 The `routing/http` package experienced following removals:
* Server and client no longer support the experimental `Provide` method.
`ProvideBitswap` is still usable, but marked as deprecated. A protocol-agnostic
provide mechanism is being worked on in [IPIP-378](https://github.com/ipfs/specs/pull/378).
* Server no longer exports `FindProvidersPath` and `ProvidePath`.
* 🛠 The `coreiface/options/namesys` package has been removed.
* 🛠 The `namesys.StartSpan` function is no longer exported.

### Fixed

Expand Down
36 changes: 17 additions & 19 deletions gateway/blocks_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"net/http"
"strings"
"time"

"github.com/ipfs/boxo/blockservice"
blockstore "github.com/ipfs/boxo/blockstore"
Expand All @@ -17,8 +18,8 @@ import (
"github.com/ipfs/boxo/ipld/merkledag"
ufile "github.com/ipfs/boxo/ipld/unixfs/file"
uio "github.com/ipfs/boxo/ipld/unixfs/io"
"github.com/ipfs/boxo/ipns"
"github.com/ipfs/boxo/namesys"
"github.com/ipfs/boxo/namesys/resolve"
"github.com/ipfs/boxo/path"
"github.com/ipfs/boxo/path/resolver"
blocks "github.com/ipfs/go-block-format"
Expand All @@ -38,7 +39,6 @@ import (
"github.com/ipld/go-ipld-prime/traversal/selector"
selectorparse "github.com/ipld/go-ipld-prime/traversal/selector/parse"
routinghelpers "github.com/libp2p/go-libp2p-routing-helpers"
"github.com/libp2p/go-libp2p/core/peer"
"github.com/libp2p/go-libp2p/core/routing"
mc "github.com/multiformats/go-multicodec"

Expand Down Expand Up @@ -556,18 +556,23 @@ func (bb *BlocksBackend) getPathRoots(ctx context.Context, contentPath path.Immu
return pathRoots, lastPath, nil
}

func (bb *BlocksBackend) ResolveMutable(ctx context.Context, p path.Path) (path.ImmutablePath, error) {
func (bb *BlocksBackend) ResolveMutable(ctx context.Context, p path.Path) (path.ImmutablePath, time.Duration, error) {
switch p.Namespace() {
case path.IPNSNamespace:
p, err := resolve.ResolveIPNS(ctx, bb.namesys, p)
p, ttl, err := namesys.ResolveIPNS(ctx, bb.namesys, p)
if err != nil {
return nil, err
return nil, 0, err
}
ip, err := path.NewImmutablePath(p)
if err != nil {
return nil, 0, err

Check warning on line 568 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L568

Added line #L568 was not covered by tests
}
return path.NewImmutablePath(p)
return ip, ttl, nil
case path.IPFSNamespace:
return path.NewImmutablePath(p)
ip, err := path.NewImmutablePath(p)
return ip, 0, err

Check warning on line 573 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L572-L573

Added lines #L572 - L573 were not covered by tests
default:
return nil, NewErrorStatusCode(fmt.Errorf("unsupported path namespace: %s", p.Namespace()), http.StatusNotImplemented)
return nil, 0, NewErrorStatusCode(fmt.Errorf("unsupported path namespace: %s", p.Namespace()), http.StatusNotImplemented)

Check warning on line 575 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L575

Added line #L575 was not covered by tests
}
}

Expand All @@ -576,19 +581,12 @@ func (bb *BlocksBackend) GetIPNSRecord(ctx context.Context, c cid.Cid) ([]byte,
return nil, NewErrorStatusCode(errors.New("IPNS Record responses are not supported by this gateway"), http.StatusNotImplemented)
}

// Fails fast if the CID is not an encoded Libp2p Key, avoids wasteful
// round trips to the remote routing provider.
if mc.Code(c.Type()) != mc.Libp2pKey {
return nil, NewErrorStatusCode(errors.New("cid codec must be libp2p-key"), http.StatusBadRequest)
}

// The value store expects the key itself to be encoded as a multihash.
id, err := peer.FromCid(c)
name, err := ipns.NameFromCid(c)

Check warning on line 584 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L584

Added line #L584 was not covered by tests
if err != nil {
return nil, err
return nil, NewErrorStatusCode(err, http.StatusBadRequest)

Check warning on line 586 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L586

Added line #L586 was not covered by tests
}

return bb.routing.GetValue(ctx, "/ipns/"+string(id))
return bb.routing.GetValue(ctx, string(name.RoutingKey()))

Check warning on line 589 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L589

Added line #L589 was not covered by tests
}

func (bb *BlocksBackend) GetDNSLinkRecord(ctx context.Context, hostname string) (path.Path, error) {
Expand Down Expand Up @@ -628,7 +626,7 @@ func (bb *BlocksBackend) ResolvePath(ctx context.Context, path path.ImmutablePat
func (bb *BlocksBackend) resolvePath(ctx context.Context, p path.Path) (path.ImmutablePath, error) {
var err error
if p.Namespace() == path.IPNSNamespace {
p, err = resolve.ResolveIPNS(ctx, bb.namesys, p)
p, _, err = namesys.ResolveIPNS(ctx, bb.namesys, p)

Check warning on line 629 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L629

Added line #L629 was not covered by tests
if err != nil {
return nil, err
}
Expand Down
5 changes: 3 additions & 2 deletions gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sort"
"strconv"
"strings"
"time"

"github.com/ipfs/boxo/files"
"github.com/ipfs/boxo/gateway/assets"
Expand Down Expand Up @@ -303,11 +304,11 @@ type IPFSBackend interface {
GetIPNSRecord(context.Context, cid.Cid) ([]byte, error)

// ResolveMutable takes a mutable path and resolves it into an immutable one. This means recursively resolving any
// DNSLink or IPNS records.
// DNSLink or IPNS records. It should also return a TTL. If the TTL is unknown, 0 should be returned.
//
// For example, given a mapping from `/ipns/dnslink.tld -> /ipns/ipns-id/mydirectory` and `/ipns/ipns-id` to
// `/ipfs/some-cid`, the result of passing `/ipns/dnslink.tld/myfile` would be `/ipfs/some-cid/mydirectory/myfile`.
ResolveMutable(context.Context, path.Path) (path.ImmutablePath, error)
ResolveMutable(context.Context, path.Path) (path.ImmutablePath, time.Duration, error)

// GetDNSLinkRecord returns the DNSLink TXT record for the provided FQDN.
// Unlike ResolvePath, it does not perform recursive resolution. It only
Expand Down
79 changes: 64 additions & 15 deletions gateway/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ func TestGatewayGet(t *testing.T) {
return p
}

backend.namesys["/ipns/example.com"] = path.NewIPFSPath(k.Cid())
backend.namesys["/ipns/working.example.com"] = k
backend.namesys["/ipns/double.example.com"] = mustMakeDNSLinkPath("working.example.com")
backend.namesys["/ipns/triple.example.com"] = mustMakeDNSLinkPath("double.example.com")
backend.namesys["/ipns/broken.example.com"] = mustMakeDNSLinkPath(k.Cid().String())
backend.namesys["/ipns/example.com"] = newMockNamesysItem(path.NewIPFSPath(k.Cid()), 0)
backend.namesys["/ipns/working.example.com"] = newMockNamesysItem(k, 0)
backend.namesys["/ipns/double.example.com"] = newMockNamesysItem(mustMakeDNSLinkPath("working.example.com"), 0)
backend.namesys["/ipns/triple.example.com"] = newMockNamesysItem(mustMakeDNSLinkPath("double.example.com"), 0)
backend.namesys["/ipns/broken.example.com"] = newMockNamesysItem(mustMakeDNSLinkPath(k.Cid().String()), 0)
// We picked .man because:
// 1. It's a valid TLD.
// 2. Go treats it as the file extension for "man" files (even though
// nobody actually *uses* this extension, AFAIK).
//
// Unfortunately, this may not work on all platforms as file type
// detection is platform dependent.
backend.namesys["/ipns/example.man"] = k
backend.namesys["/ipns/example.man"] = newMockNamesysItem(k, 0)

for _, test := range []struct {
host string
Expand Down Expand Up @@ -98,7 +98,7 @@ func TestPretty404(t *testing.T) {
t.Logf("test server url: %s", ts.URL)

host := "example.net"
backend.namesys["/ipns/"+host] = path.NewIPFSPath(root)
backend.namesys["/ipns/"+host] = newMockNamesysItem(path.NewIPFSPath(root), 0)

for _, test := range []struct {
path string
Expand Down Expand Up @@ -158,7 +158,56 @@ func TestHeaders(t *testing.T) {
dagCborRoots = dirRoots + "," + dagCborCID
)

t.Run("Cache-Control is not immutable on generated /ipfs/ HTML dir listings", func(t *testing.T) {
t.Run("Cache-Control uses TTL for /ipns/ when it is known", func(t *testing.T) {
t.Parallel()

ts, backend, root := newTestServerAndNode(t, nil, "ipns-hostname-redirects.car")
backend.namesys["/ipns/example.net"] = newMockNamesysItem(path.NewIPFSPath(root), time.Second*30)

t.Run("UnixFS generated directory listing without index.html has no Cache-Control", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net/", nil)
res := mustDoWithoutRedirect(t, req)
require.Empty(t, res.Header["Cache-Control"])
})

t.Run("UnixFS directory with index.html has Cache-Control", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net/foo/", nil)
res := mustDoWithoutRedirect(t, req)
require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
})

t.Run("UnixFS file has Cache-Control", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net/foo/index.html", nil)
res := mustDoWithoutRedirect(t, req)
require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
})

t.Run("Raw block has Cache-Control", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=raw", nil)
res := mustDoWithoutRedirect(t, req)
require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
})

t.Run("DAG-JSON block has Cache-Control", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=dag-json", nil)
res := mustDoWithoutRedirect(t, req)
require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
})

t.Run("DAG-CBOR block has Cache-Control", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=dag-cbor", nil)
res := mustDoWithoutRedirect(t, req)
require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
})

t.Run("CAR block has Cache-Control", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipns/example.net?format=car", nil)
res := mustDoWithoutRedirect(t, req)
require.Equal(t, "public, max-age=30", res.Header.Get("Cache-Control"))
})
})

t.Run("Cache-Control is not immutable on generated /ipfs/ HTML dir listings", func(t *testing.T) {
req := mustNewRequest(t, http.MethodGet, ts.URL+"/ipfs/"+rootCID+"/", nil)
res := mustDoWithoutRedirect(t, req)

Expand Down Expand Up @@ -500,7 +549,7 @@ func TestRedirects(t *testing.T) {
t.Parallel()

ts, backend, root := newTestServerAndNode(t, nil, "ipns-hostname-redirects.car")
backend.namesys["/ipns/example.net"] = path.NewIPFSPath(root)
backend.namesys["/ipns/example.net"] = newMockNamesysItem(path.NewIPFSPath(root), 0)

// make request to directory containing index.html
req := mustNewRequest(t, http.MethodGet, ts.URL+"/foo", nil)
Expand Down Expand Up @@ -535,7 +584,7 @@ func TestRedirects(t *testing.T) {
t.Parallel()

backend, root := newMockBackend(t, "redirects-spa.car")
backend.namesys["/ipns/example.com"] = path.NewIPFSPath(root)
backend.namesys["/ipns/example.com"] = newMockNamesysItem(path.NewIPFSPath(root), 0)

ts := newTestServerWithConfig(t, backend, Config{
Headers: map[string][]string{},
Expand Down Expand Up @@ -672,8 +721,8 @@ func TestDeserializedResponses(t *testing.T) {
t.Parallel()

backend, root := newMockBackend(t, "fixtures.car")
backend.namesys["/ipns/trustless.com"] = path.NewIPFSPath(root)
backend.namesys["/ipns/trusted.com"] = path.NewIPFSPath(root)
backend.namesys["/ipns/trustless.com"] = newMockNamesysItem(path.NewIPFSPath(root), 0)
backend.namesys["/ipns/trusted.com"] = newMockNamesysItem(path.NewIPFSPath(root), 0)

ts := newTestServerWithConfig(t, backend, Config{
Headers: map[string][]string{},
Expand Down Expand Up @@ -735,8 +784,8 @@ func (mb *errorMockBackend) GetCAR(ctx context.Context, path path.ImmutablePath,
return ContentPathMetadata{}, nil, mb.err
}

func (mb *errorMockBackend) ResolveMutable(ctx context.Context, p path.Path) (path.ImmutablePath, error) {
return nil, mb.err
func (mb *errorMockBackend) ResolveMutable(ctx context.Context, path path.Path) (path.ImmutablePath, time.Duration, error) {
return nil, 0, mb.err
}

func (mb *errorMockBackend) GetIPNSRecord(ctx context.Context, c cid.Cid) ([]byte, error) {
Expand Down Expand Up @@ -819,7 +868,7 @@ func (mb *panicMockBackend) GetCAR(ctx context.Context, immutablePath path.Immut
panic("i am panicking")
}

func (mb *panicMockBackend) ResolveMutable(ctx context.Context, p path.Path) (path.ImmutablePath, error) {
func (mb *panicMockBackend) ResolveMutable(ctx context.Context, p path.Path) (path.ImmutablePath, time.Duration, error) {
panic("i am panicking")
}

Expand Down
35 changes: 17 additions & 18 deletions gateway/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var log = logging.Logger("boxo/gateway")

const (
ipfsPathPrefix = "/ipfs/"
ipnsPathPrefix = "/ipns/"
ipnsPathPrefix = ipns.NamespacePrefix
immutableCacheControl = "public, max-age=29030400, immutable"
)

Expand Down Expand Up @@ -188,6 +188,7 @@ type requestData struct {

// Defined for non IPNS Record requests.
immutablePath path.ImmutablePath
ttl time.Duration

// Defined if resolution has already happened.
pathMetadata *ContentPathMetadata
Expand Down Expand Up @@ -279,7 +280,7 @@ func (i *handler) getOrHeadHandler(w http.ResponseWriter, r *http.Request) {
}

if contentPath.Namespace().Mutable() {
rq.immutablePath, err = i.backend.ResolveMutable(r.Context(), contentPath)
rq.immutablePath, rq.ttl, err = i.backend.ResolveMutable(r.Context(), contentPath)
if err != nil {
err = fmt.Errorf("failed to resolve %s: %w", debugStr(contentPath.String()), err)
i.webError(w, r, err, http.StatusInternalServerError)
Expand Down Expand Up @@ -409,32 +410,30 @@ func panicHandler(w http.ResponseWriter) {
}
}

func addCacheControlHeaders(w http.ResponseWriter, r *http.Request, contentPath path.Path, cid cid.Cid, responseFormat string) (modtime time.Time) {
func addCacheControlHeaders(w http.ResponseWriter, r *http.Request, contentPath path.Path, ttl time.Duration, cid cid.Cid, responseFormat string) (modtime time.Time) {
// Best effort attempt to set an Etag based on the CID and response format.
// Setting an ETag is handled separately for CARs and IPNS records.
if etag := getEtag(r, cid, responseFormat); etag != "" {
w.Header().Set("Etag", etag)
}

// Set Cache-Control and Last-Modified based on contentPath properties
// Set Cache-Control and Last-Modified based on contentPath properties.
if contentPath.Namespace().Mutable() {
// mutable namespaces such as /ipns/ can't be cached forever

// For now we set Last-Modified to Now() to leverage caching heuristics built into modern browsers:
// https://github.com/ipfs/kubo/pull/8074#pullrequestreview-645196768
// but we should not set it to fake values and use Cache-Control based on TTL instead
modtime = time.Now()

// TODO: set Cache-Control based on TTL of IPNS/DNSLink: https://github.com/ipfs/kubo/issues/1818#issuecomment-1015849462
// TODO: set Last-Modified based on /ipns/ publishing timestamp?
if ttl > 0 {
// When we know the TTL, set the Cache-Control header and disable Last-Modified.
w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d", int(ttl.Seconds())))
modtime = noModtime
} else {
// Otherwise, we set Last-Modified to the current time to leverage caching heuristics
// built into modern browsers: https://github.com/ipfs/kubo/pull/8074#pullrequestreview-645196768
modtime = time.Now()
}
} else {
// immutable! CACHE ALL THE THINGS, FOREVER! wolololol
w.Header().Set("Cache-Control", immutableCacheControl)
modtime = noModtime // disable Last-Modified

// Set modtime to 'zero time' to disable Last-Modified header (superseded by Cache-Control)
modtime = noModtime

// TODO: set Last-Modified? - TBD - /ipfs/ modification metadata is present in unixfs 1.5 https://github.com/ipfs/kubo/issues/6920?
// TODO: consider setting Last-Modified if UnixFS V1.5 ever gets released
// with metadata: https://github.com/ipfs/kubo/issues/6920
}

return modtime
Expand Down
2 changes: 1 addition & 1 deletion gateway/handler_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (i *handler) serveRawBlock(ctx context.Context, w http.ResponseWriter, r *h
setContentDispositionHeader(w, name, "attachment")

// Set remaining headers
modtime := addCacheControlHeaders(w, r, rq.contentPath, blockCid, rawResponseFormat)
modtime := addCacheControlHeaders(w, r, rq.contentPath, rq.ttl, blockCid, rawResponseFormat)
w.Header().Set("Content-Type", rawResponseFormat)
w.Header().Set("X-Content-Type-Options", "nosniff") // no funny business in the browsers :^)

Expand Down
2 changes: 1 addition & 1 deletion gateway/handler_car.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R
setContentDispositionHeader(w, name, "attachment")

// Set Cache-Control (same logic as for a regular files)
addCacheControlHeaders(w, r, rq.contentPath, rootCid, carResponseFormat)
addCacheControlHeaders(w, r, rq.contentPath, rq.ttl, rootCid, carResponseFormat)

// Generate the CAR Etag.
etag := getCarEtag(rq.immutablePath, params, rootCid)
Expand Down
2 changes: 1 addition & 1 deletion gateway/handler_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (i *handler) renderCodec(ctx context.Context, w http.ResponseWriter, r *htt
}

// Set HTTP headers (for caching, etc). Etag will be replaced if handled by serveCodecHTML.
modtime := addCacheControlHeaders(w, r, rq.contentPath, resolvedPath.Cid(), responseContentType)
modtime := addCacheControlHeaders(w, r, rq.contentPath, rq.ttl, resolvedPath.Cid(), responseContentType)
name := setCodecContentDisposition(w, r, resolvedPath, responseContentType)
w.Header().Set("Content-Type", responseContentType)
w.Header().Set("X-Content-Type-Options", "nosniff")
Expand Down
Loading

0 comments on commit 17cd5dd

Please sign in to comment.