Skip to content

Commit a3566b6

Browse files
committed
feat(gateway): IPIP-523 format query param takes precedence over Accept header
this change simplifies precedence rules by making the ?format= URL query parameter always take priority over the Accept HTTP header when both are present. in practice, this is largely compatible with existing browser use cases since browsers send Accept headers with wildcards which were already treated as non-specific. prioritizing ?format= also ensures deterministic HTTP caching behavior, protecting against CDNs that comingle different response types under the same cache key. the only breaking change is for edge cases where a client sends both a specific Accept header and a different ?format= value. previously Accept would win, now ?format= wins. this scenario is rare and arguably represents client misconfiguration. when detected, gateway returns HTTP 400 to signal the ambiguity. specs: ipfs/specs#523 tests: ipfs/gateway-conformance#252
1 parent 23312e7 commit a3566b6

File tree

4 files changed

+93
-26
lines changed

4 files changed

+93
-26
lines changed

gateway/gateway_test.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,29 @@ func TestHeaders(t *testing.T) {
574574
runTest("DNSLink gateway with ?format="+formatParam, "/empty-dir/?format="+formatParam, "", dnslinkGatewayHost, "")
575575
}
576576

577-
runTest("Accept: application/vnd.ipld.car overrides ?format=raw in Content-Location", contentPath+"?format=raw", "application/vnd.ipld.car", "", contentPath+"?format=car")
577+
// IPIP-523: Test that matching ?format and Accept work together (Accept params are used)
578+
runTest("Matching ?format=car and Accept: application/vnd.ipld.car;version=1;order=dfs;dups=n", contentPath+"?format=car", "application/vnd.ipld.car;version=1;order=dfs;dups=n", "", "")
579+
580+
// IPIP-523: Test that conflicting ?format and Accept returns error
581+
t.Run("Conflicting ?format and Accept returns error", func(t *testing.T) {
582+
t.Parallel()
583+
req := mustNewRequest(t, http.MethodGet, ts.URL+contentPath+"?format=raw", nil)
584+
req.Header.Set("Accept", "application/vnd.ipld.car")
585+
resp := mustDoWithoutRedirect(t, req)
586+
defer resp.Body.Close()
587+
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
588+
})
589+
590+
// IPIP-523: Browser Accept header with wildcards should not interfere with ?format
591+
t.Run("Browser Accept header does not interfere with ?format=raw", func(t *testing.T) {
592+
t.Parallel()
593+
req := mustNewRequest(t, http.MethodGet, ts.URL+contentPath+"?format=raw", nil)
594+
req.Header.Set("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8")
595+
resp := mustDoWithoutRedirect(t, req)
596+
defer resp.Body.Close()
597+
require.Equal(t, http.StatusOK, resp.StatusCode)
598+
require.Equal(t, rawResponseFormat, resp.Header.Get("Content-Type"))
599+
})
578600
})
579601
}
580602

gateway/handler.go

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -665,15 +665,36 @@ func init() {
665665
}
666666
}
667667

668-
// return explicit response format if specified in request as query parameter or via Accept HTTP header
668+
// customResponseFormat determines the response format and extracts any parameters
669+
// from the ?format= query parameter and Accept HTTP header.
670+
//
671+
// This function is format-agnostic: it handles generic HTTP content negotiation
672+
// and returns parameters embedded in the Accept header (e.g., "application/vnd.ipld.car;order=dfs").
673+
//
674+
// Format-specific URL query parameters (e.g., ?car-order=, ?car-dups= for CAR)
675+
// are intentionally NOT handled here. They are processed by format-specific
676+
// handlers which merge Accept header params with URL params, giving URL params
677+
// precedence per IPIP-523. See buildCarParams() for the CAR example. This
678+
// pattern can be extended for other formats that need URL-based parameters.
669679
func customResponseFormat(r *http.Request) (mediaType string, params map[string]string, err error) {
670-
// First, inspect Accept header, as it may not only include content type, but also optional parameters.
680+
// Check ?format= URL query parameter first (IPIP-523).
681+
formatParam := r.URL.Query().Get("format")
682+
var formatMediaType string
683+
if formatParam != "" {
684+
if responseFormat, ok := formatParamToResponseFormat[formatParam]; ok {
685+
formatMediaType = responseFormat
686+
}
687+
}
688+
689+
// Inspect Accept header for vendor-specific content types and optional parameters
671690
// such as CAR version or additional ones from IPIP-412.
672691
//
673692
// Browsers and other user agents will send Accept header with generic types like:
674693
// Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8
675694
// We only care about explicit, vendor-specific content-types and respond to the first match (in order).
676695
// TODO: make this RFC compliant and respect weights (eg. return CAR for Accept:application/vnd.ipld.dag-json;q=0.1,application/vnd.ipld.car;q=0.2)
696+
var acceptMediaType string
697+
var acceptParams map[string]string
677698
for _, header := range r.Header.Values("Accept") {
678699
for _, value := range strings.Split(header, ",") {
679700
accept := strings.TrimSpace(value)
@@ -687,16 +708,32 @@ func customResponseFormat(r *http.Request) (mediaType string, params map[string]
687708
if err != nil {
688709
return "", nil, err
689710
}
690-
return mediatype, params, nil
711+
acceptMediaType = mediatype
712+
acceptParams = params
713+
break
691714
}
692715
}
716+
if acceptMediaType != "" {
717+
break
718+
}
693719
}
694720

695-
// If no Accept header, translate query param to a content type, if present.
696-
if formatParam := r.URL.Query().Get("format"); formatParam != "" {
697-
if responseFormat, ok := formatParamToResponseFormat[formatParam]; ok {
698-
return responseFormat, nil, nil
721+
// If both ?format and Accept are present with conflicting values, return error (IPIP-523).
722+
if formatMediaType != "" && acceptMediaType != "" && formatMediaType != acceptMediaType {
723+
return "", nil, fmt.Errorf("ambiguous request: ?format=%q (%s) vs Accept: %q", formatParam, formatMediaType, acceptMediaType)
724+
}
725+
726+
// ?format takes precedence (IPIP-523), but use Accept params if available for the same type.
727+
if formatMediaType != "" {
728+
if acceptMediaType == formatMediaType {
729+
return formatMediaType, acceptParams, nil
699730
}
731+
return formatMediaType, nil, nil
732+
}
733+
734+
// Fall back to Accept header if no ?format query param.
735+
if acceptMediaType != "" {
736+
return acceptMediaType, acceptParams, nil
700737
}
701738

702739
// If none of special-cased content types is found, return empty string
@@ -716,10 +753,9 @@ func addContentLocation(r *http.Request, w http.ResponseWriter, rq *requestData)
716753

717754
format := responseFormatToFormatParam[rq.responseFormat]
718755

719-
// Skip Content-Location if there is no conflict between
720-
// 'format' in URL and value in 'Accept' header.
721-
// If both are present and don't match, we continue and generate
722-
// Content-Location to ensure value from Accept overrides 'format' from URL.
756+
// Skip Content-Location if ?format is already present in URL and matches
757+
// the response format. Content-Location is only needed when format was
758+
// requested via Accept header without ?format in URL.
723759
if urlFormat := r.URL.Query().Get("format"); urlFormat != "" && urlFormat == format {
724760
return
725761
}
@@ -736,9 +772,13 @@ func addContentLocation(r *http.Request, w http.ResponseWriter, rq *requestData)
736772
}
737773
query.Set("format", format)
738774

739-
// Set response params as query elements.
775+
// Set response params as query elements, but only if URL doesn't already
776+
// have them (URL query params take precedence per IPIP-523).
740777
for k, v := range rq.responseParams {
741-
query.Set(format+"-"+k, v)
778+
paramKey := format + "-" + k
779+
if !query.Has(paramKey) {
780+
query.Set(paramKey, v)
781+
}
742782
}
743783

744784
w.Header().Set("Content-Location", path+"?"+query.Encode())

gateway/handler_car.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,20 +142,20 @@ func buildCarParams(r *http.Request, contentTypeParams map[string]string) (CarPa
142142
params.Scope = DagScopeAll
143143
}
144144

145-
// application/vnd.ipld.car content type parameters from Accept header
146-
147-
// Get CAR version, duplicates and order from the query parameters and override
148-
// with parameters from Accept header if they exist, since they have priority.
149-
versionStr := queryParams.Get(carVersionKey)
150-
duplicatesStr := queryParams.Get(carDuplicatesKey)
151-
orderStr := queryParams.Get(carOrderKey)
152-
if v, ok := contentTypeParams["version"]; ok {
145+
// application/vnd.ipld.car content type parameters from Accept header and URL query
146+
147+
// Get CAR version, duplicates and order from Accept header first,
148+
// then override with URL query parameters if they exist (IPIP-523).
149+
versionStr := contentTypeParams["version"]
150+
duplicatesStr := contentTypeParams["dups"]
151+
orderStr := contentTypeParams["order"]
152+
if v := queryParams.Get(carVersionKey); v != "" {
153153
versionStr = v
154154
}
155-
if v, ok := contentTypeParams["order"]; ok {
155+
if v := queryParams.Get(carOrderKey); v != "" {
156156
orderStr = v
157157
}
158-
if v, ok := contentTypeParams["dups"]; ok {
158+
if v := queryParams.Get(carDuplicatesKey); v != "" {
159159
duplicatesStr = v
160160
}
161161

@@ -164,7 +164,7 @@ func buildCarParams(r *http.Request, contentTypeParams map[string]string) (CarPa
164164
case "": // noop, client does not care about version
165165
case "1": // noop, we support this
166166
default:
167-
return CarParams{}, errors.New("unsupported application/vnd.ipld.car version: only version=1 is supported")
167+
return CarParams{}, fmt.Errorf("unsupported CAR version %q: only version=1 is supported", versionStr)
168168
}
169169

170170
// optional order from IPIP-412

gateway/handler_car_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,13 @@ func TestCarParams(t *testing.T) {
9494
{"application/vnd.ipld.car; dups=n", nil, DagOrderDFS, DuplicateBlocksExcluded},
9595
{"application/vnd.ipld.car", nil, DagOrderDFS, DuplicateBlocksExcluded},
9696
{"application/vnd.ipld.car;version=1;order=dfs;dups=y", nil, DagOrderDFS, DuplicateBlocksIncluded},
97-
{"application/vnd.ipld.car;version=1;order=dfs;dups=y", url.Values{"car-order": []string{"unk"}}, DagOrderDFS, DuplicateBlocksIncluded},
97+
// IPIP-523: URL query params take priority over Accept header params
98+
{"application/vnd.ipld.car;version=1;order=dfs;dups=y", url.Values{"car-order": []string{"unk"}}, DagOrderUnknown, DuplicateBlocksIncluded},
9899
{"application/vnd.ipld.car;version=1;dups=y", url.Values{"car-order": []string{"unk"}}, DagOrderUnknown, DuplicateBlocksIncluded},
100+
// IPIP-523: URL params work without Accept header (non-default dups to detect wiring bugs)
101+
{"", url.Values{"format": []string{"car"}, "car-dups": []string{"y"}}, DagOrderDFS, DuplicateBlocksIncluded},
102+
// IPIP-523: URL dups=y overrides Accept dups=n
103+
{"application/vnd.ipld.car;order=dfs;dups=n", url.Values{"car-dups": []string{"y"}}, DagOrderDFS, DuplicateBlocksIncluded},
99104
}
100105
for _, test := range tests {
101106
r := mustNewRequest(t, http.MethodGet, "http://example.com/?"+test.params.Encode(), nil)

0 commit comments

Comments
 (0)