httpcaddyfile: Fix `handle` grouping inside `route` (#5315)

Co-authored-by: Francis Lavoie <lavofr@gmail.com>

5 files changed, 93 insertions, 21 deletions

diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index 3f1943c..45da4a8 100644
--- a/caddyconfig/httpcaddyfile/builtins.go
+++ b/caddyconfig/httpcaddyfile/builtins.go
@@ -731,29 +731,20 @@ func parseError(h Helper) (caddyhttp.MiddlewareHandler, error) {
 
 // parseRoute parses the route directive.
 func parseRoute(h Helper) (caddyhttp.MiddlewareHandler, error) {
-	sr := new(caddyhttp.Subroute)
-
 	allResults, err := parseSegmentAsConfig(h)
 	if err != nil {
 		return nil, err
 	}
 
 	for _, result := range allResults {
-		switch handler := result.Value.(type) {
-		case caddyhttp.Route:
-			sr.Routes = append(sr.Routes, handler)
-		case caddyhttp.Subroute:
-			// directives which return a literal subroute instead of a route
-			// means they intend to keep those handlers together without
-			// them being reordered; we're doing that anyway since we're in
-			// the route directive, so just append its handlers
-			sr.Routes = append(sr.Routes, handler.Routes...)
+		switch result.Value.(type) {
+		case caddyhttp.Route, caddyhttp.Subroute:
 		default:
 			return nil, h.Errf("%s directive returned something other than an HTTP route or subroute: %#v (only handler directives can be used in routes)", result.directive, result.Value)
 		}
 	}
 
-	return sr, nil
+	return buildSubroute(allResults, h.groupCounter, false)
 }
 
 func parseHandle(h Helper) (caddyhttp.MiddlewareHandler, error) {
diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index 5ab092d..a772dba 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -289,7 +289,7 @@ func ParseSegmentAsSubroute(h Helper) (caddyhttp.MiddlewareHandler, error) {
 		return nil, err
 	}
 
-	return buildSubroute(allResults, h.groupCounter)
+	return buildSubroute(allResults, h.groupCounter, true)
 }
 
 // parseSegmentAsConfig parses the segment such that its subdirectives
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index 77f990b..00678ef 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -618,7 +618,7 @@ func (st *ServerType) serversFromPairings(
 
 			// set up each handler directive, making sure to honor directive order
 			dirRoutes := sblock.pile["route"]
-			siteSubroute, err := buildSubroute(dirRoutes, groupCounter)
+			siteSubroute, err := buildSubroute(dirRoutes, groupCounter, true)
 			if err != nil {
 				return nil, err
 			}
@@ -959,14 +959,16 @@ func appendSubrouteToRouteList(routeList caddyhttp.RouteList,
 
 // buildSubroute turns the config values, which are expected to be routes
 // into a clean and orderly subroute that has all the routes within it.
-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 an ordered HTTP handler, so it cannot be used here", val.directive)
+func buildSubroute(routes []ConfigValue, groupCounter counter, needsSorting bool) (*caddyhttp.Subroute, error) {
+	if needsSorting {
+		for _, val := range routes {
+			if !directiveIsOrdered(val.directive) {
+				return nil, fmt.Errorf("directive '%s' is not an ordered HTTP handler, so it cannot be used here", val.directive)
+			}
 		}
-	}
 
-	sortRoutes(routes)
+		sortRoutes(routes)
+	}
 
 	subroute := new(caddyhttp.Subroute)
 
diff --git a/caddytest/integration/caddyfile_adapt/handle_nested_in_route.txt b/caddytest/integration/caddyfile_adapt/handle_nested_in_route.txt
new file mode 100644
index 0000000..1f77d5c
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/handle_nested_in_route.txt
@@ -0,0 +1,78 @@
+:8881 {
+	route {
+		handle /foo/* {
+			respond "Foo"
+		}
+		handle {
+			respond "Bar"
+		}
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8881"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"body": "Foo",
+																	"handler": "static_response"
+																}
+															]
+														}
+													]
+												}
+											],
+											"match": [
+												{
+													"path": [
+														"/foo/*"
+													]
+												}
+											]
+										},
+										{
+											"group": "group2",
+											"handle": [
+												{
+													"handler": "subroute",
+													"routes": [
+														{
+															"handle": [
+																{
+																	"body": "Bar",
+																	"handler": "static_response"
+																}
+															]
+														}
+													]
+												}
+											]
+										}
+									]
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/caddytest/integration/caddyfile_adapt/php_fastcgi_expanded_form.txt b/caddytest/integration/caddyfile_adapt/php_fastcgi_expanded_form.txt
index 2fa2b79..8a57b9e 100644
--- a/caddytest/integration/caddyfile_adapt/php_fastcgi_expanded_form.txt
+++ b/caddytest/integration/caddyfile_adapt/php_fastcgi_expanded_form.txt
@@ -74,6 +74,7 @@ route {
 											]
 										},
 										{
+											"group": "group0",
 											"handle": [
 												{
 													"handler": "rewrite",
@@ -129,4 +130,4 @@ route {
 			}
 		}
 	}
-}
\ No newline at end of file
+}