httpcaddyfile: Adjust path matcher sorting to solve for specificity (#5462)

This commit is contained in:
Francis Lavoie 2023-03-27 15:43:44 -04:00 committed by GitHub
parent 0cc49c053f
commit 330be2d8c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 82 additions and 29 deletions

View File

@ -427,26 +427,16 @@ func sortRoutes(routes []ConfigValue) {
jPathLen = len(jPM[0]) jPathLen = len(jPM[0])
} }
// some directives involve setting values which can overwrite sortByPath := func() bool {
// each other, so it makes most sense to reverse the order so
// that the lease specific matcher is first; everything else
// has most-specific matcher first
if iDir == "vars" {
// we can only confidently compare path lengths if both // we can only confidently compare path lengths if both
// directives have a single path to match (issue #5037) // directives have a single path to match (issue #5037)
if iPathLen > 0 && jPathLen > 0 { if iPathLen > 0 && jPathLen > 0 {
// sort least-specific (shortest) path first // if both paths are the same except for a trailing wildcard,
return iPathLen < jPathLen // sort by the shorter path first (which is more specific)
} if strings.TrimSuffix(iPM[0], "*") == strings.TrimSuffix(jPM[0], "*") {
return iPathLen < jPathLen
}
// 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 // sort most-specific (longest) path first
return iPathLen > jPathLen return iPathLen > jPathLen
} }
@ -455,7 +445,18 @@ func sortRoutes(routes []ConfigValue) {
// sort whichever one has a matcher first; if both have // sort whichever one has a matcher first; if both have
// a matcher, sort equally (stable sort preserves order) // a matcher, sort equally (stable sort preserves order)
return len(iRoute.MatcherSetsRaw) > 0 && len(jRoute.MatcherSetsRaw) == 0 return len(iRoute.MatcherSetsRaw) > 0 && len(jRoute.MatcherSetsRaw) == 0
}()
// some directives involve setting values which can overwrite
// each other, so it makes most sense to reverse the order so
// that the least-specific matcher is first, allowing the last
// matching one to win
if iDir == "vars" {
return !sortByPath
} }
// everything else is most-specific matcher first
return sortByPath
}) })
} }

View File

@ -100,16 +100,16 @@ vars {
], ],
"source": "{http.request.host}" "source": "{http.request.host}"
}, },
{
"foo": "bar",
"handler": "vars"
},
{ {
"abc": true, "abc": true,
"def": 1, "def": 1,
"ghi": 2.3, "ghi": 2.3,
"handler": "vars", "handler": "vars",
"jkl": "mn op" "jkl": "mn op"
},
{
"foo": "bar",
"handler": "vars"
} }
] ]
} }

View File

@ -1,12 +1,15 @@
*.example.com { *.example.com {
@foo host foo.example.com @foo host foo.example.com
handle @foo { handle @foo {
handle_path /strip* { handle_path /strip {
respond "this should be first" respond "this should be first"
} }
handle { handle_path /strip* {
respond "this should be second" respond "this should be second"
} }
handle {
respond "this should be last"
}
} }
handle { handle {
respond "this should be last" respond "this should be last"
@ -35,13 +38,13 @@
"handler": "subroute", "handler": "subroute",
"routes": [ "routes": [
{ {
"group": "group5", "group": "group6",
"handle": [ "handle": [
{ {
"handler": "subroute", "handler": "subroute",
"routes": [ "routes": [
{ {
"group": "group2", "group": "group3",
"handle": [ "handle": [
{ {
"handler": "subroute", "handler": "subroute",
@ -68,17 +71,25 @@
"match": [ "match": [
{ {
"path": [ "path": [
"/strip*" "/strip"
] ]
} }
] ]
}, },
{ {
"group": "group2", "group": "group3",
"handle": [ "handle": [
{ {
"handler": "subroute", "handler": "subroute",
"routes": [ "routes": [
{
"handle": [
{
"handler": "rewrite",
"strip_path_prefix": "/strip"
}
]
},
{ {
"handle": [ "handle": [
{ {
@ -89,6 +100,31 @@
} }
] ]
} }
],
"match": [
{
"path": [
"/strip*"
]
}
]
},
{
"group": "group3",
"handle": [
{
"handler": "subroute",
"routes": [
{
"handle": [
{
"body": "this should be last",
"handler": "static_response"
}
]
}
]
}
] ]
} }
] ]
@ -103,7 +139,7 @@
] ]
}, },
{ {
"group": "group5", "group": "group6",
"handle": [ "handle": [
{ {
"handler": "subroute", "handler": "subroute",

View File

@ -1,7 +1,8 @@
:80 :80
vars /foobar foo last vars /foobar foo last
vars /foo foo middle vars /foo foo middle-last
vars /foo* foo middle-first
vars * foo first vars * foo first
---------- ----------
{ {
@ -21,6 +22,21 @@ vars * foo first
} }
] ]
}, },
{
"match": [
{
"path": [
"/foo*"
]
}
],
"handle": [
{
"foo": "middle-first",
"handler": "vars"
}
]
},
{ {
"match": [ "match": [
{ {
@ -31,7 +47,7 @@ vars * foo first
], ],
"handle": [ "handle": [
{ {
"foo": "middle", "foo": "middle-last",
"handler": "vars" "handler": "vars"
} }
] ]