diff --git a/.github/workflows/gateway-conformance.yml b/.github/workflows/gateway-conformance.yml index 47bb861e7..e1b57c180 100644 --- a/.github/workflows/gateway-conformance.yml +++ b/.github/workflows/gateway-conformance.yml @@ -22,7 +22,7 @@ jobs: steps: # 1. Download the gateway-conformance fixtures - name: Download gateway-conformance fixtures - uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.8 + uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.9 with: output: fixtures merged: true @@ -47,7 +47,7 @@ jobs: # 4. Run the gateway-conformance tests - name: Run gateway-conformance tests without IPNS and DNSLink - uses: ipfs/gateway-conformance/.github/actions/test@v0.8 + uses: ipfs/gateway-conformance/.github/actions/test@v0.9 with: gateway-url: http://127.0.0.1:8040 subdomain-url: http://example.net:8040 @@ -84,7 +84,7 @@ jobs: steps: # 1. Download the gateway-conformance fixtures - name: Download gateway-conformance fixtures - uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.8 + uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.9 with: output: fixtures merged: true @@ -114,7 +114,7 @@ jobs: # 4. Run the gateway-conformance tests - name: Run gateway-conformance tests without IPNS and DNSLink - uses: ipfs/gateway-conformance/.github/actions/test@v0.8 + uses: ipfs/gateway-conformance/.github/actions/test@v0.9 with: gateway-url: http://127.0.0.1:8040 # we test gateway that is backed by a remote block gateway subdomain-url: http://example.net:8040 @@ -152,7 +152,7 @@ jobs: steps: # 1. Download the gateway-conformance fixtures - name: Download gateway-conformance fixtures - uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.8 + uses: ipfs/gateway-conformance/.github/actions/extract-fixtures@v0.9 with: output: fixtures merged: true @@ -182,7 +182,7 @@ jobs: # 4. Run the gateway-conformance tests - name: Run gateway-conformance tests without IPNS and DNSLink - uses: ipfs/gateway-conformance/.github/actions/test@v0.8 + uses: ipfs/gateway-conformance/.github/actions/test@v0.9 with: gateway-url: http://127.0.0.1:8040 # we test gateway that is backed by a remote car gateway subdomain-url: http://example.net:8040 diff --git a/CHANGELOG.md b/CHANGELOG.md index f4ba9b6f0..a4cb29e5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ The following emojis are used to highlight certain changes: ### Changed +- `gateway`: ✨ [IPIP-523](https://github.com/ipfs/specs/pull/523) `?format=` URL query parameter now takes precedence over `Accept` HTTP header, ensuring deterministic HTTP cache behavior and allowing browsers to use `?format=` even when they send `Accept` headers with specific content types. [#1074](https://github.com/ipfs/boxo/pull/1074) + ### Removed ### Fixed diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 23c5e55b4..123cff6b1 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -575,7 +575,30 @@ func TestHeaders(t *testing.T) { runTest("DNSLink gateway with ?format="+formatParam, "/empty-dir/?format="+formatParam, "", dnslinkGatewayHost, "") } - runTest("Accept: application/vnd.ipld.car overrides ?format=raw in Content-Location", contentPath+"?format=raw", "application/vnd.ipld.car", "", contentPath+"?format=car") + // IPIP-523: Test that matching ?format and Accept work together (Accept params are used) + 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", "", "") + + // IPIP-523: Test that conflicting ?format and Accept uses ?format (URL wins) + t.Run("Conflicting ?format and Accept uses ?format from URL", func(t *testing.T) { + t.Parallel() + req := mustNewRequest(t, http.MethodGet, ts.URL+contentPath+"?format=raw", nil) + req.Header.Set("Accept", "application/vnd.ipld.car") + resp := mustDoWithoutRedirect(t, req) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, rawResponseFormat, resp.Header.Get("Content-Type")) + }) + + // IPIP-523: Browser Accept header with wildcards should not interfere with ?format + t.Run("Browser Accept header does not interfere with ?format=raw", func(t *testing.T) { + t.Parallel() + req := mustNewRequest(t, http.MethodGet, ts.URL+contentPath+"?format=raw", nil) + req.Header.Set("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8") + resp := mustDoWithoutRedirect(t, req) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, rawResponseFormat, resp.Header.Get("Content-Type")) + }) }) } diff --git a/gateway/handler.go b/gateway/handler.go index 6b47faa44..0f199f862 100644 --- a/gateway/handler.go +++ b/gateway/handler.go @@ -666,15 +666,36 @@ func init() { } } -// return explicit response format if specified in request as query parameter or via Accept HTTP header +// customResponseFormat determines the response format and extracts any parameters +// from the ?format= query parameter and Accept HTTP header. +// +// This function is format-agnostic: it handles generic HTTP content negotiation +// and returns parameters embedded in the Accept header (e.g., "application/vnd.ipld.car;order=dfs"). +// +// Format-specific URL query parameters (e.g., ?car-order=, ?car-dups= for CAR) +// are intentionally NOT handled here. They are processed by format-specific +// handlers which merge Accept header params with URL params, giving URL params +// precedence per IPIP-523. See buildCarParams() for the CAR example. This +// pattern can be extended for other formats that need URL-based parameters. func customResponseFormat(r *http.Request) (mediaType string, params map[string]string, err error) { - // First, inspect Accept header, as it may not only include content type, but also optional parameters. + // Check ?format= URL query parameter first (IPIP-523). + formatParam := r.URL.Query().Get("format") + var formatMediaType string + if formatParam != "" { + if responseFormat, ok := formatParamToResponseFormat[formatParam]; ok { + formatMediaType = responseFormat + } + } + + // Inspect Accept header for vendor-specific content types and 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). // 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) + var acceptMediaType string + var acceptParams map[string]string for _, header := range r.Header.Values("Accept") { for _, value := range strings.Split(header, ",") { accept := strings.TrimSpace(value) @@ -688,16 +709,29 @@ func customResponseFormat(r *http.Request) (mediaType string, params map[string] if err != nil { return "", nil, err } - return mediatype, params, nil + acceptMediaType = mediatype + acceptParams = params + break } } + if acceptMediaType != "" { + break + } } - // If no Accept header, translate query param to a content type, if present. - if formatParam := r.URL.Query().Get("format"); formatParam != "" { - if responseFormat, ok := formatParamToResponseFormat[formatParam]; ok { - return responseFormat, nil, nil + // ?format takes precedence (IPIP-523), even when Accept header specifies a different format. + // This ensures deterministic HTTP caching and allows browsers to use ?format reliably. + if formatMediaType != "" { + // Use Accept params only if Accept matches ?format (e.g., for CAR version/order params) + if acceptMediaType == formatMediaType { + return formatMediaType, acceptParams, nil } + return formatMediaType, nil, nil + } + + // Fall back to Accept header if no ?format query param. + if acceptMediaType != "" { + return acceptMediaType, acceptParams, nil } // If none of special-cased content types is found, return empty string @@ -717,10 +751,9 @@ func addContentLocation(r *http.Request, w http.ResponseWriter, rq *requestData) format := responseFormatToFormatParam[rq.responseFormat] - // Skip Content-Location if there is no conflict between - // 'format' in URL and value in 'Accept' header. - // If both are present and don't match, we continue and generate - // Content-Location to ensure value from Accept overrides 'format' from URL. + // Skip Content-Location if ?format is already present in URL and matches + // the response format. Content-Location is only needed when format was + // requested via Accept header without ?format in URL. if urlFormat := r.URL.Query().Get("format"); urlFormat != "" && urlFormat == format { return } @@ -737,9 +770,13 @@ func addContentLocation(r *http.Request, w http.ResponseWriter, rq *requestData) } query.Set("format", format) - // Set response params as query elements. + // Set response params as query elements, but only if URL doesn't already + // have them (URL query params take precedence per IPIP-523). for k, v := range rq.responseParams { - query.Set(format+"-"+k, v) + paramKey := format + "-" + k + if !query.Has(paramKey) { + query.Set(paramKey, v) + } } w.Header().Set("Content-Location", path+"?"+query.Encode()) diff --git a/gateway/handler_car.go b/gateway/handler_car.go index dcf1b00c2..f6e34984c 100644 --- a/gateway/handler_car.go +++ b/gateway/handler_car.go @@ -142,20 +142,20 @@ func buildCarParams(r *http.Request, contentTypeParams map[string]string) (CarPa params.Scope = DagScopeAll } - // application/vnd.ipld.car content type parameters from Accept header - - // Get CAR version, duplicates and order from the query parameters and override - // with parameters from Accept header if they exist, since they have priority. - versionStr := queryParams.Get(carVersionKey) - duplicatesStr := queryParams.Get(carDuplicatesKey) - orderStr := queryParams.Get(carOrderKey) - if v, ok := contentTypeParams["version"]; ok { + // application/vnd.ipld.car content type parameters from Accept header and URL query + + // Get CAR version, duplicates and order from Accept header first, + // then override with URL query parameters if they exist (IPIP-523). + versionStr := contentTypeParams["version"] + duplicatesStr := contentTypeParams["dups"] + orderStr := contentTypeParams["order"] + if v := queryParams.Get(carVersionKey); v != "" { versionStr = v } - if v, ok := contentTypeParams["order"]; ok { + if v := queryParams.Get(carOrderKey); v != "" { orderStr = v } - if v, ok := contentTypeParams["dups"]; ok { + if v := queryParams.Get(carDuplicatesKey); v != "" { duplicatesStr = v } diff --git a/gateway/handler_car_test.go b/gateway/handler_car_test.go index 6f6d25e1f..644c8f2e3 100644 --- a/gateway/handler_car_test.go +++ b/gateway/handler_car_test.go @@ -94,8 +94,13 @@ func TestCarParams(t *testing.T) { {"application/vnd.ipld.car; dups=n", nil, DagOrderDFS, DuplicateBlocksExcluded}, {"application/vnd.ipld.car", nil, DagOrderDFS, DuplicateBlocksExcluded}, {"application/vnd.ipld.car;version=1;order=dfs;dups=y", nil, DagOrderDFS, DuplicateBlocksIncluded}, - {"application/vnd.ipld.car;version=1;order=dfs;dups=y", url.Values{"car-order": []string{"unk"}}, DagOrderDFS, DuplicateBlocksIncluded}, + // IPIP-523: URL query params take priority over Accept header params + {"application/vnd.ipld.car;version=1;order=dfs;dups=y", url.Values{"car-order": []string{"unk"}}, DagOrderUnknown, DuplicateBlocksIncluded}, {"application/vnd.ipld.car;version=1;dups=y", url.Values{"car-order": []string{"unk"}}, DagOrderUnknown, DuplicateBlocksIncluded}, + // IPIP-523: URL params work without Accept header (non-default dups to detect wiring bugs) + {"", url.Values{"format": []string{"car"}, "car-dups": []string{"y"}}, DagOrderDFS, DuplicateBlocksIncluded}, + // IPIP-523: URL dups=y overrides Accept dups=n + {"application/vnd.ipld.car;order=dfs;dups=n", url.Values{"car-dups": []string{"y"}}, DagOrderDFS, DuplicateBlocksIncluded}, } for _, test := range tests { r := mustNewRequest(t, http.MethodGet, "http://example.com/?"+test.params.Encode(), nil)