From e3970d448c8733f8a32722c80b5fee3c69bc58ad Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Tue, 18 Mar 2025 11:31:20 -0500 Subject: [PATCH 1/2] fix: preserve query parameters --- gateway/handler_unixfs_redirects.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gateway/handler_unixfs_redirects.go b/gateway/handler_unixfs_redirects.go index da7273984..fdf173d5b 100644 --- a/gateway/handler_unixfs_redirects.go +++ b/gateway/handler_unixfs_redirects.go @@ -171,7 +171,12 @@ 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 original request + if r.URL.RawQuery != "" { + redirectURL = redirectURL + "?" + r.URL.RawQuery + } + http.Redirect(w, r, redirectURL, rule.Status) return true, "", nil } } From 2d9d254f87605987d699e62699b5d040b97660a4 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 19 Mar 2025 15:54:40 +0100 Subject: [PATCH 2/2] test: static/dynamic/named/splat query params adds missing test + ensures any params from _redirects are preserved this follows what is already supported by Cloudflare Pages implementation: https://developers.cloudflare.com/pages/configuration/redirects/#per-file --- CHANGELOG.md | 2 + gateway/gateway_test.go | 135 ++++++++++++++++++++ gateway/handler_unixfs_redirects.go | 19 ++- gateway/testdata/redirects-query-params.car | Bin 0 -> 749 bytes 4 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 gateway/testdata/redirects-query-params.car 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 fdf173d5b..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" @@ -172,9 +173,23 @@ func (i *handler) handleRedirectsFileRules(w http.ResponseWriter, r *http.Reques // Or redirect if rule.Status >= 301 && rule.Status <= 308 { redirectURL := rule.To - // preserve query parameters from original request + // Preserve query parameters from the original request if they exist if r.URL.RawQuery != "" { - redirectURL = redirectURL + "?" + 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 0000000000000000000000000000000000000000..2be6f62ce98412c33ebe988ae29557e9fc31e5ce GIT binary patch literal 749 zcma))KZp}S6vka6a;%E5HmT%^8Z=xo`QuqE1REtn4g?{boeaA#*~QtN&CE;gY=qMb zoP{WgirRQ89M=ffh=_%9wT0q=+6Wff*^7;{xrB?Nc-_qVzTx}LH{(7+B_-+k5BYP4 zw^@k;3!lEOuJ?~UUCbQV@#W!qRM;zR^5GZ<$f`9Y3^D6_{_q|)lTlm%+=+|k1v~!eO{p-D)dNa*D|AHE2nRq zVSDQjuFu~M8>MVxPx0;RH~Iaqe)iwpTe|tA?(8D2+DL^~C@{O*-acV{%WNg@AdKRG zBs9-X!)!7u#_30FKqGMBcQmR;pxndO0v#h%#k7Y6=vZMf@kQO=Dk zl`*9P)l%8E{{j)9LX(-IU^~K?G;lNVWNdmT^i$e@fmN96NgtUT8{*#)(|K*6=2Ee? zVHrYroV=jGh1l%m2%#p>f$Rz&B7OmlWMa*1P}83TlfYv?g=~c?GFF|)Setv86{=oJ z9bg1uY$WQc*bJtnNj2XH0^~lbqoZ=Grrdxi(Jo^+$D>4GF-98pU1hFL1gAlLrSzTu P_dx;_Lc*+6!nc0`;)EFK literal 0 HcmV?d00001