Repository: caddyserver/caddy (language: go)

Issue:
Nesting `handle` within `route` loses `group` markings
Discovered in https://caddy.community/t/exclusive-basicauth-paths/18528

Consider this Caddyfile:

```
:8881 {
    route {
        handle /foo/* {
            respond "Foo"
        }
        handle {
            respond "Bar"
        }
    }
}
```

The adapted JSON looks like this:

```json
{
  "apps": {
    "http": {
      "servers": {
        "srv0": {
          "listen": [
            ":8881"
          ],
          "routes": [
            {
              "handle": [
                {
                  "handler": "subroute",
                  "routes": [
                    {
                      "handle": [
                        {
                          "handler": "subroute",
                          "routes": [
                            {
                              "handle": [
                                {
                                  "body": "Foo",
                                  "handler": "static_response"
                                }
                              ]
                            }
                          ]
                        }
                      ],
                      "match": [
                        {
                          "path": [
                            "/foo/*"
                          ]
                        }
                      ]
                    },
                    {
                      "handle": [
                        {
                          "handler": "subroute",
                          "routes": [
                            {
                              "handle": [
                                {
                                  "body": "Bar",
                                  "handler": "static_response"
                                }
                              ]
                            }
                          ]
                        }
                      ]
                    }
                  ]
                }
              ]
            }
          ]
        }
      }
    }
  }
}
```

Notice that there's no `"group"` for the handle subroutes as would be expected. This means that the mutual exclusivity part of the `handle` blocks gets broken.

See this Caddyfile without the wrapping `route`, which _does_ have the `"group"` as expected:

```
:8881 {
    handle /foo/* {
        respond "Foo"
    }
    handle {
        respond "Bar"
    }
}
```

```json
{
  "apps": {
    "http": {
      "servers": {
        "srv0": {
          "listen": [
            ":8881"
          ],
          "routes": [
            {
              "group": "group2",
              "match": [
                {
                  "path": [
                    "/foo/*"
                  ]
                }
              ],
              "handle": [
                {
                  "handler": "subroute",
                  "routes": [
                    {
                      "handle": [
                        {
                          "body": "Foo",
                          "handler": "static_response"
                        }
                      ]
                    }
                  ]
                }
              ]
            },
            {
              "group": "group2",
              "handle": [
                {
                  "handler": "subroute",
                  "routes": [
                    {
                      "handle": [
                        {
                          "body": "Bar",
                          "handler": "static_response"
                        }
                      ]
                    }
                  ]
                }
              ]
            }
          ]
        }
      }
    }
  }
}
```

Something with the way `route` is handled makes `group` not work for `handle`. We should figure out what's causing it.


Discussion / hints:

Fix patch:
diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go
index 3f1943c84fc..45da4a8d0ae 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 5ab092d779e..a772dbaacd7 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 77f990b9a5e..00678ef2420 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)
 


Test patch:
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 00000000000..1f77d5c4391
--- /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 2fa2b79797e..8a57b9e375b 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
+}