Skip to content

fix(gw): duplicate blocks and range etags #486

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.

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

Merged
merged 4 commits into from
Oct 31, 2023
Merged
Changes from all commits
Commits
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
12 changes: 7 additions & 5 deletions gateway/gateway.go
Original file line number Diff line number Diff line change
@@ -174,7 +174,7 @@ const (
)

// DuplicateBlocksPolicy represents the content type parameter 'dups' (IPIP-412)
type DuplicateBlocksPolicy int
type DuplicateBlocksPolicy uint8

const (
DuplicateBlocksUnspecified DuplicateBlocksPolicy = iota // 0 - implicit default
@@ -183,14 +183,16 @@ const (
)

// NewDuplicateBlocksPolicy returns DuplicateBlocksPolicy based on the content type parameter 'dups' (IPIP-412)
func NewDuplicateBlocksPolicy(dupsValue string) DuplicateBlocksPolicy {
func NewDuplicateBlocksPolicy(dupsValue string) (DuplicateBlocksPolicy, error) {
switch dupsValue {
case "y":
return DuplicateBlocksIncluded
return DuplicateBlocksIncluded, nil
case "n":
return DuplicateBlocksExcluded
return DuplicateBlocksExcluded, nil
case "":
return DuplicateBlocksUnspecified, nil
}
return DuplicateBlocksUnspecified
return 0, fmt.Errorf("unsupported application/vnd.ipld.car content type dups parameter: %q", dupsValue)
}

func (d DuplicateBlocksPolicy) Bool() bool {
45 changes: 28 additions & 17 deletions gateway/handler_car.go
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ package gateway

import (
"context"
"encoding/binary"
"fmt"
"io"
"net/http"
@@ -166,20 +167,18 @@ func buildCarParams(r *http.Request, contentTypeParams map[string]string) (CarPa
}

// 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 {
dups, err := NewDuplicateBlocksPolicy(contentTypeParams["dups"])
if err != nil {
return CarParams{}, err
}
if dups == DuplicateBlocksUnspecified {
// 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
dups = DuplicateBlocksExcluded
}
params.Duplicates = dups

return params, nil
}
@@ -226,31 +225,43 @@ func getCarRootCidAndLastSegment(imPath path.ImmutablePath) (cid.Cid, string, er
}

func getCarEtag(imPath path.ImmutablePath, params CarParams, rootCid cid.Cid) string {
data := imPath.String()
h := xxhash.New()
h.WriteString(imPath.String())
// be careful with hashes here, we need boundaries and per entry salt, we don't want a request that has:
// - scope = dfs
// and:
// - order = dfs
// to result in the same hash because if we just do hash(scope + order) they would both yield hash("dfs").
if params.Scope != DagScopeAll {
data += string(params.Scope)
h.WriteString("\x00scope=")
h.WriteString(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)
h.WriteString("\x00order=")
h.WriteString(string(params.Order))
}

// 'dups' from IPIP-412 impact Etag only if 'y'
if dups := params.Duplicates.String(); dups == "y" {
data += dups
if dups := params.Duplicates; dups == DuplicateBlocksIncluded {
h.WriteString("\x00dups=y")
}

if params.Range != nil {
if params.Range.From != 0 || params.Range.To != nil {
data += strconv.FormatInt(params.Range.From, 10)
h.WriteString("\x00range=")
var b [8]byte
binary.LittleEndian.PutUint64(b[:], uint64(params.Range.From))
h.Write(b[:])
if params.Range.To != nil {
data += strconv.FormatInt(*params.Range.To, 10)
binary.LittleEndian.PutUint64(b[:], uint64(*params.Range.To))
h.Write(b[:])
}
}
}

suffix := strconv.FormatUint(xxhash.Sum64([]byte(data)), 32)
suffix := strconv.FormatUint(h.Sum64(), 32)
return `W/"` + rootCid.String() + ".car." + suffix + `"`
}
24 changes: 24 additions & 0 deletions gateway/handler_car_test.go
Original file line number Diff line number Diff line change
@@ -110,6 +110,30 @@ func TestCarParams(t *testing.T) {
require.Equal(t, test.expectedDuplicates.String(), params.Duplicates.String())
}
})

t.Run("buildCarParams from Accept header: order and dups parsing (invalid)", func(t *testing.T) {
t.Parallel()

// below ensure the implicit default (DFS and no duplicates) is correctly inferred
// from the value read from Accept header
tests := []string{
"application/vnd.ipld.car; dups=invalid",
"application/vnd.ipld.car; order=invalid",
"application/vnd.ipld.car; order=dfs; dups=invalid",
"application/vnd.ipld.car; order=invalid; dups=y",
}
for _, test := range tests {
r := mustNewRequest(t, http.MethodGet, "http://example.com/", nil)
r.Header.Set("Accept", test)

mediaType, formatParams, err := customResponseFormat(r)
assert.NoError(t, err)
assert.Equal(t, carResponseFormat, mediaType)

_, err = buildCarParams(r, formatParams)
assert.ErrorContains(t, err, "unsupported application/vnd.ipld.car content type")
}
})
}

func TestContentTypeFromCarParams(t *testing.T) {