diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index a5a3a504b..c9bc061c9 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -606,13 +606,15 @@ func (q *queryOps) do(r *http.Request, repl *caddy.Replacer) { continue } - for fieldName, vals := range query { - for i := range vals { - if replaceParam.re != nil { - query[fieldName][i] = replaceParam.re.ReplaceAllString(query[fieldName][i], replace) - } else { - query[fieldName][i] = strings.ReplaceAll(query[fieldName][i], search, replace) - } + vals, ok := query[key] + if !ok { + continue + } + for i := range vals { + if replaceParam.re != nil { + query[key][i] = replaceParam.re.ReplaceAllString(query[key][i], replace) + } else { + query[key][i] = strings.ReplaceAll(query[key][i], search, replace) } } } diff --git a/modules/caddyhttp/rewrite/rewrite_test.go b/modules/caddyhttp/rewrite/rewrite_test.go index 923b164a3..1d23aa241 100644 --- a/modules/caddyhttp/rewrite/rewrite_test.go +++ b/modules/caddyhttp/rewrite/rewrite_test.go @@ -483,6 +483,66 @@ func TestQueryOpsRenameNoOpCases(t *testing.T) { } } +func TestQueryOpsReplaceScopedToKey(t *testing.T) { + repl := caddy.NewReplacer() + + for i, tc := range []struct { + input *http.Request + expect map[string][]string + ops *queryOps + }{ + { + // a keyed replace must only touch the named key, even when + // other keys hold the same value + ops: &queryOps{ + Replace: []*queryOpsReplacement{{Key: "a", Search: "foo", Replace: "bar"}}, + }, + input: newRequest(t, "GET", "/?a=foo&b=foo"), + expect: map[string][]string{"a": {"bar"}, "b": {"foo"}}, + }, + { + // "*" still replaces across every key + ops: &queryOps{ + Replace: []*queryOpsReplacement{{Key: "*", Search: "foo", Replace: "bar"}}, + }, + input: newRequest(t, "GET", "/?a=foo&b=foo"), + expect: map[string][]string{"a": {"bar"}, "b": {"bar"}}, + }, + { + // a keyed replace against a missing key is a no-op + ops: &queryOps{ + Replace: []*queryOpsReplacement{{Key: "missing", Search: "foo", Replace: "bar"}}, + }, + input: newRequest(t, "GET", "/?a=foo&b=foo"), + expect: map[string][]string{"a": {"foo"}, "b": {"foo"}}, + }, + { + // the regexp branch must also stay scoped to the named key + ops: &queryOps{ + Replace: []*queryOpsReplacement{{Key: "a", SearchRegexp: "f.o", Replace: "bar"}}, + }, + input: newRequest(t, "GET", "/?a=foo&b=foo"), + expect: map[string][]string{"a": {"bar"}, "b": {"foo"}}, + }, + } { + repl.Set("http.request.uri", tc.input.RequestURI) + repl.Set("http.request.uri.path", tc.input.URL.Path) + repl.Set("http.request.uri.query", tc.input.URL.RawQuery) + + for _, rep := range tc.ops.Replace { + if err := rep.Provision(caddy.Context{}); err != nil { + t.Fatal(err) + } + } + + tc.ops.do(tc.input, repl) + + if actual := tc.input.URL.Query(); !reflect.DeepEqual(tc.expect, map[string][]string(actual)) { + t.Errorf("Test %d: Expected query=%v but got %v", i, tc.expect, actual) + } + } +} + func newRequest(t *testing.T, method, uri string) *http.Request { req, err := http.NewRequest(method, uri, nil) if err != nil {