httpcaddyfile: Fix nested blocks; add handle directive; refactor

The fix that was initially put forth in #2971 was good, but only for
up to one layer of nesting. The real problem was that we forgot to
increment nesting when already inside a block if we saw another open
curly brace that opens another block (dispenser.go L157-158).

The new 'handle' directive allows HTTP Caddyfiles to be designed more
like nginx location blocks if the user prefers. Inside a handle block,
directives are still ordered just like they are outside of them, but
handler blocks at a given level of nesting are mutually exclusive.

This work benefitted from some refactoring and cleanup.

5 files changed, 18 insertions, 21 deletions

diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go
index 6cb049221..69e3318ca 100644
--- a/modules/caddyhttp/autohttps.go
+++ b/modules/caddyhttp/autohttps.go
@@ -338,6 +338,9 @@ func (app *App) automaticHTTPSPhase2() error {
 		if err != nil {
 			return fmt.Errorf("%s: managing certificate for %s: %s", srvName, domains, err)
 		}
+
+		// no longer needed; allow GC to deallocate
+		srv.AutoHTTPS.domainSet = nil
 	}
 
 	return nil
diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go
index 8e0654667..06f137ed9 100644
--- a/modules/caddyhttp/matchers_test.go
+++ b/modules/caddyhttp/matchers_test.go
@@ -199,7 +199,7 @@ func TestPathMatcher(t *testing.T) {
 		},
 		{
 			match:  MatchPath{"*.ext"},
-			input:  "/foo.ext",
+			input:  "/foo/bar.ext",
 			expect: true,
 		},
 		{
diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
index dee6eb57e..8c9fd386a 100644
--- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go
@@ -81,7 +81,7 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 //
 //     php_fastcgi localhost:7777
 //
-// is equivalent to:
+// is equivalent to a route consisting of:
 //
 //     @canonicalPath {
 //         file {
@@ -104,8 +104,8 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 //         }
 //     }
 //
-// Thus, this directive produces multiple routes, each with a different
-// matcher because multiple consecutive routes are necessary to support
+// Thus, this directive produces multiple handlers, each with a different
+// matcher because multiple consecutive hgandlers are necessary to support
 // the common PHP use case. If this "common" config is not compatible
 // with a user's PHP requirements, they can use a manual approach based
 // on the example above to configure it precisely as they need.
@@ -114,7 +114,7 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 //
 //     php_fastcgi /subpath localhost:7777
 //
-// then the resulting routes are wrapped in a subroute that uses the
+// then the resulting handlers are wrapped in a subroute that uses the
 // user's matcher as a prerequisite to enter the subroute. In other
 // words, the directive's matcher is necessary, but not sufficient.
 func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
@@ -198,12 +198,13 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 		HandlersRaw:    []json.RawMessage{caddyconfig.JSONModuleObject(rpHandler, "handler", "reverse_proxy", nil)},
 	}
 
+	subroute := caddyhttp.Subroute{
+		Routes: caddyhttp.RouteList{redirRoute, rewriteRoute, rpRoute},
+	}
+
 	// the user's matcher is a prerequisite for ours, so
 	// wrap ours in a subroute and return that
 	if hasUserMatcher {
-		subroute := caddyhttp.Subroute{
-			Routes: caddyhttp.RouteList{redirRoute, rewriteRoute, rpRoute},
-		}
 		return []httpcaddyfile.ConfigValue{
 			{
 				Class: "route",
@@ -215,20 +216,14 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 		}, nil
 	}
 
-	// if the user did not specify a matcher, then
-	// we can just use our own matchers
+	// otherwise, return the literal subroute instead of
+	// individual routes, to ensure they stay together and
+	// are treated as a single unit, without necessarily
+	// creating an actual subroute in the output
 	return []httpcaddyfile.ConfigValue{
 		{
 			Class: "route",
-			Value: redirRoute,
-		},
-		{
-			Class: "route",
-			Value: rewriteRoute,
-		},
-		{
-			Class: "route",
-			Value: rpRoute,
+			Value: subroute,
 		},
 	}, nil
 }
diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go
index e2d57c44d..ad054866a 100644
--- a/modules/caddyhttp/rewrite/rewrite.go
+++ b/modules/caddyhttp/rewrite/rewrite.go
@@ -50,7 +50,7 @@ type Rewrite struct {
 	// You can also use placeholders. For example, to preserve the existing
 	// query string, you might use: "?{http.request.uri.query}&a=b". Any
 	// key-value pairs you add to the query string will not overwrite
-	// existing values.
+	// existing values (individual pairs are append-only).
 	//
 	// To clear the query string, explicitly set an empty one: "?"
 	URI string `json:"uri,omitempty"`
diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go
index d4ff02a2e..1224e3297 100644
--- a/modules/caddyhttp/routes.go
+++ b/modules/caddyhttp/routes.go
@@ -252,7 +252,6 @@ func wrapMiddleware(mh MiddlewareHandler) Middleware {
 
 		return HandlerFunc(func(w http.ResponseWriter, r *http.Request) error {
 			// TODO: This is where request tracing could be implemented
-			// TODO: Trace a diff of the request, would be cool too... see what changed since the last middleware (host, headers, URI...)
 			// TODO: see what the std lib gives us in terms of stack tracing too
 			return mh.ServeHTTP(w, r, nextCopy)
 		})