httpcaddyfile: Fix sorting of repeated directives

Fixes #5037
This commit is contained in:
Matthew Holt 2022-09-13 13:43:21 -06:00
parent 20d487be57
commit 754fe4f7b4
No known key found for this signature in database
GPG key ID: 2A349DD577D586A5
4 changed files with 49 additions and 20 deletions

View file

@ -406,7 +406,7 @@ func sortRoutes(routes []ConfigValue) {
return false 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 var iPM, jPM caddyhttp.MatchPath
if len(iRoute.MatcherSetsRaw) == 1 { if len(iRoute.MatcherSetsRaw) == 1 {
_ = json.Unmarshal(iRoute.MatcherSetsRaw[0]["path"], &iPM) _ = json.Unmarshal(iRoute.MatcherSetsRaw[0]["path"], &iPM)
@ -415,39 +415,46 @@ func sortRoutes(routes []ConfigValue) {
_ = json.Unmarshal(jRoute.MatcherSetsRaw[0]["path"], &jPM) _ = json.Unmarshal(jRoute.MatcherSetsRaw[0]["path"], &jPM)
} }
// sort by longer path (more specific) first; missing path // if there is only one path in the path matcher, sort by longer path
// matchers or multi-matchers are treated as zero-length paths // (more specific) first; missing path matchers or multi-matchers are
// treated as zero-length paths
var iPathLen, jPathLen int var iPathLen, jPathLen int
if len(iPM) > 0 { if len(iPM) == 1 {
iPathLen = len(iPM[0]) iPathLen = len(iPM[0])
} }
if len(jPM) > 0 { if len(jPM) == 1 {
jPathLen = len(jPM[0]) jPathLen = len(jPM[0])
} }
// some directives involve setting values which can overwrite // 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 // that the lease specific matcher is first; everything else
// has most-specific matcher first // has most-specific matcher first
if iDir == "vars" { if iDir == "vars" {
// if both directives have no path matcher, use whichever one // we can only confidently compare path lengths if both
// has no matcher first. // directives have a single path to match (issue #5037)
if iPathLen == 0 && jPathLen == 0 { if iPathLen > 0 && jPathLen > 0 {
return len(iRoute.MatcherSetsRaw) == 0 && len(jRoute.MatcherSetsRaw) > 0 // sort least-specific (shortest) path first
}
// sort with the least-specific (shortest) path first
return iPathLen < jPathLen return iPathLen < jPathLen
} 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
} }
// sort with the most-specific (longest) path first // 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 {
// 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 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
}
}) })
} }

View file

@ -955,7 +955,7 @@ func appendSubrouteToRouteList(routeList caddyhttp.RouteList,
func buildSubroute(routes []ConfigValue, groupCounter counter) (*caddyhttp.Subroute, error) { func buildSubroute(routes []ConfigValue, groupCounter counter) (*caddyhttp.Subroute, error) {
for _, val := range routes { for _, val := range routes {
if !directiveIsOrdered(val.directive) { 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)
} }
} }

View file

@ -100,7 +100,7 @@ func (tc *Tester) InitServer(rawConfig string, configType string) {
tc.t.Fail() tc.t.Fail()
} }
if err := tc.ensureConfigRunning(rawConfig, configType); err != nil { if err := tc.ensureConfigRunning(rawConfig, configType); err != nil {
tc.t.Logf("failed ensurng config is running: %s", err) tc.t.Logf("failed ensuring config is running: %s", err)
tc.t.Fail() tc.t.Fail()
} }
} }

View file

@ -109,6 +109,17 @@ func (r Route) Empty() bool {
r.Group == "" r.Group == ""
} }
func (r Route) String() string {
handlersRaw := "["
for _, hr := range r.HandlersRaw {
handlersRaw += " " + string(hr)
}
handlersRaw += "]"
return fmt.Sprintf(`{Group:"%s" MatcherSetsRaw:%s HandlersRaw:%s Terminal:%t}`,
r.Group, r.MatcherSetsRaw, handlersRaw, r.Terminal)
}
// RouteList is a list of server routes that can // RouteList is a list of server routes that can
// create a middleware chain. // create a middleware chain.
type RouteList []Route type RouteList []Route
@ -331,4 +342,15 @@ func (ms *MatcherSets) FromInterface(matcherSets any) error {
return nil return nil
} }
// TODO: Is this used?
func (ms MatcherSets) String() string {
result := "["
for _, matcherSet := range ms {
for _, matcher := range matcherSet {
result += fmt.Sprintf(" %#v", matcher)
}
}
return result + " ]"
}
var routeGroupCtxKey = caddy.CtxKey("route_group") var routeGroupCtxKey = caddy.CtxKey("route_group")