Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

httpcaddyfile: Fix sorting edgecase for nested handle_path #4477

Merged
merged 1 commit into from
Dec 13, 2021
Merged

httpcaddyfile: Fix sorting edgecase for nested handle_path #4477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions caddyconfig/httpcaddyfile/directives.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,9 @@ func parseSegmentAsConfig(h Helper) ([]ConfigValue, error) {
if err != nil {
return nil, h.Errf("parsing caddyfile tokens for '%s': %v", dir, err)
}

dir = normalizeDirectiveName(dir)

for _, result := range results {
result.directive = dir
allResults = append(allResults, result)
Expand Down
21 changes: 14 additions & 7 deletions caddyconfig/httpcaddyfile/httptype.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,7 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
return nil, warnings, fmt.Errorf("parsing caddyfile tokens for '%s': %v", dir, err)
}

// As a special case, we want "handle_path" to be sorted
// at the same level as "handle", so we force them to use
// the same directive name after their parsing is complete.
// See https://github.com/caddyserver/caddy/issues/3675#issuecomment-678042377
if dir == "handle_path" {
dir = "handle"
}
dir = normalizeDirectiveName(dir)

for _, result := range results {
result.directive = dir
Expand Down Expand Up @@ -1061,6 +1055,19 @@ func buildSubroute(routes []ConfigValue, groupCounter counter) (*caddyhttp.Subro
return subroute, nil
}

// normalizeDirectiveName ensures directives that should be sorted
// at the same level are named the same before sorting happens.
func normalizeDirectiveName(directive string) string {
// As a special case, we want "handle_path" to be sorted
// at the same level as "handle", so we force them to use
// the same directive name after their parsing is complete.
// See https://github.com/caddyserver/caddy/issues/3675#issuecomment-678042377
if directive == "handle_path" {
directive = "handle"
}
return directive
}

// consolidateRoutes combines routes with the same properties
// (same matchers, same Terminal and Group settings) for a
// cleaner overall output.
Expand Down
133 changes: 133 additions & 0 deletions caddytest/integration/caddyfile_adapt/sort_directives_within_handle.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
*.example.com {
@foo host foo.example.com
handle @foo {
handle_path /strip* {
respond "this should be first"
}
handle {
respond "this should be second"
}
}
handle {
respond "this should be last"
}
}
----------
{
"apps": {
"http": {
"servers": {
"srv0": {
"listen": [
":443"
],
"routes": [
{
"match": [
{
"host": [
"*.example.com"
]
}
],
"handle": [
{
"handler": "subroute",
"routes": [
{
"group": "group5",
"handle": [
{
"handler": "subroute",
"routes": [
{
"group": "group2",
"handle": [
{
"handler": "subroute",
"routes": [
{
"handle": [
{
"handler": "rewrite",
"strip_path_prefix": "/strip"
}
]
},
{
"handle": [
{
"body": "this should be first",
"handler": "static_response"
}
]
}
]
}
],
"match": [
{
"path": [
"/strip*"
]
}
]
},
{
"group": "group2",
"handle": [
{
"handler": "subroute",
"routes": [
{
"handle": [
{
"body": "this should be second",
"handler": "static_response"
}
]
}
]
}
]
}
]
}
],
"match": [
{
"host": [
"foo.example.com"
]
}
]
},
{
"group": "group5",
"handle": [
{
"handler": "subroute",
"routes": [
{
"handle": [
{
"body": "this should be last",
"handler": "static_response"
}
]
}
]
}
]
}
]
}
],
"terminal": true
}
]
}
}
}
}
}