From 754fe4f7b4dddbfc7a8ee7fee405cee057da858e Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Tue, 13 Sep 2022 13:43:21 -0600
Subject: httpcaddyfile: Fix sorting of repeated directives

Fixes #5037
---
 caddyconfig/httpcaddyfile/directives.go | 43 +++++++++++++++++++--------------
 caddyconfig/httpcaddyfile/httptype.go   |  2 +-
 2 files changed, 26 insertions(+), 19 deletions(-)

(limited to 'caddyconfig')

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)
 		}
 	}
 
-- 
cgit v1.2.3