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

Author: lidel

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
 
 

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()
 

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