rewrite: Add option to force modifying the query

Author: francislavoie

caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt

@@ -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} {
+			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"
+															"handler": "rewrite",
+															"modify_query": true,
+															"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> {
+//		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.
@@ -48,6 +50,16 @@ func parseCaddyfileRewrite(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler,
 		if h.NextArg() {
 			return nil, h.ArgErr()
 		}
+
+		for nesting := h.Nesting(); h.NextBlock(nesting); {
+			switch h.Val() {
+			case "modify_query":
+				rewr.ModifyQuery = true
+
+			default:
+				return nil, h.Errf("unknown subdirective: %s", h.Val())
+			}
+		}
 	}
 	return rewr, nil
 }

modules/caddyhttp/rewrite/rewrite.go

@@ -88,6 +88,17 @@ type Rewrite struct {
 	// Performs regular expression replacements on the URI path.
 	PathRegexp []*regexReplacer `json:"path_regexp,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.
+	ModifyQuery bool `json:"modify_query,omitempty"`
+
 	logger *zap.Logger
 }
 
@@ -216,10 +227,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.ModifyQuery {
 				query = injectedQuery
 			}
+			if rewr.ModifyQuery {
+				qsStart = 0
+			}
 		}
 
 		if query != "" {

modules/caddyhttp/rewrite/rewrite_test.go

@@ -214,6 +214,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}", ModifyQuery: true},
+			input:  newRequest(t, "GET", "/foo"),
+			expect: newRequest(t, "GET", "?bar=1"),
+		},
+		{
+			rule:   Rewrite{URI: "{test.path_and_query}", ModifyQuery: true},
+			input:  newRequest(t, "GET", "/"),
+			expect: newRequest(t, "GET", "/foo?bar=1"),
+		},
 
 		{
 			rule:   Rewrite{StripPathPrefix: "/prefix"},
@@ -343,6 +360,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