From c38a040e85a7d277299acaab39bdad171bc45675 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Thu, 19 Jan 2023 05:04:41 +0800 Subject: [PATCH] httpcaddyfile: Fix `handle` grouping inside `route` (#5315) Co-authored-by: Francis Lavoie --- caddyconfig/httpcaddyfile/builtins.go | 15 +--- caddyconfig/httpcaddyfile/directives.go | 2 +- caddyconfig/httpcaddyfile/httptype.go | 16 ++-- .../handle_nested_in_route.txt | 78 +++++++++++++++++++ .../php_fastcgi_expanded_form.txt | 3 +- 5 files changed, 93 insertions(+), 21 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/handle_nested_in_route.txt diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 3f1943c8..45da4a8d 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 5ab092d7..a772dbaa 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 77f990b9..00678ef2 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 00000000..1f77d5c4 --- /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 2fa2b797..8a57b9e3 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 +}