v2: Implement Caddyfile enhancements (breaking changes) (#2960)

* http: path matcher: exact match by default; substring matches (#2959)

This is a breaking change.

* caddyfile: Change "matcher" directive to "@matcher" syntax (#2959)

* cmd: Assume caddyfile adapter for config files named Caddyfile

* Sub-sort handlers by path matcher length (#2959)

Caddyfile-generated subroutes have handlers, which are sorted first by
directive order (this is unchanged), but within directives we now sort
by specificity of path matcher in descending order (longest path first,
assuming that longest path is most specific).

This only applies if there is only one matcher set, and the path
matcher in that set has only one path in it. Path matchers with two or
more paths are not sorted like this; and routes with more than one
matcher set are not sorted like this either, since specificity is
difficult or impossible to infer correctly.

This is a special case, but definitely a very common one, as a lot of
routing decisions are based on paths.

* caddyfile: New 'route' directive for appearance-order handling (#2959)

* caddyfile: Make rewrite directives mutually exclusive (#2959)

This applies only to rewrites in the top-level subroute created by the
HTTP caddyfile.

4 files changed, 137 insertions, 39 deletions

diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index b523d95..ebb03cc 100644
--- a/caddyconfig/httpcaddyfile/builtins.go
+++ b/caddyconfig/httpcaddyfile/builtins.go
@@ -33,6 +33,7 @@ func init() {
 	RegisterDirective("tls", parseTLS)
 	RegisterHandlerDirective("redir", parseRedir)
 	RegisterHandlerDirective("respond", parseRespond)
+	RegisterHandlerDirective("route", parseRoute)
 }
 
 func parseBind(h Helper) ([]ConfigValue, error) {
@@ -297,3 +298,35 @@ func parseRespond(h Helper) (caddyhttp.MiddlewareHandler, error) {
 	}
 	return sr, nil
 }
+
+func parseRoute(h Helper) (caddyhttp.MiddlewareHandler, error) {
+	sr := new(caddyhttp.Subroute)
+
+	for h.Next() {
+		for nesting := h.Nesting(); h.NextBlock(nesting); {
+			dir := h.Val()
+
+			dirFunc, ok := registeredDirectives[dir]
+			if !ok {
+				return nil, h.Errf("unrecognized directive: %s", dir)
+			}
+
+			subHelper := h
+			subHelper.Dispenser = h.NewFromNextTokens()
+
+			results, err := dirFunc(subHelper)
+			if err != nil {
+				return nil, h.Errf("parsing caddyfile tokens for '%s': %v", dir, err)
+			}
+			for _, result := range results {
+				handler, ok := result.Value.(caddyhttp.Route)
+				if !ok {
+					return nil, h.Errf("%s directive returned something other than an HTTP route: %#v (only handler directives can be used in routes)", dir, result.Value)
+				}
+				sr.Routes = append(sr.Routes, handler)
+			}
+		}
+	}
+
+	return sr, nil
+}
diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index 8f30db2..19ecb26 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -36,6 +36,7 @@ var defaultDirectiveOrder = []string{
 	"request_header",
 	"encode",
 	"templates",
+	"route",
 	"redir",
 	"respond",
 	"reverse_proxy",
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index d8fde46..64b93b0 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -114,37 +114,45 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 		}
 
 		// extract matcher definitions
-		d := sb.block.DispenseDirective("matcher")
-		matcherDefs, err := parseMatcherDefinitions(d)
-		if err != nil {
-			return nil, warnings, err
+		matcherDefs := make(map[string]caddy.ModuleMap)
+		for _, segment := range sb.block.Segments {
+			if dir := segment.Directive(); strings.HasPrefix(dir, matcherPrefix) {
+				d := sb.block.DispenseDirective(dir)
+				err := parseMatcherDefinitions(d, matcherDefs)
+				if err != nil {
+					return nil, warnings, err
+				}
+			}
 		}
 
 		for _, segment := range sb.block.Segments {
 			dir := segment.Directive()
-			if dir == "matcher" {
-				// TODO: This is a special case because we pre-processed it; handle this better
+
+			if strings.HasPrefix(dir, matcherPrefix) {
+				// matcher definitions were pre-processed
 				continue
 			}
-			if dirFunc, ok := registeredDirectives[dir]; ok {
-				results, err := dirFunc(Helper{
-					Dispenser:   caddyfile.NewDispenser(segment),
-					options:     options,
-					warnings:    &warnings,
-					matcherDefs: matcherDefs,
-					parentBlock: sb.block,
-				})
-				if err != nil {
-					return nil, warnings, fmt.Errorf("parsing caddyfile tokens for '%s': %v", dir, err)
-				}
-				for _, result := range results {
-					result.directive = dir
-					sb.pile[result.Class] = append(sb.pile[result.Class], result)
-				}
-			} else {
+
+			dirFunc, ok := registeredDirectives[dir]
+			if !ok {
 				tkn := segment[0]
 				return nil, warnings, fmt.Errorf("%s:%d: unrecognized directive: %s", tkn.File, tkn.Line, dir)
 			}
+
+			results, err := dirFunc(Helper{
+				Dispenser:   caddyfile.NewDispenser(segment),
+				options:     options,
+				warnings:    &warnings,
+				matcherDefs: matcherDefs,
+				parentBlock: sb.block,
+			})
+			if err != nil {
+				return nil, warnings, fmt.Errorf("parsing caddyfile tokens for '%s': %v", dir, err)
+			}
+			for _, result := range results {
+				result.directive = dir
+				sb.pile[result.Class] = append(sb.pile[result.Class], result)
+			}
 		}
 	}
 
@@ -372,10 +380,63 @@ func (st *ServerType) serversFromPairings(
 				}
 				sort.SliceStable(dirRoutes, func(i, j int) bool {
 					iDir, jDir := dirRoutes[i].directive, dirRoutes[j].directive
+					if iDir == jDir {
+						// TODO: we really need to refactor this into a separate function or method...
+						// sub-sort by path matcher length, if there's only one
+						iRoute := dirRoutes[i].Value.(caddyhttp.Route)
+						jRoute := dirRoutes[j].Value.(caddyhttp.Route)
+						if len(iRoute.MatcherSetsRaw) == 1 && len(jRoute.MatcherSetsRaw) == 1 {
+							// for slightly better efficiency, only decode the path matchers once,
+							// then just store them arbitrarily in the decoded MatcherSets field,
+							// ours should be the only thing in there
+							var iPM, jPM caddyhttp.MatchPath
+							if len(iRoute.MatcherSets) == 1 {
+								iPM = iRoute.MatcherSets[0][0].(caddyhttp.MatchPath)
+							}
+							if len(jRoute.MatcherSets) == 1 {
+								jPM = jRoute.MatcherSets[0][0].(caddyhttp.MatchPath)
+							}
+							// if it's our first time seeing this route's path matcher, decode it
+							if iPM == nil {
+								var pathMatcher caddyhttp.MatchPath
+								_ = json.Unmarshal(iRoute.MatcherSetsRaw[0]["path"], &pathMatcher)
+								iRoute.MatcherSets = caddyhttp.MatcherSets{{pathMatcher}}
+								iPM = pathMatcher
+							}
+							if jPM == nil {
+								var pathMatcher caddyhttp.MatchPath
+								_ = json.Unmarshal(jRoute.MatcherSetsRaw[0]["path"], &pathMatcher)
+								jRoute.MatcherSets = caddyhttp.MatcherSets{{pathMatcher}}
+								jPM = pathMatcher
+							}
+							// finally, if there is only one path in the
+							// matcher, sort by longer path first
+							if len(iPM) == 1 && len(jPM) == 1 {
+								return len(iPM[0]) > len(jPM[0])
+							}
+						}
+					}
 					return dirPositions[iDir] < dirPositions[jDir]
 				})
 			}
+
+			// add all the routes piled in from directives
 			for _, r := range dirRoutes {
+				// as a special case, group rewrite directives so that they are mutually exclusive;
+				// this means that only the first matching rewrite will be evaluated, and that's
+				// probably a good thing, since there should never be a need to do more than one
+				// rewrite (I think?), and cascading rewrites smell bad... imagine these rewrites:
+				//     rewrite /docs/json/* /docs/json/index.html
+				//     rewrite /docs/*      /docs/index.html
+				// (We use this on the Caddy website, or at least we did once.) The first rewrite's
+				// result is also matched by the second rewrite, making the first rewrite pointless.
+				// See issue #2959.
+				if r.directive == "rewrite" {
+					route := r.Value.(caddyhttp.Route)
+					route.Group = "rewriting"
+					r.Value = route
+				}
+
 				handlerSubroute.Routes = append(handlerSubroute.Routes, r.Value.(caddyhttp.Route))
 			}
 
@@ -480,17 +541,16 @@ func matcherSetFromMatcherToken(
 	if tkn.Text == "*" {
 		// match all requests == no matchers, so nothing to do
 		return nil, true, nil
-	} else if strings.HasPrefix(tkn.Text, "/") || strings.HasPrefix(tkn.Text, "=/") {
+	} else if strings.HasPrefix(tkn.Text, "/") {
 		// convenient way to specify a single path match
 		return caddy.ModuleMap{
 			"path": caddyconfig.JSON(caddyhttp.MatchPath{tkn.Text}, warnings),
 		}, true, nil
-	} else if strings.HasPrefix(tkn.Text, "match:") {
+	} else if strings.HasPrefix(tkn.Text, matcherPrefix) {
 		// pre-defined matcher
-		matcherName := strings.TrimPrefix(tkn.Text, "match:")
-		m, ok := matcherDefs[matcherName]
+		m, ok := matcherDefs[tkn.Text]
 		if !ok {
-			return nil, false, fmt.Errorf("unrecognized matcher name: %+v", matcherName)
+			return nil, false, fmt.Errorf("unrecognized matcher name: %+v", tkn.Text)
 		}
 		return m, true, nil
 	}
@@ -577,35 +637,37 @@ func (st *ServerType) compileEncodedMatcherSets(sblock caddyfile.ServerBlock) ([
 	return matcherSetsEnc, nil
 }
 
-func parseMatcherDefinitions(d *caddyfile.Dispenser) (map[string]caddy.ModuleMap, error) {
-	matchers := make(map[string]caddy.ModuleMap)
+func parseMatcherDefinitions(d *caddyfile.Dispenser, matchers map[string]caddy.ModuleMap) error {
 	for d.Next() {
 		definitionName := d.Val()
+
+		if _, ok := matchers[definitionName]; ok {
+			return fmt.Errorf("matcher is defined more than once: %s", definitionName)
+		}
+		matchers[definitionName] = make(caddy.ModuleMap)
+
 		for nesting := d.Nesting(); d.NextBlock(nesting); {
 			matcherName := d.Val()
 			mod, err := caddy.GetModule("http.matchers." + matcherName)
 			if err != nil {
-				return nil, fmt.Errorf("getting matcher module '%s': %v", matcherName, err)
+				return fmt.Errorf("getting matcher module '%s': %v", matcherName, err)
 			}
 			unm, ok := mod.New().(caddyfile.Unmarshaler)
 			if !ok {
-				return nil, fmt.Errorf("matcher module '%s' is not a Caddyfile unmarshaler", matcherName)
+				return fmt.Errorf("matcher module '%s' is not a Caddyfile unmarshaler", matcherName)
 			}
 			err = unm.UnmarshalCaddyfile(d.NewFromNextTokens())
 			if err != nil {
-				return nil, err
+				return err
 			}
 			rm, ok := unm.(caddyhttp.RequestMatcher)
 			if !ok {
-				return nil, fmt.Errorf("matcher module '%s' is not a request matcher", matcherName)
-			}
-			if _, ok := matchers[definitionName]; !ok {
-				matchers[definitionName] = make(caddy.ModuleMap)
+				return fmt.Errorf("matcher module '%s' is not a request matcher", matcherName)
 			}
 			matchers[definitionName][matcherName] = caddyconfig.JSON(rm, nil)
 		}
 	}
-	return matchers, nil
+	return nil
 }
 
 func encodeMatcherSet(matchers map[string]caddyhttp.RequestMatcher) (caddy.ModuleMap, error) {
@@ -643,5 +705,7 @@ type sbAddrAssociation struct {
 	serverBlocks []serverBlock
 }
 
+const matcherPrefix = "@"
+
 // Interface guard
 var _ caddyfile.ServerType = (*ServerType)(nil)
diff --git a/caddyconfig/httpcaddyfile/parser_test.go b/caddyconfig/httpcaddyfile/parser_test.go
index bcecf66..ae5751c 100644
--- a/caddyconfig/httpcaddyfile/parser_test.go
+++ b/caddyconfig/httpcaddyfile/parser_test.go
@@ -29,7 +29,7 @@ func TestParse(t *testing.T) {
 	}{
 		{
 			input: `http://localhost
-			matcher debug {
+			@debug {
 			  query showdebug=1
 			}
 			`,
@@ -38,7 +38,7 @@ func TestParse(t *testing.T) {
 		},
 		{
 			input: `http://localhost
-			matcher debug {
+			@debug {
 			  query bad format
 			}
 			`,