Cleaned up comments, changed the order of operations and added more t…

…ests

caddytest/integration/caddyfile_test.go

@@ -517,6 +517,58 @@ func TestUriOps(t *testing.T) {
 	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar0&baz=buz&taz=nottest&changethis=val", 200, "changed=val&foo=bar0&foo=bar&key%3Dvalue=example&taz=test")
 }
 
+func TestSetThenAddQueryParams(t *testing.T) {
+	tester := caddytest.NewTester(t)
+
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query foo bar
+	uri query +foo baz
+	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint", 200, "foo=bar&foo=baz")
+}
+
+func TestSetThenDeleteParams(t *testing.T) {
+	tester := caddytest.NewTester(t)
+
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query bar foo{query.foo}
+	uri query -foo
+	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "bar=foobar")
+}
+
+func TestRenameAndOtherOps(t *testing.T) {
+	tester := caddytest.NewTester(t)
+
+	tester.InitServer(`
+	{
+		admin localhost:2999
+		http_port     9080
+	}
+	:9080
+	uri query foo>bar
+	uri query bar taz
+	uri query +bar baz
+	
+	respond "{query}"`, "caddyfile")
+
+	tester.AssertGetResponse("http://localhost:9080/endpoint?foo=bar", 200, "bar=taz&bar=baz")
+}
+
 func TestUriOpsBlock(t *testing.T) {
 	tester := caddytest.NewTester(t)
 

modules/caddyhttp/rewrite/rewrite.go

@@ -90,8 +90,7 @@ type Rewrite struct {
 	PathRegexp []*regexReplacer `json:"path_regexp,omitempty"`
 
 	// Mutates the query string of the URI.
-
-	Query *queryOps `json:"query_operations,omitempty"`
+	Query *queryOps `json:"query,omitempty"`
 
 	logger *zap.Logger
 }
@@ -479,39 +478,33 @@ func changePath(req *http.Request, newVal func(pathOrRawPath string) string) {
 	}
 }
 
-// queryOps describes the operations to perform on a query like add, delete and replace fields
+// queryOps describes the operations to perform on query keys: add, set, rename and delete.
 type queryOps struct {
-	// Adds query parameters; does not replace any existing header fields.
+	// Adds query parameters; does not overwrite an existing query field,
+	// and only appends an additional value for that key if any already exist.
 	Add []queryOpsArguments `json:"add,omitempty"`
 
-	// Sets query parameters; replaces existing header fields.
+	// Sets query parameters; overwrites a query key with the given value.
 	Set []queryOpsArguments `json:"set,omitempty"`
 
-	// Renames query keys
+	// Renames a query key from Key to Val, without affecting the value.
 	Rename []queryOpsArguments `json:"rename,omitempty"`
 
-	// Names of query parameters to delete
+	// Deletes a given query key by name.
 	Delete []string `json:"delete,omitempty"`
 }
 
 func (q *queryOps) do(r *http.Request, repl *caddy.Replacer) {
 	query := r.URL.Query()
 
-	for _, addParam := range q.Add {
-		key := repl.ReplaceAll(addParam.Key, "")
-		if key == "" {
-			continue
-		}
-		val := repl.ReplaceAll(addParam.Val, "")
-		query[key] = append(query[key], val)
-	}
-
-	for _, deleteParam := range q.Delete {
-		param := repl.ReplaceAll(deleteParam, "")
-		if param == "" {
+	for _, renameParam := range q.Rename {
+		key := repl.ReplaceAll(renameParam.Key, "")
+		val := repl.ReplaceAll(renameParam.Val, "")
+		if key == "" || val == "" {
 			continue
 		}
-		delete(query, param)
+		query[val] = query[key]
+		delete(query, key)
 	}
 
 	for _, setParam := range q.Set {
@@ -523,14 +516,21 @@ func (q *queryOps) do(r *http.Request, repl *caddy.Replacer) {
 		query[key] = []string{val}
 	}
 
-	for _, renameParam := range q.Rename {
-		key := repl.ReplaceAll(renameParam.Key, "")
-		val := repl.ReplaceAll(renameParam.Val, "")
-		if key == "" || val == "" {
+	for _, addParam := range q.Add {
+		key := repl.ReplaceAll(addParam.Key, "")
+		if key == "" {
 			continue
 		}
-		query[val] = query[key]
-		delete(query, key)
+		val := repl.ReplaceAll(addParam.Val, "")
+		query[key] = append(query[key], val)
+	}
+
+	for _, deleteParam := range q.Delete {
+		param := repl.ReplaceAll(deleteParam, "")
+		if param == "" {
+			continue
+		}
+		delete(query, param)
 	}
 
 	r.URL.RawQuery = query.Encode()
@@ -539,6 +539,10 @@ func (q *queryOps) do(r *http.Request, repl *caddy.Replacer) {
 type queryOpsArguments struct {
 	// A key in the query string. Note that query string keys may appear multiple times.
 	Key string `json:"key,omitempty"`
+
+	// The value for the given operation; for add and set, this is
+	// simply the value of the query, and for rename this is the
+	// query key to rename to.
 	Val string `json:"val,omitempty"`
 }