httpcaddyfile: Prevent rewrite routes from consolidating (fix #3108)

It's hard to say whether this was actually a bug, but the linked issue shows why the old behavior was confusing. Basically, we infer that a rewrite handler is supposed to act as an internal redirect, which likely means it will no longer match the matcher(s) it did before the rewrite. So if the rewrite directive shares a matcher with any adjacent route or directive, it can be confusing/misleading if we consolidate the rewrite into the same route as the next handler, which shouldn't (probably) match after the rewrite is complete. This is kiiiind of a hacky workaround to a quirky problem. For edge cases like these, it is probably "cleaner" to just use handle blocks instead, to group handlers under the same matcher, nginx-style.

Matthew Holt committed Mar 18, 2020 at 12:18 UTC 89124aa570004c9b565d6a7841aa847b64dee692

1 file changed +10 -2

caddyconfig/httpcaddyfile/httptype.go

+10 -2

@@ -618,7 +618,16 @@ func buildSubroute(routes []ConfigValue, groupCounter counter) (*caddyhttp.Subro

618 }

619 }

620 // if there is more than one, put them in a group

621 - if info.count > 1 {

621 + // (special case: "rewrite" directive must always be in

622 + // its own group--even if there is only one--because we

623 + // do not want a rewrite to be consolidated into other

624 + // adjacent routes that happen to have the same matcher,

625 + // see caddyserver/caddy#3108 - because the implied

626 + // intent of rewrite is to do an internal redirect,

627 + // we can't assume that the request will continue to

628 + // match the same matcher; anyway, giving a route a

629 + // unique group name should keep it from consolidating)

630 + if info.count > 1 || meDir == "rewrite" {

631 info.groupName = groupCounter.nextGroup()

632 }

633 }

@@ -662,7 +671,6 @@ func buildSubroute(routes []ConfigValue, groupCounter counter) (*caddyhttp.Subro

671 // consolidateRoutes combines routes with the same properties

672 // (same matchers, same Terminal and Group settings) for a

673 // cleaner overall output.

665 -// FIXME: See caddyserver/caddy#3108

674 func consolidateRoutes(routes caddyhttp.RouteList) caddyhttp.RouteList {

675 for i := 0; i < len(routes)-1; i++ {

676 if reflect.DeepEqual(routes[i].MatcherSetsRaw, routes[i+1].MatcherSetsRaw) &&