diff --git a/gateway/gateway.go b/gateway/gateway.go index 06d236d1f..28cf5d295 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -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 { diff --git a/gateway/handler_car.go b/gateway/handler_car.go index 1c670b045..21b90108c 100644 --- a/gateway/handler_car.go +++ b/gateway/handler_car.go @@ -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 + `"` } diff --git a/gateway/handler_car_test.go b/gateway/handler_car_test.go index efbad2b22..da2d16255 100644 --- a/gateway/handler_car_test.go +++ b/gateway/handler_car_test.go @@ -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) {