rewrite: Add option to force modifying the query

Author: francislavoie

caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.caddyfiletest

@@ -11,7 +11,9 @@ reverse_proxy 127.0.0.1:65535 {
 
 	@accel header X-Accel-Redirect *
 	handle_response @accel {
-		respond "Header X-Accel-Redirect!"
+		rewrite * {rp.header.X-Accel-Redirect} {
+			force_modify_query
+		}
 	}
 
 	@another {
@@ -104,10 +106,12 @@ reverse_proxy 127.0.0.1:65535 {
 											},
 											"routes": [
 												{
+													"group": "group0",
 													"handle": [
 														{
-															"body": "Header X-Accel-Redirect!",
-															"handler": "static_response"
+															"force_modify_query": true,
+															"handler": "rewrite",
+															"uri": "{http.reverse_proxy.header.X-Accel-Redirect}"
 														}
 													]
 												}

modules/caddyhttp/rewrite/caddyfile.go

@@ -34,7 +34,9 @@ func init() {
 
 // parseCaddyfileRewrite sets up a basic rewrite handler from Caddyfile tokens. Syntax:
 //
-//	rewrite [<matcher>] <to>
+//	rewrite [<matcher>] <to> {
+//		force_modify_query
+//	}
 //
 // Only URI components which are given in <to> will be set in the resulting URI.
 // See the docs for the rewrite handler for more information.
@@ -50,12 +52,30 @@ func parseCaddyfileRewrite(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue,
 		return nil, h.Errf("too many arguments; should only be a matcher and a URI")
 	}
 
+	parseBlock := func(rewr *Rewrite) error {
+		for nesting := h.Nesting(); h.NextBlock(nesting); {
+			switch h.Val() {
+			case "force_modify_query":
+				rewr.ForceModifyQuery = true
+
+			default:
+				return h.Errf("unknown subdirective: %s", h.Val())
+			}
+		}
+		return nil
+	}
+
 	// with only one arg, assume it's a rewrite URI with no matcher token
 	if argsCount == 1 {
 		if !h.NextArg() {
 			return nil, h.ArgErr()
 		}
-		return h.NewRoute(nil, Rewrite{URI: h.Val()}), nil
+		rewr := Rewrite{URI: h.Val()}
+		err := parseBlock(&rewr)
+		if err != nil {
+			return nil, err
+		}
+		return h.NewRoute(nil, rewr), nil
 	}
 
 	// parse the matcher token into a matcher set
@@ -66,7 +86,12 @@ func parseCaddyfileRewrite(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue,
 	h.Next() // consume directive name again, matcher parsing does a reset
 	h.Next() // advance to the rewrite URI
 
-	return h.NewRoute(userMatcherSet, Rewrite{URI: h.Val()}), nil
+	rewr := Rewrite{URI: h.Val()}
+	err = parseBlock(&rewr)
+	if err != nil {
+		return nil, err
+	}
+	return h.NewRoute(userMatcherSet, rewr), nil
 }
 
 // parseCaddyfileMethod sets up a basic method rewrite handler from Caddyfile tokens. Syntax:

modules/caddyhttp/rewrite/rewrite.go

@@ -92,6 +92,17 @@ type Rewrite struct {
 	// Mutates the query string of the URI.
 	Query *queryOps `json:"query,omitempty"`
 
+	// If true, the rewrite will be forced to also apply to the
+	// query part of the URL. This is only needed if the configured
+	// URI does not include a '?' character which is normally used
+	// to determine whether the query should be modified. In other
+	// words, this allows rewriting both the path and query when
+	// using a placeholder as the replacement value, whereas otherwise
+	// only the path would be rewritten because the placeholder itself
+	// does not contain a '?' character. Only use this if the placeholder
+	// is trusted to not be vulnerable to query injections.
+	ForceModifyQuery bool `json:"force_modify_query,omitempty"`
+
 	logger *zap.Logger
 }
 
@@ -226,10 +237,15 @@ func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
 			// recompute; new path contains a query string
 			var injectedQuery string
 			newPath, injectedQuery = before, after
-			// don't overwrite explicitly-configured query string
-			if query == "" {
+
+			// don't overwrite explicitly-configured query string,
+			// unless configured explicitly to do so
+			if query == "" || rewr.ForceModifyQuery {
 				query = injectedQuery
 			}
+			if rewr.ForceModifyQuery {
+				qsStart = 0
+			}
 		}
 
 		if query != "" {

modules/caddyhttp/rewrite/rewrite_test.go

@@ -224,6 +224,23 @@ func TestRewrite(t *testing.T) {
 			input:  newRequest(t, "GET", "/foo#fragFirst?c=d"),
 			expect: newRequest(t, "GET", "/bar#fragFirst?c=d"),
 		},
+		{
+			rule:   Rewrite{URI: "{test.path_and_query}"},
+			input:  newRequest(t, "GET", "/"),
+			expect: newRequest(t, "GET", "/foo"),
+		},
+		{
+			// TODO: This might be an incorrect result, since it also replaces
+			// the path with empty string when that might not be the intent.
+			rule:   Rewrite{URI: "{test.query}", ForceModifyQuery: true},
+			input:  newRequest(t, "GET", "/foo"),
+			expect: newRequest(t, "GET", "?bar=1"),
+		},
+		{
+			rule:   Rewrite{URI: "{test.path_and_query}", ForceModifyQuery: true},
+			input:  newRequest(t, "GET", "/"),
+			expect: newRequest(t, "GET", "/foo?bar=1"),
+		},
 
 		{
 			rule:   Rewrite{StripPathPrefix: "/prefix"},
@@ -358,6 +375,9 @@ func TestRewrite(t *testing.T) {
 		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)
+		repl.Set("test.path", "/foo")
+		repl.Set("test.query", "?bar=1")
+		repl.Set("test.path_and_query", "/foo?bar=1")
 
 		// we can't directly call Provision() without a valid caddy.Context
 		// (TODO: fix that) so here we ad-hoc compile the regex