Switch to embedding a Rewrite module inside reverseproxy

caddytest/integration/caddyfile_adapt/reverse_proxy_options.txt

@@ -1,14 +1,11 @@
 
 https://example.com {
-	reverse_proxy /path http://localhost:54321 {
-		header_up Host {host}
-		header_up X-Real-IP {remote}
-		header_up X-Forwarded-For {remote}
-		header_up X-Forwarded-Port {server_port}
-		header_up X-Forwarded-Proto "http"
+	reverse_proxy /path https://localhost:54321 {
+		header_up Host {upstream_hostport}
+		header_up Foo bar
 
-		no_body
-		override_method GET
+		method GET
+		rewrite /rewritten?uri={uri}
 
 		buffer_requests
 
@@ -61,26 +58,19 @@ https://example.com {
 													"headers": {
 														"request": {
 															"set": {
-																"Host": [
-																	"{http.request.host}"
-																],
-																"X-Forwarded-For": [
-																	"{http.request.remote}"
-																],
-																"X-Forwarded-Port": [
-																	"{server_port}"
+																"Foo": [
+																	"bar"
 																],
-																"X-Forwarded-Proto": [
-																	"http"
-																],
-																"X-Real-Ip": [
-																	"{http.request.remote}"
+																"Host": [
+																	"{http.reverse_proxy.upstream.hostport}"
 																]
 															}
 														}
 													},
-													"no_body": true,
-													"override_method": "GET",
+													"rewrite": {
+														"method": "GET",
+														"uri": "/rewritten?uri={http.request.uri}"
+													},
 													"transport": {
 														"compression": false,
 														"dial_fallback_delay": 5000000000,
@@ -101,6 +91,7 @@ https://example.com {
 															]
 														},
 														"response_header_timeout": 8000000000,
+														"tls": {},
 														"versions": [
 															"h2c",
 															"2"

modules/caddyhttp/reverseproxy/caddyfile.go

@@ -27,6 +27,7 @@ import (
 	"github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp/headers"
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite"
 	"github.com/dustin/go-humanize"
 )
 
@@ -89,8 +90,8 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 //         trusted_proxies [private_ranges] <ranges...>
 //         header_up   [+|-]<field> [<value|regexp> [<replacement>]]
 //         header_down [+|-]<field> [<value|regexp> [<replacement>]]
-//         override_method <method>
-//         no_body
+//         method <method>
+//         rewrite <to>
 //
 //         # round trip
 //         transport <name> {
@@ -602,17 +603,29 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					return d.Err(err.Error())
 				}
 
-			case "override_method":
+			case "method":
 				if !d.NextArg() {
 					return d.ArgErr()
 				}
-				h.OverrideMethod = d.Val()
+				if h.Rewrite == nil {
+					h.Rewrite = &rewrite.Rewrite{}
+				}
+				h.Rewrite.Method = d.Val()
+				if d.NextArg() {
+					return d.ArgErr()
+				}
 
-			case "no_body":
+			case "rewrite":
+				if !d.NextArg() {
+					return d.ArgErr()
+				}
+				if h.Rewrite == nil {
+					h.Rewrite = &rewrite.Rewrite{}
+				}
+				h.Rewrite.URI = d.Val()
 				if d.NextArg() {
 					return d.ArgErr()
 				}
-				h.NoBody = true
 
 			case "transport":
 				if !d.NextArg() {

modules/caddyhttp/reverseproxy/reverseproxy.go

@@ -35,6 +35,7 @@ import (
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp/headers"
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite"
 	"go.uber.org/zap"
 	"golang.org/x/net/http/httpguts"
 )
@@ -136,18 +137,17 @@ type Handler struct {
 	// used for the requests and responses (in bytes).
 	MaxBufferSize int64 `json:"max_buffer_size,omitempty"`
 
-	// If true, the request body will not be copied to the backend.
-	// This is useful if you wish to send a request to a backend
-	// to have it decide whether the request should continue being
-	// handled; also known as forward auth.
-	// By default, this is false (disabled).
-	NoBody bool `json:"no_body,omitempty"`
-
-	// If set, overrides the request method on the upstream request.
-	// This is useful for a forward auth scenario, where the method
-	// should always be `GET`.
-	// By default, the incoming request's method is used.
-	OverrideMethod string `json:"override_method,omitempty"`
+	// If configured, rewrites the copy of the upstream request.
+	// Allows changing the request method and URI (path and query).
+	// Since the rewrite is applied to the copy, it does not persist
+	// past the reverse proxy handler.
+	// If the method is changed to `GET` or `HEAD`, the request body
+	// will not be copied to the backend. This allows a later request
+	// handler -- either in a `handle_response` route, or after -- to
+	// read the body.
+	// By default, no rewrite is performed, and the method and URI
+	// from the incoming request is used as-is for proxying.
+	Rewrite *rewrite.Rewrite `json:"rewrite,omitempty"`
 
 	// List of handlers and their associated matchers to evaluate
 	// after successful roundtrips. The first handler that matches
@@ -271,6 +271,13 @@ func (h *Handler) Provision(ctx caddy.Context) error {
 		}
 	}
 
+	if h.Rewrite != nil {
+		err := h.Rewrite.Provision(ctx)
+		if err != nil {
+			return fmt.Errorf("provisioning rewrite: %v", err)
+		}
+	}
+
 	// set up transport
 	if h.Transport == nil {
 		t := &HTTPTransport{}
@@ -398,7 +405,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht
 	repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
 
 	// prepare the request for proxying; this is needed only once
-	clonedReq, err := h.prepareRequest(r)
+	clonedReq, err := h.prepareRequest(r, repl)
 	if err != nil {
 		return caddyhttp.Error(http.StatusInternalServerError,
 			fmt.Errorf("preparing request for upstream round-trip: %v", err))
@@ -549,16 +556,18 @@ func (h *Handler) proxyLoopIteration(r *http.Request, origReq *http.Request, w h
 // properties of the cloned request and should be done just once (before
 // proxying) regardless of proxy retries. This assumes that no mutations
 // of the cloned request are performed by h during or after proxying.
-func (h Handler) prepareRequest(req *http.Request) (*http.Request, error) {
+func (h Handler) prepareRequest(req *http.Request, repl *caddy.Replacer) (*http.Request, error) {
 	req = cloneRequest(req)
 
-	if h.OverrideMethod != "" {
-		req.Method = h.OverrideMethod
-	}
-
-	if h.NoBody {
-		req.ContentLength = 0
-		req.Body = nil
+	// if enabled, perform rewrites on the cloned request; if
+	// the method is GET or HEAD, prevent the request body
+	// from being copied to the upstream
+	if h.Rewrite != nil {
+		changed := h.Rewrite.Rewrite(req, repl)
+		if changed && (h.Rewrite.Method == "GET" || h.Rewrite.Method == "HEAD") {
+			req.ContentLength = 0
+			req.Body = nil
+		}
 	}
 
 	// if enabled, buffer client request; this should only be

modules/caddyhttp/rewrite/rewrite.go

@@ -106,7 +106,7 @@ func (rewr Rewrite) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddy
 		zap.Object("request", caddyhttp.LoggableHTTPRequest{Request: r}),
 	)
 
-	changed := rewr.rewrite(r, repl, logger)
+	changed := rewr.Rewrite(r, repl)
 
 	if changed {
 		logger.Debug("rewrote request",
@@ -121,7 +121,7 @@ func (rewr Rewrite) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddy
 // rewrite performs the rewrites on r using repl, which should
 // have been obtained from r, but is passed in for efficiency.
 // It returns true if any changes were made to r.
-func (rewr Rewrite) rewrite(r *http.Request, repl *caddy.Replacer, logger *zap.Logger) bool {
+func (rewr Rewrite) Rewrite(r *http.Request, repl *caddy.Replacer) bool {
 	oldMethod := r.Method
 	oldURI := r.RequestURI
 

modules/caddyhttp/rewrite/rewrite_test.go

@@ -292,7 +292,7 @@ func TestRewrite(t *testing.T) {
 			rep.re = re
 		}
 
-		changed := tc.rule.rewrite(tc.input, repl, nil)
+		changed := tc.rule.Rewrite(tc.input, repl)
 
 		if expected, actual := !reqEqual(originalInput, tc.input), changed; expected != actual {
 			t.Errorf("Test %d: Expected changed=%t but was %t", i, expected, actual)