Skip to content

Commit

Permalink
httpcaddyfile: Fix route ordering bug
Browse files Browse the repository at this point in the history
https://caddy.community/t/cant-get-simple-alias-to-work/7911/8?u=matt

This removes an optimization where we amortized path matcher decoding.
The decoded matchers were index by... position... which obviously
changes during sorting. Duh.

Anyway, sorting is sliiightly slower now but the Caddyfile is not
really CPU-sensitive, so this is fine.
  • Loading branch information
mholt committed May 7, 2020
1 parent 8dbc5f7 commit cd9317e
Showing 1 changed file with 33 additions and 41 deletions.
74 changes: 33 additions & 41 deletions caddyconfig/httpcaddyfile/directives.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,53 +276,45 @@ func sortRoutes(routes []ConfigValue) {
dirPositions[dir] = i
}

// while we are sorting, we will need to decode a route's path matcher
// in order to sub-sort by path length; we can amortize this operation
// for efficiency by storing the decoded matchers in a slice
decodedMatchers := make([]caddyhttp.MatchPath, len(routes))

sort.SliceStable(routes, func(i, j int) bool {
// if the directives are different, just use the established directive order
iDir, jDir := routes[i].directive, routes[j].directive
if iDir == jDir {
// directives are the same; sub-sort by path matcher length
// if there's only one matcher set and one path (common case)
iRoute, ok := routes[i].Value.(caddyhttp.Route)
if !ok {
return false
}
jRoute, ok := routes[j].Value.(caddyhttp.Route)
if !ok {
return false
}
if iDir != jDir {
return dirPositions[iDir] < dirPositions[jDir]
}

// use already-decoded matcher, or decode if it's the first time seeing it
iPM, jPM := decodedMatchers[i], decodedMatchers[j]
if iPM == nil && len(iRoute.MatcherSetsRaw) == 1 {
var pathMatcher caddyhttp.MatchPath
_ = json.Unmarshal(iRoute.MatcherSetsRaw[0]["path"], &pathMatcher)
decodedMatchers[i] = pathMatcher
iPM = pathMatcher
}
if jPM == nil && len(jRoute.MatcherSetsRaw) == 1 {
var pathMatcher caddyhttp.MatchPath
_ = json.Unmarshal(jRoute.MatcherSetsRaw[0]["path"], &pathMatcher)
decodedMatchers[j] = pathMatcher
jPM = pathMatcher
}
// directives are the same; sub-sort by path matcher length if there's
// only one matcher set and one path (this is a very common case and
// usually -- but not always -- helpful/expected, oh well; user can
// always take manual control of order using handler or route blocks)
iRoute, ok := routes[i].Value.(caddyhttp.Route)
if !ok {
return false
}
jRoute, ok := routes[j].Value.(caddyhttp.Route)
if !ok {
return false
}

// sort by longer path (more specific) first; missing
// path matchers are treated as zero-length paths
var iPathLen, jPathLen int
if iPM != nil {
iPathLen = len(iPM[0])
}
if jPM != nil {
jPathLen = len(jPM[0])
}
return iPathLen > jPathLen
// decode the path matchers, if there is just one of them
var iPM, jPM caddyhttp.MatchPath
if len(iRoute.MatcherSetsRaw) == 1 {
_ = json.Unmarshal(iRoute.MatcherSetsRaw[0]["path"], &iPM)
}
if len(jRoute.MatcherSetsRaw) == 1 {
_ = json.Unmarshal(jRoute.MatcherSetsRaw[0]["path"], &jPM)
}

return dirPositions[iDir] < dirPositions[jDir]
// sort by longer path (more specific) first; missing path
// matchers or multi-matchers are treated as zero-length paths
var iPathLen, jPathLen int
if len(iPM) > 0 {
iPathLen = len(iPM[0])
}
if len(jPM) > 0 {
jPathLen = len(jPM[0])
}
return iPathLen > jPathLen
})
}

Expand Down

0 comments on commit cd9317e

Please sign in to comment.