Repository: caddyserver/caddy (language: go)

Issue:
Query rewrite operations
Currently, the only way we offer to rewrite the query part of the request URI is with the `rewrite` handler (when it has a `?` in the target), or with `uri replace` to perform replacements on the entire URI.

These options are limiting, because many query rewrite operations are difficult to perform with those.

For example, it's impossible (I can't think of a way) to remove a single query key from a request without knowing all the other possible query keys and re-including them all with `&key={query.key}` in the rewrite.

It's also unwieldy to replace a specific query key with another (one way is with `uri replace` but it's awkward and error-prone).

I think the solution is to add `uri query` which supports multiple different types of operations on queries, similarly to the [`header` directive](https://caddyserver.com/docs/caddyfile/directives/header) which has `+` and `-` and so on.

```
# add an additional `foo=bar` (even if there was already `foo=baz`)
uri query +foo bar

# overwrite `foo` with `foo=bar`
uri query foo bar

# delete `foo`
uri query -foo

# rename key `foo` to `bar`
uri query foo>bar

# set a default value (if the key is not already set)
uri query ?foo bar

# replace values in `foo`, changing `bar` to `baz`
uri query foo bar baz

# do multiple operations at once
uri query {
	foo bar
	bar baz
	-baz
}
```

We may or may not actually implement all these operations, but I think most of them would be very nice to have.

Should this be its own directive instead of being added to `uri`? To be discussed.


Discussion / hints:
I really like this idea. As part of the `uri` directive, yeah.
I'll keep this open actually, until we have the `replace` operation implemented. /cc @armadi1809
For replace, we want it to have all the features the header directive has right?  
I don't quite remember what that entails, so I'll let you make a decision on that, or you can ask specifically what we should include/keep
@armadi1809 Yes, I think that'd be great! Both fast substring replacements and regex replacements. Ideally for all params or just a named one. Feel free to use the headers directive as a starting point.

Fix patch:
diff --git a/modules/caddyhttp/rewrite/caddyfile.go b/modules/caddyhttp/rewrite/caddyfile.go
index 31f7e9b4859..0ce5c41d217 100644
--- a/modules/caddyhttp/rewrite/caddyfile.go
+++ b/modules/caddyhttp/rewrite/caddyfile.go
@@ -213,6 +213,9 @@ func applyQueryOps(h httpcaddyfile.Helper, qo *queryOps, args []string) error {
 		renameValKey := strings.Split(key, ">")
 		qo.Rename = append(qo.Rename, queryOpsArguments{Key: renameValKey[0], Val: renameValKey[1]})
 
+	case len(args) == 3:
+		qo.Replace = append(qo.Replace, &queryOpsReplacement{Key: key, SearchRegexp: args[1], Replace: args[2]})
+
 	default:
 		if len(args) != 2 {
 			return h.ArgErr()
diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go
index 1859f9df2b5..3479f064991 100644
--- a/modules/caddyhttp/rewrite/rewrite.go
+++ b/modules/caddyhttp/rewrite/rewrite.go
@@ -118,6 +118,12 @@ func (rewr *Rewrite) Provision(ctx caddy.Context) error {
 		rep.re = re
 	}
 
+	for _, replacementOp := range rewr.Query.Replace {
+		err := replacementOp.Provision(ctx)
+		if err != nil {
+			return fmt.Errorf("compiling regular expression %s in query rewrite replace operation: %v", replacementOp.SearchRegexp, err)
+		}
+	}
 	return nil
 }
 
@@ -490,13 +496,27 @@ type queryOps struct {
 	// and only appends an additional value for that key if any already exist.
 	Add []queryOpsArguments `json:"add,omitempty"`
 
+	// Replaces query parameters.
+	Replace []*queryOpsReplacement `json:"replace,omitempty"`
+
 	// Deletes a given query key by name.
 	Delete []string `json:"delete,omitempty"`
 }
 
+// Provision compiles the query replace operation regex.
+func (replacement *queryOpsReplacement) Provision(_ caddy.Context) error {
+	if replacement.SearchRegexp != "" {
+		re, err := regexp.Compile(replacement.SearchRegexp)
+		if err != nil {
+			return fmt.Errorf("replacement for query field '%s': %v", replacement.Key, err)
+		}
+		replacement.re = re
+	}
+	return nil
+}
+
 func (q *queryOps) do(r *http.Request, repl *caddy.Replacer) {
 	query := r.URL.Query()
-
 	for _, renameParam := range q.Rename {
 		key := repl.ReplaceAll(renameParam.Key, "")
 		val := repl.ReplaceAll(renameParam.Val, "")
@@ -525,6 +545,36 @@ func (q *queryOps) do(r *http.Request, repl *caddy.Replacer) {
 		query[key] = append(query[key], val)
 	}
 
+	for _, replaceParam := range q.Replace {
+		key := repl.ReplaceAll(replaceParam.Key, "")
+		search := repl.ReplaceKnown(replaceParam.Search, "")
+		replace := repl.ReplaceKnown(replaceParam.Replace, "")
+
+		// replace all query keys...
+		if key == "*" {
+			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)
+					}
+				}
+			}
+			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)
+				}
+			}
+		}
+	}
+
 	for _, deleteParam := range q.Delete {
 		param := repl.ReplaceAll(deleteParam, "")
 		if param == "" {
@@ -546,5 +596,21 @@ type queryOpsArguments struct {
 	Val string `json:"val,omitempty"`
 }
 
+type queryOpsReplacement struct {
+	// The key to replace in the query string.
+	Key string `json:"key,omitempty"`
+
+	// The substring to search for.
+	Search string `json:"search,omitempty"`
+
+	// The regular expression to search with.
+	SearchRegexp string `json:"search_regexp,omitempty"`
+
+	// The string with which to replace matches.
+	Replace string `json:"replace,omitempty"`
+
+	re *regexp.Regexp
+}
+
 // Interface guard
 var _ caddyhttp.MiddlewareHandler = (*Rewrite)(nil)


Test patch:
diff --git a/caddytest/integration/caddyfile_adapt/uri_query_operations.caddyfiletest b/caddytest/integration/caddyfile_adapt/uri_query_operations.caddyfiletest
new file mode 100644
index 00000000000..a53462480ca
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/uri_query_operations.caddyfiletest
@@ -0,0 +1,106 @@
+:9080
+uri query +foo bar
+uri query -baz
+uri query taz test
+uri query key=value example
+uri query changethis>changed
+uri query {
+	findme value replacement
+	+foo1 baz
+}
+
+respond "{query}"
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":9080"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"handler": "rewrite",
+									"query": {
+										"add": [
+											{
+												"key": "foo",
+												"val": "bar"
+											}
+										]
+									}
+								},
+								{
+									"handler": "rewrite",
+									"query": {
+										"delete": [
+											"baz"
+										]
+									}
+								},
+								{
+									"handler": "rewrite",
+									"query": {
+										"set": [
+											{
+												"key": "taz",
+												"val": "test"
+											}
+										]
+									}
+								},
+								{
+									"handler": "rewrite",
+									"query": {
+										"set": [
+											{
+												"key": "key=value",
+												"val": "example"
+											}
+										]
+									}
+								},
+								{
+									"handler": "rewrite",
+									"query": {
+										"rename": [
+											{
+												"key": "changethis",
+												"val": "changed"
+											}
+										]
+									}
+								},
+								{
+									"handler": "rewrite",
+									"query": {
+										"add": [
+											{
+												"key": "foo1",
+												"val": "baz"
+											}
+										],
+										"replace": [
+											{
+												"key": "findme",
+												"replace": "replacement",
+												"search_regexp": "value"
+											}
+										]
+									}
+								},
+								{
+									"body": "{http.request.uri.query}",
+									"handler": "static_response"
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go
index 5d1fa3f0898..628363a5269 100644
--- a/caddytest/integration/caddyfile_test.go
+++ b/caddytest/integration/caddyfile_test.go
@@ -569,6 +569,93 @@ func TestRenameAndOtherOps(t *testing.T) {
 	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "bar=taz&bar=baz")
 }
 
+func TestReplaceOps(t *testing.T) {
+	tester := caddytest.NewTester(t)
+
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query foo bar baz	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "foo=baz")
+}
+
+func TestReplaceWithReplacementPlaceholder(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query foo bar {query.placeholder}	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?placeholder=baz&foo=bar", 200, "foo=baz&placeholder=baz")
+
+}
+
+func TestReplaceWithKeyPlaceholder(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query {query.placeholder} bar baz	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?placeholder=foo&foo=bar", 200, "foo=baz&placeholder=foo")
+}
+
+func TestPartialReplacement(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query foo ar az	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "foo=baz")
+}
+
+func TestNonExistingSearch(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query foo var baz	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "foo=bar")
+}
+
+func TestReplaceAllOps(t *testing.T) {
+	tester := caddytest.NewTester(t)
+
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query * bar baz	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar&baz=bar", 200, "baz=baz&foo=baz")
+}
+
 func TestUriOpsBlock(t *testing.T) {
 	tester := caddytest.NewTester(t)