fix: prevent error handler from overriding sub handler matchers

[Hellio404]

Fixes: #6957

It make sure that all routes within the handle_errors are within a subroute.

[CLAassistant]

All committers have signed the CLA.

[mholt]

Thanks! Is this really the correct fix though? I imagine there's a JSON structure we can generate that isn't this deep that does it correctly, but I'm not super familiar with the issue right now to know what that would be.

[Hellio404]

I tried to have something that is similar to what the normal routes already have as per documentation

so I see it as something like this: (Not a real example)

@status_404 {
  status 404
}
handle @status_404 {
  handle /en/* { ... }
   handle /es/* { ... }
}

so the above code if we use Method matcher or any other matcher, it will result in the same depth as what this PR introduced

[Hellio404]

To summarize the issue, it is mainly due to the following line:

caddy/caddyconfig/httpcaddyfile/builtins.go

Lines 850 to 852 in 9f71483

	for i := range subroute.Routes {
	subroute.Routes[i].MatcherSetsRaw = []caddy.ModuleMap{statusMatcher}
	}

It was overriding the MatcherSetsRaw property for all handlers inside the handle_errors block.
Most of our tests used only direct static_response routes, which have no MatcherSetsRaw, so nothing broke.
However, once you add complex sub-routes inside, you encounter two issues:

1. Overriding MatcherSetsRaw.
2. The default handle_errors (with no status code specified) that contains inner handlers is not sorted correctly (we expect it to be moved to the bottom.)

correct case:

handle_errors { respond "error" }
handle_errors 404 {...}
----
[
    { <---- 404 was moved to the top because it has a status code
        "handle": [...],
        "match": [{ "expression": "{http.error.status_code} in [404]" }]
    },
    {
        "handle": [...]
    }
]

But if you have sub-routes inside the default handle_errors:

handle_errors 404 {...}
handle_errors { handle /es/* { respond "Error desconocido" } }
---
[
    { <---- The default was moved to the top because the sorting checks `Routes[0].MatcherSetsRaw`
        "handle": [...]
    },
    {
        "handle": [...],
        "match": [{ "expression": "{http.error.status_code} in [404]" }]
    }
]

[Involture]

Hi,
Thank you very much for writing the fix.
I need it as well. I hope it is reviewed soon.

[mholt]

Sounds good -- thank you for working on this. We'll give it a shot!