httpcaddyfile: Fix sorting of repeated directives

Fixes #5037

4 files changed, 49 insertions, 20 deletions

diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index 89706b3..e2113eb 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -406,7 +406,7 @@ func sortRoutes(routes []ConfigValue) {
 			return false
 		}
 
-		// decode the path matchers, if there is just one of them
+		// decode the path matchers if there is just one matcher set
 		var iPM, jPM caddyhttp.MatchPath
 		if len(iRoute.MatcherSetsRaw) == 1 {
 			_ = json.Unmarshal(iRoute.MatcherSetsRaw[0]["path"], &iPM)
@@ -415,38 +415,45 @@ func sortRoutes(routes []ConfigValue) {
 			_ = json.Unmarshal(jRoute.MatcherSetsRaw[0]["path"], &jPM)
 		}
 
-		// sort by longer path (more specific) first; missing path
-		// matchers or multi-matchers are treated as zero-length paths
+		// if there is only one path in the path matcher, sort by longer path
+		// (more specific) first; missing path matchers or multi-matchers are
+		// treated as zero-length paths
 		var iPathLen, jPathLen int
-		if len(iPM) > 0 {
+		if len(iPM) == 1 {
 			iPathLen = len(iPM[0])
 		}
-		if len(jPM) > 0 {
+		if len(jPM) == 1 {
 			jPathLen = len(jPM[0])
 		}
 
 		// some directives involve setting values which can overwrite
-		// eachother, so it makes most sense to reverse the order so
+		// each other, so it makes most sense to reverse the order so
 		// that the lease specific matcher is first; everything else
 		// has most-specific matcher first
 		if iDir == "vars" {
-			// if both directives have no path matcher, use whichever one
-			// has no matcher first.
-			if iPathLen == 0 && jPathLen == 0 {
-				return len(iRoute.MatcherSetsRaw) == 0 && len(jRoute.MatcherSetsRaw) > 0
+			// we can only confidently compare path lengths if both
+			// directives have a single path to match (issue #5037)
+			if iPathLen > 0 && jPathLen > 0 {
+				// sort least-specific (shortest) path first
+				return iPathLen < jPathLen
 			}
 
-			// sort with the least-specific (shortest) path first
-			return iPathLen < jPathLen
+			// if both directives don't have a single path to compare,
+			// sort whichever one has no matcher first; if both have
+			// no matcher, sort equally (stable sort preserves order)
+			return len(iRoute.MatcherSetsRaw) == 0 && len(jRoute.MatcherSetsRaw) > 0
 		} else {
-			// if both directives have no path matcher, use whichever one
-			// has any kind of matcher defined first.
-			if iPathLen == 0 && jPathLen == 0 {
-				return len(iRoute.MatcherSetsRaw) > 0 && len(jRoute.MatcherSetsRaw) == 0
+			// we can only confidently compare path lengths if both
+			// directives have a single path to match (issue #5037)
+			if iPathLen > 0 && jPathLen > 0 {
+				// sort most-specific (longest) path first
+				return iPathLen > jPathLen
 			}
 
-			// sort with the most-specific (longest) path first
-			return iPathLen > jPathLen
+			// if both directives don't have a single path to compare,
+			// sort whichever one has a matcher first; if both have
+			// a matcher, sort equally (stable sort preserves order)
+			return len(iRoute.MatcherSetsRaw) > 0 && len(jRoute.MatcherSetsRaw) == 0
 		}
 	})
 }
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index 72d98bd..9cf386a 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -955,7 +955,7 @@ func appendSubrouteToRouteList(routeList caddyhttp.RouteList,
 func buildSubroute(routes []ConfigValue, groupCounter counter) (*caddyhttp.Subroute, error) {
 	for _, val := range routes {
 		if !directiveIsOrdered(val.directive) {
-			return nil, fmt.Errorf("directive '%s' is not ordered, so it cannot be used here", val.directive)
+			return nil, fmt.Errorf("directive '%s' is not an ordered HTTP handler, so it cannot be used here", val.directive)
 		}
 	}
 
diff --git a/caddytest/caddytest.go b/caddytest/caddytest.go
index 9addd14..4fb3394 100644
--- a/caddytest/caddytest.go
+++ b/caddytest/caddytest.go
@@ -100,7 +100,7 @@ func (tc *Tester) InitServer(rawConfig string, configType string) {
 		tc.t.Fail()
 	}
 	if err := tc.ensureConfigRunning(rawConfig, configType); err != nil {
-		tc.t.Logf("failed ensurng config is running: %s", err)
+		tc.t.Logf("failed ensuring config is running: %s", err)
 		tc.t.Fail()
 	}
 }
diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go
index 45cbd80..ce9bece 100644
--- a/modules/caddyhttp/routes.go
+++ b/modules/caddyhttp/routes.go
@@ -109,6 +109,17 @@ func (r Route) Empty() bool {
 		r.Group == ""
 }
 
+func (r Route) String() string {
+	handlersRaw := "["
+	for _, hr := range r.HandlersRaw {
+		handlersRaw += " " + string(hr)
+	}
+	handlersRaw += "]"
+
+	return fmt.Sprintf(`{Group:"%s" MatcherSetsRaw:%s HandlersRaw:%s Terminal:%t}`,
+		r.Group, r.MatcherSetsRaw, handlersRaw, r.Terminal)
+}
+
 // RouteList is a list of server routes that can
 // create a middleware chain.
 type RouteList []Route
@@ -331,4 +342,15 @@ func (ms *MatcherSets) FromInterface(matcherSets any) error {
 	return nil
 }
 
+// TODO: Is this used?
+func (ms MatcherSets) String() string {
+	result := "["
+	for _, matcherSet := range ms {
+		for _, matcher := range matcherSet {
+			result += fmt.Sprintf(" %#v", matcher)
+		}
+	}
+	return result + " ]"
+}
+
 var routeGroupCtxKey = caddy.CtxKey("route_group")