summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrancis Lavoie <lavofr@gmail.com>2023-03-27 15:43:44 -0400
committerGitHub <noreply@github.com>2023-03-27 15:43:44 -0400
commit330be2d8c793147d3914f944eecb96c18f2eabff (patch)
tree87003a6af2df30d146cd0487657362489edd908e
parent0cc49c053f77bf6efa8107fa50d2e256a91d0ff8 (diff)
httpcaddyfile: Adjust path matcher sorting to solve for specificity (#5462)
-rw-r--r--caddyconfig/httpcaddyfile/directives.go33
-rw-r--r--caddytest/integration/caddyfile_adapt/map_and_vars_with_raw_types.txt8
-rw-r--r--caddytest/integration/caddyfile_adapt/sort_directives_within_handle.txt50
-rw-r--r--caddytest/integration/caddyfile_adapt/sort_vars_in_reverse.txt20
4 files changed, 82 insertions, 29 deletions
diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index a772dba..b8faa4a 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -427,26 +427,16 @@ func sortRoutes(routes []ConfigValue) {
jPathLen = len(jPM[0])
}
- // some directives involve setting values which can overwrite
- // 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" {
+ sortByPath := func() bool {
// we can only confidently compare path lengths if both
// directives have a single path to match (issue #5037)
if iPathLen > 0 && jPathLen > 0 {
- // sort least-specific (shortest) path first
- return iPathLen < jPathLen
- }
+ // if both paths are the same except for a trailing wildcard,
+ // 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
return iPathLen > jPathLen
}
@@ -455,7 +445,18 @@ func sortRoutes(routes []ConfigValue) {
// 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
+ }()
+
+ // 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
})
}
diff --git a/caddytest/integration/caddyfile_adapt/map_and_vars_with_raw_types.txt b/caddytest/integration/caddyfile_adapt/map_and_vars_with_raw_types.txt
index af9faf4..cc75630 100644
--- a/caddytest/integration/caddyfile_adapt/map_and_vars_with_raw_types.txt
+++ b/caddytest/integration/caddyfile_adapt/map_and_vars_with_raw_types.txt
@@ -101,15 +101,15 @@ vars {
"source": "{http.request.host}"
},
{
- "foo": "bar",
- "handler": "vars"
- },
- {
"abc": true,
"def": 1,
"ghi": 2.3,
"handler": "vars",
"jkl": "mn op"
+ },
+ {
+ "foo": "bar",
+ "handler": "vars"
}
]
}
diff --git a/caddytest/integration/caddyfile_adapt/sort_directives_within_handle.txt b/caddytest/integration/caddyfile_adapt/sort_directives_within_handle.txt
index 0cf9d88..ac0d53c 100644
--- a/caddytest/integration/caddyfile_adapt/sort_directives_within_handle.txt
+++ b/caddytest/integration/caddyfile_adapt/sort_directives_within_handle.txt
@@ -1,12 +1,15 @@
*.example.com {
@foo host foo.example.com
handle @foo {
- handle_path /strip* {
+ handle_path /strip {
respond "this should be first"
}
- handle {
+ handle_path /strip* {
respond "this should be second"
}
+ handle {
+ respond "this should be last"
+ }
}
handle {
respond "this should be last"
@@ -35,13 +38,13 @@
"handler": "subroute",
"routes": [
{
- "group": "group5",
+ "group": "group6",
"handle": [
{
"handler": "subroute",
"routes": [
{
- "group": "group2",
+ "group": "group3",
"handle": [
{
"handler": "subroute",
@@ -68,13 +71,13 @@
"match": [
{
"path": [
- "/strip*"
+ "/strip"
]
}
]
},
{
- "group": "group2",
+ "group": "group3",
"handle": [
{
"handler": "subroute",
@@ -82,6 +85,14 @@
{
"handle": [
{
+ "handler": "rewrite",
+ "strip_path_prefix": "/strip"
+ }
+ ]
+ },
+ {
+ "handle": [
+ {
"body": "this should be second",
"handler": "static_response"
}
@@ -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": [
{
"handler": "subroute",
diff --git a/caddytest/integration/caddyfile_adapt/sort_vars_in_reverse.txt b/caddytest/integration/caddyfile_adapt/sort_vars_in_reverse.txt
index dff75e1..38a912f 100644
--- a/caddytest/integration/caddyfile_adapt/sort_vars_in_reverse.txt
+++ b/caddytest/integration/caddyfile_adapt/sort_vars_in_reverse.txt
@@ -1,7 +1,8 @@
:80
vars /foobar foo last
-vars /foo foo middle
+vars /foo foo middle-last
+vars /foo* foo middle-first
vars * foo first
----------
{
@@ -25,13 +26,28 @@ vars * foo first
"match": [
{
"path": [
+ "/foo*"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "foo": "middle-first",
+ "handler": "vars"
+ }
+ ]
+ },
+ {
+ "match": [
+ {
+ "path": [
"/foo"
]
}
],
"handle": [
{
- "foo": "middle",
+ "foo": "middle-last",
"handler": "vars"
}
]