diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d6595d61..0e8819478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ The following emojis are used to highlight certain changes: ### Fixed +- `gateway`: query parameters are now supported and preserved in redirects triggered by a [`_redirects`](https://specs.ipfs.tech/http-gateways/web-redirects-file/) file [#886](https://github.com/ipfs/boxo/pull/886) + ### Security diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 349d76a0e..fa94d6a33 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -816,6 +816,141 @@ func TestRedirects(t *testing.T) { do(http.MethodHead) }) + t.Run("_redirects file redirect preserves both static and dynamic query parameters", func(t *testing.T) { + t.Parallel() + + backend, root := newMockBackend(t, "redirects-query-params.car") + backend.namesys["/ipns/example.org"] = newMockNamesysItem(path.FromCid(root), 0) + + ts := newTestServerWithConfig(t, backend, Config{ + NoDNSLink: false, + PublicGateways: map[string]*PublicGateway{ + "example.org": { + UseSubdomains: true, + DeserializedResponses: true, + }, + }, + DeserializedResponses: true, + }) + + // Request for missing page with some dynamic parameters + missingPageURL := ts.URL + "/source1/source-file?dynamic-query1=dynamic-value1&dynamic-query2=dynamic-value2" + + // Expected response is redirect URL that preserves both static and dynamic parameters + expectedTargetURL := "/target-file?dynamic-query1=dynamic-value1&dynamic-query2=dynamic-value2&static-query1=static-val1&static-query2=static-val2" + + do := func(method string) { + // Make initial request to non-existing page that should return redirect that contains both query params from original request AND target URL in _redirects file. + req := mustNewRequest(t, method, missingPageURL, nil) + req.Header.Add("Accept", "text/html") + req.Host = "example.org" + + res := mustDoWithoutRedirect(t, req) + defer res.Body.Close() + + // Status code should match 301 from _redirects catch-all rule + require.Equal(t, http.StatusMovedPermanently, res.StatusCode) + + // Check Redirect target contains all query parameters + redirectURL := res.Header.Get("Location") + require.Equal(t, expectedTargetURL, redirectURL) + + } + + do(http.MethodGet) + do(http.MethodHead) + }) + + t.Run("_redirects file redirect supports named placeholders as query parameters", func(t *testing.T) { + t.Parallel() + + backend, root := newMockBackend(t, "redirects-query-params.car") + backend.namesys["/ipns/example.org"] = newMockNamesysItem(path.FromCid(root), 0) + + ts := newTestServerWithConfig(t, backend, Config{ + NoDNSLink: false, + PublicGateways: map[string]*PublicGateway{ + "example.org": { + UseSubdomains: true, + DeserializedResponses: true, + }, + }, + DeserializedResponses: true, + }) + + // Request for missing page under path that has named placeholders and some dynamic parameters + missingPageURL := ts.URL + "/source2/codeA/nameA?dynamic-query1=dynamic-value1&dynamic-query2=dynamic-value2" + + // Expected response is redirect URL that preserves both named and dynamic parameters + expectedTargetURL := "/target-file?code=codeA&dynamic-query1=dynamic-value1&dynamic-query2=dynamic-value2&name=nameA" + + do := func(method string) { + // Make initial request to non-existing page that should return redirect that contains both query params from original request AND target URL in _redirects file. + req := mustNewRequest(t, method, missingPageURL, nil) + req.Header.Add("Accept", "text/html") + req.Host = "example.org" + + res := mustDoWithoutRedirect(t, req) + defer res.Body.Close() + + // Status code should match 301 from _redirects catch-all rule + require.Equal(t, http.StatusMovedPermanently, res.StatusCode) + + // Check Redirect target contains all query parameters + redirectURL := res.Header.Get("Location") + require.Equal(t, expectedTargetURL, redirectURL) + + } + + do(http.MethodGet) + do(http.MethodHead) + }) + + t.Run("_redirects file redirect supports catch-all splat with dynamic query parameters", func(t *testing.T) { + t.Parallel() + + backend, root := newMockBackend(t, "redirects-query-params.car") + backend.namesys["/ipns/example.org"] = newMockNamesysItem(path.FromCid(root), 0) + + ts := newTestServerWithConfig(t, backend, Config{ + NoDNSLink: false, + PublicGateways: map[string]*PublicGateway{ + "example.org": { + UseSubdomains: true, + DeserializedResponses: true, + }, + }, + DeserializedResponses: true, + }) + + // Request for missing page under path that is a catch-all splat (incl. some dynamic parameters) + missingPageURL := ts.URL + "/source3/this/is/catch-all/splat-with?dynamic-query1=dynamic-value1&dynamic-query2=dynamic-value2" + + // Expected response is redirect URL to a different server that preserves entire splat, including dynamic query parameters + expectedTargetURL := "https://example.net/target3/this/is/catch-all/splat-with?dynamic-query1=dynamic-value1&dynamic-query2=dynamic-value2" + + do := func(method string) { + // Make initial request to non-existing page that should return redirect that contains both query params from original request AND target URL in _redirects file. + req := mustNewRequest(t, method, missingPageURL, nil) + req.Header.Add("Accept", "text/html") + req.Host = "example.org" + + res := mustDoWithoutRedirect(t, req) + defer res.Body.Close() + + // Status code should match 301 from _redirects catch-all rule + require.Equal(t, http.StatusMovedPermanently, res.StatusCode) + + // Check Redirect target contains all query parameters + redirectURL := res.Header.Get("Location") + require.Equal(t, expectedTargetURL, redirectURL) + + } + + do(http.MethodGet) + do(http.MethodHead) + }) + t.Run("Superfluous namespace", func(t *testing.T) { t.Parallel() diff --git a/gateway/handler_unixfs_redirects.go b/gateway/handler_unixfs_redirects.go index da7273984..8881044ec 100644 --- a/gateway/handler_unixfs_redirects.go +++ b/gateway/handler_unixfs_redirects.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "net/url" gopath "path" "strconv" "strings" @@ -171,7 +172,26 @@ func (i *handler) handleRedirectsFileRules(w http.ResponseWriter, r *http.Reques // Or redirect if rule.Status >= 301 && rule.Status <= 308 { - http.Redirect(w, r, rule.To, rule.Status) + redirectURL := rule.To + // Preserve query parameters from the original request if they exist + if r.URL.RawQuery != "" { + u, err := url.Parse(rule.To) + if err != nil { + return false, "", err + } + // Merge original query parameters into target + originalQuery, _ := url.ParseQuery(r.URL.RawQuery) + targetQuery := u.Query() + for key, values := range originalQuery { + for _, value := range values { + targetQuery.Add(key, value) + } + } + // Set the merged query parameters back + u.RawQuery = targetQuery.Encode() + redirectURL = u.String() + } + http.Redirect(w, r, redirectURL, rule.Status) return true, "", nil } } diff --git a/gateway/testdata/redirects-query-params.car b/gateway/testdata/redirects-query-params.car new file mode 100644 index 000000000..2be6f62ce Binary files /dev/null and b/gateway/testdata/redirects-query-params.car differ