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.

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
+}

caddyconfig/httpcaddyfile/directives.go

@@ -36,6 +36,7 @@ var defaultDirectiveOrder = []string{
 	"request_header",
 	"encode",
 	"templates",
+	"route",
 	"redir",
 	"respond",
 	"reverse_proxy",

caddyconfig/httpcaddyfile/httptype.go

@@ -114,19 +114,31 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 		}
 
 		// extract matcher definitions
-		d := sb.block.DispenseDirective("matcher")
-		matcherDefs, err := parseMatcherDefinitions(d)
+		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 {
+
+			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,
@@ -141,10 +153,6 @@ func (st ServerType) Setup(originalServerBlocks []caddyfile.ServerBlock,
 				result.directive = dir
 				sb.pile[result.Class] = append(sb.pile[result.Class], result)
 			}
-			} else {
-				tkn := segment[0]
-				return nil, warnings, fmt.Errorf("%s:%d: unrecognized directive: %s", tkn.File, tkn.Line, dir)
-			}
 		}
 	}
 
@@ -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)

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
 			}
 			`,

cmd/main.go

@@ -134,6 +134,13 @@ func loadConfig(configFile, adapterName string) ([]byte, error) {
 		}
 	}
 
+	// as a special case, if a config file called "Caddyfile" was
+	// specified, and no adapter is specified, assume caddyfile adapter
+	// for convenience
+	if filepath.Base(configFile) == "Caddyfile" && adapterName == "" {
+		adapterName = "caddyfile"
+	}
+
 	// load config adapter
 	if adapterName != "" {
 		cfgAdapter = caddyconfig.GetAdapter(adapterName)

modules/caddyhttp/matchers.go

@@ -46,7 +46,17 @@ type (
 	// [customized or disabled](/docs/json/apps/http/servers/automatic_https/).
 	MatchHost []string
 
-	// MatchPath matches requests by the URI's path (case-insensitive).
+	// MatchPath matches requests by the URI's path (case-insensitive). Path
+	// matches are exact, but wildcards may be used:
+	//
+	// - At the end, for a prefix match (`/prefix/*`)
+	// - At the beginning, for a suffix match (`*.suffix`)
+	// - On both sides, for a substring match (`*/contains/*`)
+	// - In the middle, for a globular match (`/accounts/*/info`)
+	//
+	// This matcher is fast, so it does not support regular expressions or
+	// capture groups. For slower but more capable matching, use the path_regexp
+	// matcher.
 	MatchPath []string
 
 	// MatchPathRE matches requests by a regular expression on the URI's path.
@@ -197,11 +207,15 @@ func (m MatchPath) Match(r *http.Request) bool {
 	// being matched by *.php to be treated as PHP scripts
 	lowerPath = strings.TrimRight(lowerPath, ". ")
 
+	repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
+
 	for _, matchPath := range m {
-		// special case: first character is equals sign,
-		// treat it as an exact match
-		if strings.HasPrefix(matchPath, "=") {
-			if lowerPath == matchPath[1:] {
+		matchPath = repl.ReplaceAll(matchPath, "")
+
+		// special case: first and last characters are wildcard,
+		// treat it as a fast substring match
+		if strings.HasPrefix(matchPath, "*") && strings.HasSuffix(matchPath, "*") {
+			if strings.Contains(lowerPath, matchPath[1:len(matchPath)-1]) {
 				return true
 			}
 			continue
@@ -216,15 +230,22 @@ func (m MatchPath) Match(r *http.Request) bool {
 			continue
 		}
 
+		// special case: last character is a wildcard,
+		// treat it as a fast prefix match
+		if strings.HasSuffix(matchPath, "*") {
+			if strings.HasPrefix(lowerPath, matchPath[:len(matchPath)-1]) {
+				return true
+			}
+			continue
+		}
+
+		// for everything else, try globular matching, which also
+		// is exact matching if there are no glob/wildcard chars;
 		// can ignore error here because we can't handle it anyway
 		matches, _ := filepath.Match(matchPath, lowerPath)
 		if matches {
 			return true
 		}
-
-		if strings.HasPrefix(lowerPath, matchPath) {
-			return true
-		}
 	}
 	return false
 }

modules/caddyhttp/matchers_test.go

@@ -182,9 +182,19 @@ func TestPathMatcher(t *testing.T) {
 			input:  "/foo/bar",
 			expect: false,
 		},
+		{
+			match:  MatchPath{"/foo/bar/"},
+			input:  "/foo/bar/",
+			expect: true,
+		},
 		{
 			match:  MatchPath{"/foo/bar/", "/other"},
 			input:  "/other/",
+			expect: false,
+		},
+		{
+			match:  MatchPath{"/foo/bar/", "/other"},
+			input:  "/other",
 			expect: true,
 		},
 		{
@@ -213,25 +223,30 @@ func TestPathMatcher(t *testing.T) {
 			expect: false,
 		},
 		{
-			match:  MatchPath{"=/foo"},
-			input:  "/foo",
-			expect: true,
-		},
-		{
-			match:  MatchPath{"=/foo"},
-			input:  "/foo/bar",
-			expect: false,
-		},
-		{
-			match:  MatchPath{"=/foo"},
-			input:  "/FOO",
+			match:  MatchPath{"*substring*"},
+			input:  "/foo/substring/bar.txt",
 			expect: true,
 		},
+		{
+			match:  MatchPath{"/foo"},
+			input:  "/foo/bar",
+			expect: false,
+		},
+		{
+			match:  MatchPath{"/foo"},
+			input:  "/foo/bar",
+			expect: false,
+		},
 		{
 			match:  MatchPath{"/foo"},
 			input:  "/FOO",
 			expect: true,
 		},
+		{
+			match:  MatchPath{"/foo*"},
+			input:  "/FOOOO",
+			expect: true,
+		},
 		{
 			match:  MatchPath{"/foo/bar.txt"},
 			input:  "/foo/BAR.txt",
@@ -239,6 +254,10 @@ func TestPathMatcher(t *testing.T) {
 		},
 	} {
 		req := &http.Request{URL: &url.URL{Path: tc.input}}
+		repl := caddy.NewReplacer()
+		ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)
+		req = req.WithContext(ctx)
+
 		actual := tc.match.Match(req)
 		if actual != tc.expect {
 			t.Errorf("Test %d %v: Expected %t, got %t for '%s'", i, tc.match, tc.expect, actual, tc.input)
@@ -251,8 +270,13 @@ func TestPathMatcherWindows(t *testing.T) {
 	// only Windows has this bug where it will ignore
 	// trailing dots and spaces in a filename, but we
 	// test for it on all platforms to be more consistent
-	match := MatchPath{"*.php"}
+
 	req := &http.Request{URL: &url.URL{Path: "/index.php . . .."}}
+	repl := caddy.NewReplacer()
+	ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)
+	req = req.WithContext(ctx)
+
+	match := MatchPath{"*.php"}
 	matched := match.Match(req)
 	if !matched {
 		t.Errorf("Expected to match; should ignore trailing dots and spaces")