From 9ee4787b6813b3a66c200956a228145331131021 Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Thu, 22 Jun 2023 14:23:36 +0200 Subject: [PATCH 1/3] feat(gateway): support for order=, dups= parameters from IPIP-412 --- CHANGELOG.md | 5 +++ gateway/blocks_backend.go | 31 +++++++++++++-- gateway/gateway.go | 20 ++++++++-- gateway/gateway_test.go | 4 +- gateway/handler_car.go | 59 ++++++++++++++++++++++++---- gateway/handler_car_test.go | 77 +++++++++++++++++++++++++++++++++---- gateway/metrics.go | 2 +- gateway/utilities_test.go | 2 +- 8 files changed, 173 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 231c110b7..621975a0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ The following emojis are used to highlight certain changes: ### Added +* ✨ The gateway now supports the optional `order` and `dups` CAR parameters + from [IPIP-412](https://github.com/ipfs/specs/pull/412). `BlocksBackend` only + DFS ordering. If the request explicitly requests an ordering other than `dfs` + and `unk`, the request will return an error. + ### Changed * 🛠 The `ipns` package has been refactored. You should no longer use the direct Protobuf diff --git a/gateway/blocks_backend.go b/gateway/blocks_backend.go index 6d76bb013..822533a95 100644 --- a/gateway/blocks_backend.go +++ b/gateway/blocks_backend.go @@ -232,7 +232,25 @@ func (bb *BlocksBackend) Head(ctx context.Context, path ImmutablePath) (ContentP return md, fileNode, nil } -func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { + // Check if we support the request order. On unknown, change it to DFS. We change + // the parameter directly, which means that the caller can use the value to later construct + // the Content-Type header. + switch params.Order { + case DagOrderUnknown: + params.Order = DagOrderDFS + case DagOrderDFS: + // Do nothing + default: + return ContentPathMetadata{}, nil, fmt.Errorf("unsupported order: %s", params.Order) + } + + // Similarly, if params.Duplicates is not set, let's set it to false. + if params.Duplicates == nil { + v := false + params.Duplicates = &v + } + pathMetadata, err := bb.ResolvePath(ctx, p) if err != nil { return ContentPathMetadata{}, nil, err @@ -245,7 +263,12 @@ func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params Car r, w := io.Pipe() go func() { - cw, err := storage.NewWritable(w, []cid.Cid{pathMetadata.LastSegment.Cid()}, car.WriteAsCarV1(true)) + cw, err := storage.NewWritable( + w, + []cid.Cid{pathMetadata.LastSegment.Cid()}, + car.WriteAsCarV1(true), + car.AllowDuplicatePuts(*params.Duplicates), + ) if err != nil { // io.PipeWriter.CloseWithError always returns nil. _ = w.CloseWithError(err) @@ -279,7 +302,7 @@ func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params Car } // walkGatewaySimpleSelector walks the subgraph described by the path and terminal element parameters -func walkGatewaySimpleSelector(ctx context.Context, p ipfspath.Path, params CarParams, lsys *ipld.LinkSystem, pathResolver resolver.Resolver) error { +func walkGatewaySimpleSelector(ctx context.Context, p ipfspath.Path, params *CarParams, lsys *ipld.LinkSystem, pathResolver resolver.Resolver) error { // First resolve the path since we always need to. lastCid, remainder, err := pathResolver.ResolveToLastNode(ctx, p) if err != nil { @@ -312,7 +335,7 @@ func walkGatewaySimpleSelector(ctx context.Context, p ipfspath.Path, params CarP Ctx: ctx, LinkSystem: *lsys, LinkTargetNodePrototypeChooser: bsfetcher.DefaultPrototypeChooser, - LinkVisitOnlyOnce: true, // This is safe for the "all" selector + LinkVisitOnlyOnce: !*params.Duplicates, }, } diff --git a/gateway/gateway.go b/gateway/gateway.go index cf2ca9104..f872caa94 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -121,8 +121,10 @@ func (i ImmutablePath) IsValid() error { var _ path.Path = (*ImmutablePath)(nil) type CarParams struct { - Range *DagByteRange - Scope DagScope + Range *DagByteRange + Scope DagScope + Order DagOrder + Duplicates *bool } // DagByteRange describes a range request within a UnixFS file. "From" and @@ -189,6 +191,13 @@ const ( DagScopeBlock DagScope = "block" ) +type DagOrder string + +const ( + DagOrderDFS DagOrder = "dfs" + DagOrderUnknown DagOrder = "unk" +) + type ContentPathMetadata struct { PathSegmentRoots []cid.Cid LastSegment path.Resolved @@ -278,8 +287,11 @@ type IPFSBackend interface { ResolvePath(context.Context, ImmutablePath) (ContentPathMetadata, error) // GetCAR returns a CAR file for the given immutable path. It returns an error - // if there was an issue before the CAR streaming begins. - GetCAR(context.Context, ImmutablePath, CarParams) (ContentPathMetadata, io.ReadCloser, error) + // if there was an issue before the CAR streaming begins. If [CarParams.Duplicates] + // is nil, or if [CaraParams.Order] is Unknown, the implementer should change it + // such that the caller can form the response "Content-Type" header with the most + // amount of information. + GetCAR(context.Context, ImmutablePath, *CarParams) (ContentPathMetadata, io.ReadCloser, error) // IsCached returns whether or not the path exists locally. IsCached(context.Context, path.Path) bool diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index cc36da68f..d7896f748 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -669,7 +669,7 @@ func (mb *errorMockBackend) Head(ctx context.Context, path ImmutablePath) (Conte return ContentPathMetadata{}, nil, mb.err } -func (mb *errorMockBackend) GetCAR(ctx context.Context, path ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (mb *errorMockBackend) GetCAR(ctx context.Context, path ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { return ContentPathMetadata{}, nil, mb.err } @@ -753,7 +753,7 @@ func (mb *panicMockBackend) Head(ctx context.Context, immutablePath ImmutablePat panic("i am panicking") } -func (mb *panicMockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (mb *panicMockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { panic("i am panicking") } diff --git a/gateway/handler_car.go b/gateway/handler_car.go index e773c920e..3e38cb09b 100644 --- a/gateway/handler_car.go +++ b/gateway/handler_car.go @@ -39,7 +39,7 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R return false } - params, err := getCarParams(r) + params, err := getCarParams(r, rq.responseParams) if err != nil { i.webError(w, r, err, http.StatusBadRequest) return false @@ -90,7 +90,7 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R // sub-DAGs and IPLD selectors: https://github.com/ipfs/go-ipfs/issues/8769 w.Header().Set("Accept-Ranges", "none") - w.Header().Set("Content-Type", carResponseFormat+"; version=1") + w.Header().Set("Content-Type", getContentTypeFromCarParams(params)) w.Header().Set("X-Content-Type-Options", "nosniff") // no funny business in the browsers :^) _, copyErr := io.Copy(w, carFile) @@ -113,7 +113,7 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R return true } -func getCarParams(r *http.Request) (CarParams, error) { +func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, error) { queryParams := r.URL.Query() rangeStr, hasRange := queryParams.Get(carRangeBytesKey), queryParams.Has(carRangeBytesKey) scopeStr, hasScope := queryParams.Get(carTerminalElementTypeKey), queryParams.Has(carTerminalElementTypeKey) @@ -123,7 +123,7 @@ func getCarParams(r *http.Request) (CarParams, error) { rng, err := NewDagByteRange(rangeStr) if err != nil { err = fmt.Errorf("invalid entity-bytes: %w", err) - return CarParams{}, err + return nil, err } params.Range = &rng } @@ -134,13 +134,58 @@ func getCarParams(r *http.Request) (CarParams, error) { params.Scope = s default: err := fmt.Errorf("unsupported dag-scope %s", scopeStr) - return CarParams{}, err + return nil, err } } else { params.Scope = DagScopeAll } - return params, nil + switch order := DagOrder(formatParams["order"]); order { + case DagOrderUnknown, DagOrderDFS: + params.Order = order + case "": + params.Order = DagOrderUnknown + default: + return nil, fmt.Errorf("unsupported order %s", order) + } + + switch dups := formatParams["dups"]; dups { + case "y": + v := true + params.Duplicates = &v + case "n": + v := false + params.Duplicates = &v + case "": + // Acceptable, we do not set anything. + default: + return nil, fmt.Errorf("unsupported dups %s", dups) + } + + return ¶ms, nil +} + +func getContentTypeFromCarParams(params *CarParams) string { + h := strings.Builder{} + h.WriteString(carResponseFormat) + h.WriteString("; version=1; order=") + + if params.Order != "" { + h.WriteString(string(params.Order)) + } else { + h.WriteString(string(DagOrderUnknown)) + } + + if params.Duplicates != nil { + h.WriteString("; dups=") + if *params.Duplicates { + h.WriteString("y") + } else { + h.WriteString("n") + } + } + + return h.String() } func getCarRootCidAndLastSegment(imPath ImmutablePath) (cid.Cid, string, error) { @@ -164,7 +209,7 @@ func getCarRootCidAndLastSegment(imPath ImmutablePath) (cid.Cid, string, error) return rootCid, lastSegment, err } -func getCarEtag(imPath ImmutablePath, params CarParams, rootCid cid.Cid) string { +func getCarEtag(imPath ImmutablePath, params *CarParams, rootCid cid.Cid) string { data := imPath.String() if params.Scope != DagScopeAll { data += "." + string(params.Scope) diff --git a/gateway/handler_car_test.go b/gateway/handler_car_test.go index 858ccb85d..ad3ced51b 100644 --- a/gateway/handler_car_test.go +++ b/gateway/handler_car_test.go @@ -28,7 +28,7 @@ func TestCarParams(t *testing.T) { } for _, test := range tests { r := mustNewRequest(t, http.MethodGet, "http://example.com/?"+test.query, nil) - params, err := getCarParams(r) + params, err := getCarParams(r, map[string]string{}) if test.expectedError { assert.Error(t, err) } else { @@ -60,7 +60,7 @@ func TestCarParams(t *testing.T) { } for _, test := range tests { r := mustNewRequest(t, http.MethodGet, "http://example.com/?"+test.query, nil) - params, err := getCarParams(r) + params, err := getCarParams(r, map[string]string{}) if test.hasError { assert.Error(t, err) } else { @@ -73,6 +73,67 @@ func TestCarParams(t *testing.T) { } } }) + + t.Run("order and duplicates parsing", func(t *testing.T) { + t.Parallel() + + T := true + F := false + + tests := []struct { + acceptHeader string + expectedOrder DagOrder + expectedDuplicates *bool + }{ + {"application/vnd.ipld.car; order=dfs; dups=y", DagOrderDFS, &T}, + {"application/vnd.ipld.car; order=unk; dups=n", DagOrderUnknown, &F}, + {"application/vnd.ipld.car; order=unk", DagOrderUnknown, nil}, + {"application/vnd.ipld.car; dups=y", DagOrderUnknown, &T}, + {"application/vnd.ipld.car; dups=n", DagOrderUnknown, &F}, + {"application/vnd.ipld.car", DagOrderUnknown, nil}, + } + for _, test := range tests { + r := mustNewRequest(t, http.MethodGet, "http://example.com/", nil) + r.Header.Set("Accept", test.acceptHeader) + + mediaType, formatParams, err := customResponseFormat(r) + assert.NoError(t, err) + assert.Equal(t, carResponseFormat, mediaType) + + params, err := getCarParams(r, formatParams) + assert.NoError(t, err) + require.Equal(t, test.expectedOrder, params.Order) + + if test.expectedDuplicates == nil { + require.Nil(t, params.Duplicates) + } else { + require.Equal(t, *test.expectedDuplicates, *params.Duplicates) + } + } + }) +} + +func TestContentTypeFromCarParams(t *testing.T) { + t.Parallel() + + T := true + F := false + + tests := []struct { + params CarParams + header string + }{ + {CarParams{}, "application/vnd.ipld.car; version=1; order=unk"}, + {CarParams{Order: DagOrderDFS, Duplicates: &T}, "application/vnd.ipld.car; version=1; order=dfs; dups=y"}, + {CarParams{Order: DagOrderUnknown, Duplicates: &T}, "application/vnd.ipld.car; version=1; order=unk; dups=y"}, + {CarParams{Order: DagOrderUnknown}, "application/vnd.ipld.car; version=1; order=unk"}, + {CarParams{Duplicates: &T}, "application/vnd.ipld.car; version=1; order=unk; dups=y"}, + {CarParams{Duplicates: &F}, "application/vnd.ipld.car; version=1; order=unk; dups=n"}, + } + for _, test := range tests { + header := getContentTypeFromCarParams(&test.params) + assert.Equal(t, test.header, header) + } } func TestGetCarEtag(t *testing.T) { @@ -87,24 +148,24 @@ func TestGetCarEtag(t *testing.T) { t.Run("Etag with entity-bytes=0:* is the same as without query param", func(t *testing.T) { t.Parallel() - noRange := getCarEtag(imPath, CarParams{}, cid) - withRange := getCarEtag(imPath, CarParams{Range: &DagByteRange{From: 0}}, cid) + noRange := getCarEtag(imPath, &CarParams{}, cid) + withRange := getCarEtag(imPath, &CarParams{Range: &DagByteRange{From: 0}}, cid) require.Equal(t, noRange, withRange) }) t.Run("Etag with entity-bytes=1:* is different than without query param", func(t *testing.T) { t.Parallel() - noRange := getCarEtag(imPath, CarParams{}, cid) - withRange := getCarEtag(imPath, CarParams{Range: &DagByteRange{From: 1}}, cid) + noRange := getCarEtag(imPath, &CarParams{}, cid) + withRange := getCarEtag(imPath, &CarParams{Range: &DagByteRange{From: 1}}, cid) require.NotEqual(t, noRange, withRange) }) t.Run("Etags with different dag-scope are different", func(t *testing.T) { t.Parallel() - a := getCarEtag(imPath, CarParams{Scope: DagScopeAll}, cid) - b := getCarEtag(imPath, CarParams{Scope: DagScopeEntity}, cid) + a := getCarEtag(imPath, &CarParams{Scope: DagScopeAll}, cid) + b := getCarEtag(imPath, &CarParams{Scope: DagScopeEntity}, cid) require.NotEqual(t, a, b) }) } diff --git a/gateway/metrics.go b/gateway/metrics.go index 69e81425f..371fc7864 100644 --- a/gateway/metrics.go +++ b/gateway/metrics.go @@ -120,7 +120,7 @@ func (b *ipfsBackendWithMetrics) ResolvePath(ctx context.Context, path Immutable return md, err } -func (b *ipfsBackendWithMetrics) GetCAR(ctx context.Context, path ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (b *ipfsBackendWithMetrics) GetCAR(ctx context.Context, path ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { begin := time.Now() name := "IPFSBackend.GetCAR" ctx, span := spanTrace(ctx, name, trace.WithAttributes(attribute.String("path", path.String()))) diff --git a/gateway/utilities_test.go b/gateway/utilities_test.go index 27ba43a14..81aa68ddb 100644 --- a/gateway/utilities_test.go +++ b/gateway/utilities_test.go @@ -149,7 +149,7 @@ func (mb *mockBackend) Head(ctx context.Context, immutablePath ImmutablePath) (C return mb.gw.Head(ctx, immutablePath) } -func (mb *mockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (mb *mockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { return mb.gw.GetCAR(ctx, immutablePath, params) } From 0fa23f53d6491e4d95f2742f5730b81850cb5c36 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 13 Jul 2023 17:01:08 +0200 Subject: [PATCH 2/3] docs: ipip-412 changelog and better error messages --- CHANGELOG.md | 28 ++++++++++++++++++++-------- gateway/blocks_backend.go | 2 +- gateway/handler_car.go | 8 ++++---- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 621975a0d..d2d4b2536 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,17 +17,29 @@ The following emojis are used to highlight certain changes: ### Added * ✨ The gateway now supports the optional `order` and `dups` CAR parameters - from [IPIP-412](https://github.com/ipfs/specs/pull/412). `BlocksBackend` only - DFS ordering. If the request explicitly requests an ordering other than `dfs` - and `unk`, the request will return an error. + from [IPIP-412](https://github.com/ipfs/specs/pull/412). + * The `BlocksBackend` only implements `order=dfs` (Depth-First Search) + ordering, which was already the default behavior. + * If a request specifies no `dups`, response with `dups=n` is returned, which + was already the default behavior. + * If a request explicitly specifies a CAR `order` other than `dfs`, it will + result in an error. + * The only change to the default behavior on CAR responses is that we follow + IPIP-412 and make `order=dfs;dups=n` explicit in the returned + `Content-Type` HTTP header. ### Changed -* 🛠 The `ipns` package has been refactored. You should no longer use the direct Protobuf - version of the IPNS Record. Instead, we have a shiny new `ipns.Record` type that wraps - all the required functionality to work the best as possible with IPNS v2 Records. Please - check the [documentation](https://pkg.go.dev/github.com/ipfs/boxo/ipns) for more information, - and follow [ipfs/specs#376](https://github.com/ipfs/specs/issues/376) for related IPIP. +* 🛠 The `ipns` package has been refactored. + * You should no longer use the direct Protobuf version of the IPNS Record. + Instead, we have a shiny new `ipns.Record` type that wraps all the required + functionality to work the best as possible with IPNS v2 Records. Please + check the [documentation](https://pkg.go.dev/github.com/ipfs/boxo/ipns) for + more information, and follow + [ipfs/specs#376](https://github.com/ipfs/specs/issues/376) for related + IPIP. + * There is no change to IPNS Records produced by `boxo/ipns`, it still + produces both V1 and V2 signatures by default, it is still backward-compatible. ### Removed diff --git a/gateway/blocks_backend.go b/gateway/blocks_backend.go index 822533a95..3eef9dd62 100644 --- a/gateway/blocks_backend.go +++ b/gateway/blocks_backend.go @@ -242,7 +242,7 @@ func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params *Ca case DagOrderDFS: // Do nothing default: - return ContentPathMetadata{}, nil, fmt.Errorf("unsupported order: %s", params.Order) + return ContentPathMetadata{}, nil, fmt.Errorf("unsupported application/vnd.ipld.car block order parameter: %q", params.Order) } // Similarly, if params.Duplicates is not set, let's set it to false. diff --git a/gateway/handler_car.go b/gateway/handler_car.go index 3e38cb09b..d227055c3 100644 --- a/gateway/handler_car.go +++ b/gateway/handler_car.go @@ -122,7 +122,7 @@ func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, if hasRange { rng, err := NewDagByteRange(rangeStr) if err != nil { - err = fmt.Errorf("invalid entity-bytes: %w", err) + err = fmt.Errorf("invalid application/vnd.ipld.car entity-bytes URL parameter: %w", err) return nil, err } params.Range = &rng @@ -133,7 +133,7 @@ func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, case DagScopeEntity, DagScopeAll, DagScopeBlock: params.Scope = s default: - err := fmt.Errorf("unsupported dag-scope %s", scopeStr) + err := fmt.Errorf("unsupported application/vnd.ipld.car dag-scope URL parameter: %q", scopeStr) return nil, err } } else { @@ -146,7 +146,7 @@ func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, case "": params.Order = DagOrderUnknown default: - return nil, fmt.Errorf("unsupported order %s", order) + return nil, fmt.Errorf("unsupported application/vnd.ipld.car content type order parameter: %q", order) } switch dups := formatParams["dups"]; dups { @@ -159,7 +159,7 @@ func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, case "": // Acceptable, we do not set anything. default: - return nil, fmt.Errorf("unsupported dups %s", dups) + return nil, fmt.Errorf("unsupported application/vnd.ipld.car content type dups parameter: %q", dups) } return ¶ms, nil From c033fe5f94d121261ba9d8b26f9c66780b4b520a Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 14 Jul 2023 01:04:57 +0200 Subject: [PATCH 3/3] refactor: simplify processing of CarParams - adds more type safety: DuplicateBlocksPolicy is no longer a pointer to bool, and together with DagOrder has an explicit *Unspecified state. - removes passing pointer and reliance on mutation. now mutation happens only once, in buildCarParams func, where implicit defaults are set when order or dups are unspecified in the request - moved car version validation and other parameter related logic into the same build funcs - fixed a bug where ?format=car eclipsed params from Accept header --- gateway/blocks_backend.go | 26 ++------ gateway/gateway.go | 50 ++++++++++++--- gateway/gateway_test.go | 4 +- gateway/handler.go | 47 +++++++------- gateway/handler_car.go | 125 ++++++++++++++++++++++-------------- gateway/handler_car_test.go | 69 ++++++++++---------- gateway/metrics.go | 2 +- gateway/utilities_test.go | 2 +- 8 files changed, 186 insertions(+), 139 deletions(-) diff --git a/gateway/blocks_backend.go b/gateway/blocks_backend.go index 3eef9dd62..01ca49fec 100644 --- a/gateway/blocks_backend.go +++ b/gateway/blocks_backend.go @@ -232,25 +232,7 @@ func (bb *BlocksBackend) Head(ctx context.Context, path ImmutablePath) (ContentP return md, fileNode, nil } -func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { - // Check if we support the request order. On unknown, change it to DFS. We change - // the parameter directly, which means that the caller can use the value to later construct - // the Content-Type header. - switch params.Order { - case DagOrderUnknown: - params.Order = DagOrderDFS - case DagOrderDFS: - // Do nothing - default: - return ContentPathMetadata{}, nil, fmt.Errorf("unsupported application/vnd.ipld.car block order parameter: %q", params.Order) - } - - // Similarly, if params.Duplicates is not set, let's set it to false. - if params.Duplicates == nil { - v := false - params.Duplicates = &v - } - +func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { pathMetadata, err := bb.ResolvePath(ctx, p) if err != nil { return ContentPathMetadata{}, nil, err @@ -267,7 +249,7 @@ func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params *Ca w, []cid.Cid{pathMetadata.LastSegment.Cid()}, car.WriteAsCarV1(true), - car.AllowDuplicatePuts(*params.Duplicates), + car.AllowDuplicatePuts(params.Duplicates.Bool()), ) if err != nil { // io.PipeWriter.CloseWithError always returns nil. @@ -302,7 +284,7 @@ func (bb *BlocksBackend) GetCAR(ctx context.Context, p ImmutablePath, params *Ca } // walkGatewaySimpleSelector walks the subgraph described by the path and terminal element parameters -func walkGatewaySimpleSelector(ctx context.Context, p ipfspath.Path, params *CarParams, lsys *ipld.LinkSystem, pathResolver resolver.Resolver) error { +func walkGatewaySimpleSelector(ctx context.Context, p ipfspath.Path, params CarParams, lsys *ipld.LinkSystem, pathResolver resolver.Resolver) error { // First resolve the path since we always need to. lastCid, remainder, err := pathResolver.ResolveToLastNode(ctx, p) if err != nil { @@ -335,7 +317,7 @@ func walkGatewaySimpleSelector(ctx context.Context, p ipfspath.Path, params *Car Ctx: ctx, LinkSystem: *lsys, LinkTargetNodePrototypeChooser: bsfetcher.DefaultPrototypeChooser, - LinkVisitOnlyOnce: !*params.Duplicates, + LinkVisitOnlyOnce: !params.Duplicates.Bool(), }, } diff --git a/gateway/gateway.go b/gateway/gateway.go index f872caa94..780691a45 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -124,7 +124,7 @@ type CarParams struct { Range *DagByteRange Scope DagScope Order DagOrder - Duplicates *bool + Duplicates DuplicateBlocksPolicy } // DagByteRange describes a range request within a UnixFS file. "From" and @@ -194,10 +194,47 @@ const ( type DagOrder string const ( - DagOrderDFS DagOrder = "dfs" - DagOrderUnknown DagOrder = "unk" + DagOrderUnspecified DagOrder = "" + DagOrderUnknown DagOrder = "unk" + DagOrderDFS DagOrder = "dfs" ) +// DuplicateBlocksPolicy represents the content type parameter 'dups' (IPIP-412) +type DuplicateBlocksPolicy int + +const ( + DuplicateBlocksUnspecified DuplicateBlocksPolicy = iota // 0 - implicit default + DuplicateBlocksIncluded // 1 - explicitly include duplicates + DuplicateBlocksExcluded // 2 - explicitly NOT include duplicates +) + +// NewDuplicateBlocksPolicy returns DuplicateBlocksPolicy based on the content type parameter 'dups' (IPIP-412) +func NewDuplicateBlocksPolicy(dupsValue string) DuplicateBlocksPolicy { + switch dupsValue { + case "y": + return DuplicateBlocksIncluded + case "n": + return DuplicateBlocksExcluded + } + return DuplicateBlocksUnspecified +} + +func (d DuplicateBlocksPolicy) Bool() bool { + // duplicates should be returned only when explicitly requested, + // so any other state than DuplicateBlocksIncluded should return false + return d == DuplicateBlocksIncluded +} + +func (d DuplicateBlocksPolicy) String() string { + switch d { + case DuplicateBlocksIncluded: + return "y" + case DuplicateBlocksExcluded: + return "n" + } + return "" +} + type ContentPathMetadata struct { PathSegmentRoots []cid.Cid LastSegment path.Resolved @@ -287,11 +324,8 @@ type IPFSBackend interface { ResolvePath(context.Context, ImmutablePath) (ContentPathMetadata, error) // GetCAR returns a CAR file for the given immutable path. It returns an error - // if there was an issue before the CAR streaming begins. If [CarParams.Duplicates] - // is nil, or if [CaraParams.Order] is Unknown, the implementer should change it - // such that the caller can form the response "Content-Type" header with the most - // amount of information. - GetCAR(context.Context, ImmutablePath, *CarParams) (ContentPathMetadata, io.ReadCloser, error) + // if there was an issue before the CAR streaming begins. + GetCAR(context.Context, ImmutablePath, CarParams) (ContentPathMetadata, io.ReadCloser, error) // IsCached returns whether or not the path exists locally. IsCached(context.Context, path.Path) bool diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index d7896f748..cc36da68f 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -669,7 +669,7 @@ func (mb *errorMockBackend) Head(ctx context.Context, path ImmutablePath) (Conte return ContentPathMetadata{}, nil, mb.err } -func (mb *errorMockBackend) GetCAR(ctx context.Context, path ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (mb *errorMockBackend) GetCAR(ctx context.Context, path ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { return ContentPathMetadata{}, nil, mb.err } @@ -753,7 +753,7 @@ func (mb *panicMockBackend) Head(ctx context.Context, immutablePath ImmutablePat panic("i am panicking") } -func (mb *panicMockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (mb *panicMockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { panic("i am panicking") } diff --git a/gateway/handler.go b/gateway/handler.go index 102593449..8c1358553 100644 --- a/gateway/handler.go +++ b/gateway/handler.go @@ -637,28 +637,9 @@ const ( // return explicit response format if specified in request as query parameter or via Accept HTTP header func customResponseFormat(r *http.Request) (mediaType string, params map[string]string, err error) { - // Translate query param to a content type, if present. - if formatParam := r.URL.Query().Get("format"); formatParam != "" { - switch formatParam { - case "raw": - return rawResponseFormat, nil, nil - case "car": - return carResponseFormat, nil, nil - case "tar": - return tarResponseFormat, nil, nil - case "json": - return jsonResponseFormat, nil, nil - case "cbor": - return cborResponseFormat, nil, nil - case "dag-json": - return dagJsonResponseFormat, nil, nil - case "dag-cbor": - return dagCborResponseFormat, nil, nil - case "ipns-record": - return ipnsRecordResponseFormat, nil, nil - } - } - + // First, inspect Accept header, as it may not only include content type, but also optional parameters. + // such as CAR version or additional ones from IPIP-412. + // // Browsers and other user agents will send Accept header with generic types like: // Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8 // We only care about explicit, vendor-specific content-types and respond to the first match (in order). @@ -681,6 +662,28 @@ func customResponseFormat(r *http.Request) (mediaType string, params map[string] } } + // If no Accept header, translate query param to a content type, if present. + if formatParam := r.URL.Query().Get("format"); formatParam != "" { + switch formatParam { + case "raw": + return rawResponseFormat, nil, nil + case "car": + return carResponseFormat, nil, nil + case "tar": + return tarResponseFormat, nil, nil + case "json": + return jsonResponseFormat, nil, nil + case "cbor": + return cborResponseFormat, nil, nil + case "dag-json": + return dagJsonResponseFormat, nil, nil + case "dag-cbor": + return dagCborResponseFormat, nil, nil + case "ipns-record": + return ipnsRecordResponseFormat, nil, nil + } + } + // If none of special-cased content types is found, return empty string // to indicate default, implicit UnixFS response should be prepared return "", nil, nil diff --git a/gateway/handler_car.go b/gateway/handler_car.go index d227055c3..553519988 100644 --- a/gateway/handler_car.go +++ b/gateway/handler_car.go @@ -30,16 +30,7 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R ctx, cancel := context.WithCancel(ctx) defer cancel() - switch rq.responseParams["version"] { - case "": // noop, client does not care about version - case "1": // noop, we support this - default: - err := fmt.Errorf("unsupported CAR version: only version=1 is supported") - i.webError(w, r, err, http.StatusBadRequest) - return false - } - - params, err := getCarParams(r, rq.responseParams) + params, err := buildCarParams(r, rq.responseParams) if err != nil { i.webError(w, r, err, http.StatusBadRequest) return false @@ -90,7 +81,7 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R // sub-DAGs and IPLD selectors: https://github.com/ipfs/go-ipfs/issues/8769 w.Header().Set("Accept-Ranges", "none") - w.Header().Set("Content-Type", getContentTypeFromCarParams(params)) + w.Header().Set("Content-Type", buildContentTypeFromCarParams(params)) w.Header().Set("X-Content-Type-Options", "nosniff") // no funny business in the browsers :^) _, copyErr := io.Copy(w, carFile) @@ -113,7 +104,15 @@ func (i *handler) serveCAR(ctx context.Context, w http.ResponseWriter, r *http.R return true } -func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, error) { +// buildCarParams returns CarParams based on the request, any optional parameters +// passed in URL, Accept header and the implicit defaults specific to boxo +// implementation, such as block order and duplicates status. +// +// If any of the optional content type parameters (e.g., CAR order or +// duplicates) are unspecified or empty, the function will automatically infer +// default values. +func buildCarParams(r *http.Request, contentTypeParams map[string]string) (CarParams, error) { + // URL query parameters queryParams := r.URL.Query() rangeStr, hasRange := queryParams.Get(carRangeBytesKey), queryParams.Has(carRangeBytesKey) scopeStr, hasScope := queryParams.Get(carTerminalElementTypeKey), queryParams.Has(carTerminalElementTypeKey) @@ -123,7 +122,7 @@ func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, rng, err := NewDagByteRange(rangeStr) if err != nil { err = fmt.Errorf("invalid application/vnd.ipld.car entity-bytes URL parameter: %w", err) - return nil, err + return CarParams{}, err } params.Range = &rng } @@ -134,55 +133,72 @@ func getCarParams(r *http.Request, formatParams map[string]string) (*CarParams, params.Scope = s default: err := fmt.Errorf("unsupported application/vnd.ipld.car dag-scope URL parameter: %q", scopeStr) - return nil, err + return CarParams{}, err } } else { params.Scope = DagScopeAll } - switch order := DagOrder(formatParams["order"]); order { - case DagOrderUnknown, DagOrderDFS: - params.Order = order - case "": - params.Order = DagOrderUnknown - default: - return nil, fmt.Errorf("unsupported application/vnd.ipld.car content type order parameter: %q", order) - } - - switch dups := formatParams["dups"]; dups { - case "y": - v := true - params.Duplicates = &v - case "n": - v := false - params.Duplicates = &v - case "": - // Acceptable, we do not set anything. + // application/vnd.ipld.car content type parameters from Accept header + + // version of CAR format + switch contentTypeParams["version"] { + case "": // noop, client does not care about version + case "1": // noop, we support this default: - return nil, fmt.Errorf("unsupported application/vnd.ipld.car content type dups parameter: %q", dups) + return CarParams{}, fmt.Errorf("unsupported application/vnd.ipld.car version: only version=1 is supported") + } + + // optional order from IPIP-412 + if order := DagOrder(contentTypeParams["order"]); order != DagOrderUnspecified { + switch order { + case DagOrderUnknown, DagOrderDFS: + params.Order = order + default: + return CarParams{}, fmt.Errorf("unsupported application/vnd.ipld.car content type order parameter: %q", order) + } + } else { + // when order is not specified, we use DFS as the implicit default + // as this has always been the default behavior and we should not break + // legacy clients + params.Order = DagOrderDFS + } + + // optional dups from IPIP-412 + if dups := NewDuplicateBlocksPolicy(contentTypeParams["dups"]); dups != DuplicateBlocksUnspecified { + switch dups { + case DuplicateBlocksExcluded, DuplicateBlocksIncluded: + params.Duplicates = dups + default: + return CarParams{}, fmt.Errorf("unsupported application/vnd.ipld.car content type dups parameter: %q", dups) + } + } else { + // when duplicate block preference is not specified, we set it to + // false, as this has always been the default behavior, we should + // not break legacy clients, and responses to requests made via ?format=car + // should benefit from block deduplication + params.Duplicates = DuplicateBlocksExcluded + } - return ¶ms, nil + return params, nil } -func getContentTypeFromCarParams(params *CarParams) string { +// buildContentTypeFromCarParams returns a string for Content-Type header. +// It does not change any values, CarParams are respected as-is. +func buildContentTypeFromCarParams(params CarParams) string { h := strings.Builder{} h.WriteString(carResponseFormat) - h.WriteString("; version=1; order=") + h.WriteString("; version=1") - if params.Order != "" { + if params.Order != DagOrderUnspecified { + h.WriteString("; order=") h.WriteString(string(params.Order)) - } else { - h.WriteString(string(DagOrderUnknown)) } - if params.Duplicates != nil { + if params.Duplicates != DuplicateBlocksUnspecified { h.WriteString("; dups=") - if *params.Duplicates { - h.WriteString("y") - } else { - h.WriteString("n") - } + h.WriteString(params.Duplicates.String()) } return h.String() @@ -209,17 +225,28 @@ func getCarRootCidAndLastSegment(imPath ImmutablePath) (cid.Cid, string, error) return rootCid, lastSegment, err } -func getCarEtag(imPath ImmutablePath, params *CarParams, rootCid cid.Cid) string { +func getCarEtag(imPath ImmutablePath, params CarParams, rootCid cid.Cid) string { data := imPath.String() if params.Scope != DagScopeAll { - data += "." + string(params.Scope) + data += string(params.Scope) + } + + // 'order' from IPIP-412 impact Etag only if set to something else + // than DFS (which is the implicit default) + if params.Order != DagOrderDFS { + data += string(params.Order) + } + + // 'dups' from IPIP-412 impact Etag only if 'y' + if dups := params.Duplicates.String(); dups == "y" { + data += dups } if params.Range != nil { if params.Range.From != 0 || params.Range.To != nil { - data += "." + strconv.FormatInt(params.Range.From, 10) + data += strconv.FormatInt(params.Range.From, 10) if params.Range.To != nil { - data += "." + strconv.FormatInt(*params.Range.To, 10) + data += strconv.FormatInt(*params.Range.To, 10) } } } diff --git a/gateway/handler_car_test.go b/gateway/handler_car_test.go index ad3ced51b..65777453d 100644 --- a/gateway/handler_car_test.go +++ b/gateway/handler_car_test.go @@ -28,7 +28,7 @@ func TestCarParams(t *testing.T) { } for _, test := range tests { r := mustNewRequest(t, http.MethodGet, "http://example.com/?"+test.query, nil) - params, err := getCarParams(r, map[string]string{}) + params, err := buildCarParams(r, map[string]string{}) if test.expectedError { assert.Error(t, err) } else { @@ -60,7 +60,7 @@ func TestCarParams(t *testing.T) { } for _, test := range tests { r := mustNewRequest(t, http.MethodGet, "http://example.com/?"+test.query, nil) - params, err := getCarParams(r, map[string]string{}) + params, err := buildCarParams(r, map[string]string{}) if test.hasError { assert.Error(t, err) } else { @@ -74,23 +74,23 @@ func TestCarParams(t *testing.T) { } }) - t.Run("order and duplicates parsing", func(t *testing.T) { + t.Run("buildCarParams from Accept header: order and dups parsing", func(t *testing.T) { t.Parallel() - T := true - F := false - + // below ensure the implicit default (DFS and no duplicates) is correctly inferred + // from the value read from Accept header tests := []struct { acceptHeader string expectedOrder DagOrder - expectedDuplicates *bool + expectedDuplicates DuplicateBlocksPolicy }{ - {"application/vnd.ipld.car; order=dfs; dups=y", DagOrderDFS, &T}, - {"application/vnd.ipld.car; order=unk; dups=n", DagOrderUnknown, &F}, - {"application/vnd.ipld.car; order=unk", DagOrderUnknown, nil}, - {"application/vnd.ipld.car; dups=y", DagOrderUnknown, &T}, - {"application/vnd.ipld.car; dups=n", DagOrderUnknown, &F}, - {"application/vnd.ipld.car", DagOrderUnknown, nil}, + {"application/vnd.ipld.car; order=dfs; dups=y", DagOrderDFS, DuplicateBlocksIncluded}, + {"application/vnd.ipld.car; order=unk; dups=n", DagOrderUnknown, DuplicateBlocksExcluded}, + {"application/vnd.ipld.car; order=unk", DagOrderUnknown, DuplicateBlocksExcluded}, + {"application/vnd.ipld.car; dups=y", DagOrderDFS, DuplicateBlocksIncluded}, + {"application/vnd.ipld.car; dups=n", DagOrderDFS, DuplicateBlocksExcluded}, + {"application/vnd.ipld.car", DagOrderDFS, DuplicateBlocksExcluded}, + {"application/vnd.ipld.car;version=1;order=dfs;dups=y", DagOrderDFS, DuplicateBlocksIncluded}, } for _, test := range tests { r := mustNewRequest(t, http.MethodGet, "http://example.com/", nil) @@ -100,15 +100,14 @@ func TestCarParams(t *testing.T) { assert.NoError(t, err) assert.Equal(t, carResponseFormat, mediaType) - params, err := getCarParams(r, formatParams) + params, err := buildCarParams(r, formatParams) assert.NoError(t, err) + + // order from IPIP-412 require.Equal(t, test.expectedOrder, params.Order) - if test.expectedDuplicates == nil { - require.Nil(t, params.Duplicates) - } else { - require.Equal(t, *test.expectedDuplicates, *params.Duplicates) - } + // dups from IPIP-412 + require.Equal(t, test.expectedDuplicates.String(), params.Duplicates.String()) } }) } @@ -116,22 +115,24 @@ func TestCarParams(t *testing.T) { func TestContentTypeFromCarParams(t *testing.T) { t.Parallel() - T := true - F := false - + // below ensures buildContentTypeFromCarParams produces correct Content-Type + // at this point we do not do any inferring, it happens in buildCarParams instead + // and tests of *Unspecified here are just present for completenes and to guard + // against regressions between refactors tests := []struct { params CarParams header string }{ - {CarParams{}, "application/vnd.ipld.car; version=1; order=unk"}, - {CarParams{Order: DagOrderDFS, Duplicates: &T}, "application/vnd.ipld.car; version=1; order=dfs; dups=y"}, - {CarParams{Order: DagOrderUnknown, Duplicates: &T}, "application/vnd.ipld.car; version=1; order=unk; dups=y"}, + {CarParams{}, "application/vnd.ipld.car; version=1"}, + {CarParams{Order: DagOrderUnspecified, Duplicates: DuplicateBlocksUnspecified}, "application/vnd.ipld.car; version=1"}, + {CarParams{Order: DagOrderDFS, Duplicates: DuplicateBlocksIncluded}, "application/vnd.ipld.car; version=1; order=dfs; dups=y"}, + {CarParams{Order: DagOrderUnknown, Duplicates: DuplicateBlocksIncluded}, "application/vnd.ipld.car; version=1; order=unk; dups=y"}, {CarParams{Order: DagOrderUnknown}, "application/vnd.ipld.car; version=1; order=unk"}, - {CarParams{Duplicates: &T}, "application/vnd.ipld.car; version=1; order=unk; dups=y"}, - {CarParams{Duplicates: &F}, "application/vnd.ipld.car; version=1; order=unk; dups=n"}, + {CarParams{Duplicates: DuplicateBlocksIncluded}, "application/vnd.ipld.car; version=1; dups=y"}, + {CarParams{Duplicates: DuplicateBlocksExcluded}, "application/vnd.ipld.car; version=1; dups=n"}, } for _, test := range tests { - header := getContentTypeFromCarParams(&test.params) + header := buildContentTypeFromCarParams(test.params) assert.Equal(t, test.header, header) } } @@ -148,24 +149,24 @@ func TestGetCarEtag(t *testing.T) { t.Run("Etag with entity-bytes=0:* is the same as without query param", func(t *testing.T) { t.Parallel() - noRange := getCarEtag(imPath, &CarParams{}, cid) - withRange := getCarEtag(imPath, &CarParams{Range: &DagByteRange{From: 0}}, cid) + noRange := getCarEtag(imPath, CarParams{}, cid) + withRange := getCarEtag(imPath, CarParams{Range: &DagByteRange{From: 0}}, cid) require.Equal(t, noRange, withRange) }) t.Run("Etag with entity-bytes=1:* is different than without query param", func(t *testing.T) { t.Parallel() - noRange := getCarEtag(imPath, &CarParams{}, cid) - withRange := getCarEtag(imPath, &CarParams{Range: &DagByteRange{From: 1}}, cid) + noRange := getCarEtag(imPath, CarParams{}, cid) + withRange := getCarEtag(imPath, CarParams{Range: &DagByteRange{From: 1}}, cid) require.NotEqual(t, noRange, withRange) }) t.Run("Etags with different dag-scope are different", func(t *testing.T) { t.Parallel() - a := getCarEtag(imPath, &CarParams{Scope: DagScopeAll}, cid) - b := getCarEtag(imPath, &CarParams{Scope: DagScopeEntity}, cid) + a := getCarEtag(imPath, CarParams{Scope: DagScopeAll}, cid) + b := getCarEtag(imPath, CarParams{Scope: DagScopeEntity}, cid) require.NotEqual(t, a, b) }) } diff --git a/gateway/metrics.go b/gateway/metrics.go index 371fc7864..69e81425f 100644 --- a/gateway/metrics.go +++ b/gateway/metrics.go @@ -120,7 +120,7 @@ func (b *ipfsBackendWithMetrics) ResolvePath(ctx context.Context, path Immutable return md, err } -func (b *ipfsBackendWithMetrics) GetCAR(ctx context.Context, path ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (b *ipfsBackendWithMetrics) GetCAR(ctx context.Context, path ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { begin := time.Now() name := "IPFSBackend.GetCAR" ctx, span := spanTrace(ctx, name, trace.WithAttributes(attribute.String("path", path.String()))) diff --git a/gateway/utilities_test.go b/gateway/utilities_test.go index 81aa68ddb..27ba43a14 100644 --- a/gateway/utilities_test.go +++ b/gateway/utilities_test.go @@ -149,7 +149,7 @@ func (mb *mockBackend) Head(ctx context.Context, immutablePath ImmutablePath) (C return mb.gw.Head(ctx, immutablePath) } -func (mb *mockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params *CarParams) (ContentPathMetadata, io.ReadCloser, error) { +func (mb *mockBackend) GetCAR(ctx context.Context, immutablePath ImmutablePath, params CarParams) (ContentPathMetadata, io.ReadCloser, error) { return mb.gw.GetCAR(ctx, immutablePath, params) }