From 092913a7a568a5eb4b28c06e12c1274bd5eb1140 Mon Sep 17 00:00:00 2001 From: Youness Farini Date: Fri, 6 Jun 2025 18:46:39 +0100 Subject: [PATCH] httpcaddyfile: Prevent error handler from overriding sub-handler matchers (#6999) Fixes: #6957 --- caddyconfig/httpcaddyfile/builtins.go | 12 +- .../error_example.caddyfiletest | 33 ++- .../error_multi_site_blocks.caddyfiletest | 52 +++- .../error_range_codes.caddyfiletest | 13 +- .../error_range_simple_codes.caddyfiletest | 26 +- .../error_simple_codes.caddyfiletest | 13 +- .../caddyfile_adapt/error_sort.caddyfiletest | 26 +- .../error_subhandlers.caddyfiletest | 260 ++++++++++++++++++ caddytest/integration/caddyfile_test.go | 40 +++ 9 files changed, 440 insertions(+), 35 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/error_subhandlers.caddyfiletest diff --git a/caddyconfig/httpcaddyfile/builtins.go b/caddyconfig/httpcaddyfile/builtins.go index 3fc08b2c8..d41ef413d 100644 --- a/caddyconfig/httpcaddyfile/builtins.go +++ b/caddyconfig/httpcaddyfile/builtins.go @@ -15,6 +15,7 @@ package httpcaddyfile import ( + "encoding/json" "fmt" "html" "net/http" @@ -843,13 +844,18 @@ func parseHandleErrors(h Helper) ([]ConfigValue, error) { return nil, h.Errf("segment was not parsed as a subroute") } + // wrap the subroutes + wrappingRoute := caddyhttp.Route{ + HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject(subroute, "handler", "subroute", nil)}, + } + subroute = &caddyhttp.Subroute{ + Routes: []caddyhttp.Route{wrappingRoute}, + } if expression != "" { statusMatcher := caddy.ModuleMap{ "expression": h.JSON(caddyhttp.MatchExpression{Expr: expression}), } - for i := range subroute.Routes { - subroute.Routes[i].MatcherSetsRaw = []caddy.ModuleMap{statusMatcher} - } + subroute.Routes[0].MatcherSetsRaw = []caddy.ModuleMap{statusMatcher} } return []ConfigValue{ { diff --git a/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest index bd42aee55..6f3059ab2 100644 --- a/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_example.caddyfiletest @@ -106,20 +106,29 @@ example.com { "handler": "subroute", "routes": [ { - "group": "group0", "handle": [ { - "handler": "rewrite", - "uri": "/{http.error.status_code}.html" - } - ] - }, - { - "handle": [ - { - "handler": "file_server", - "hide": [ - "./Caddyfile" + "handler": "subroute", + "routes": [ + { + "group": "group0", + "handle": [ + { + "handler": "rewrite", + "uri": "/{http.error.status_code}.html" + } + ] + }, + { + "handle": [ + { + "handler": "file_server", + "hide": [ + "./Caddyfile" + ] + } + ] + } ] } ] diff --git a/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest index 0e84a13c2..1bec4b3e8 100644 --- a/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_multi_site_blocks.caddyfiletest @@ -165,8 +165,17 @@ bar.localhost { { "handle": [ { - "body": "404 or 410 error", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "404 or 410 error", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -178,8 +187,17 @@ bar.localhost { { "handle": [ { - "body": "Error In range [500 .. 599]", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error In range [500 .. 599]", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -208,8 +226,17 @@ bar.localhost { { "handle": [ { - "body": "404 or 410 error from second site", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "404 or 410 error from second site", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -221,8 +248,17 @@ bar.localhost { { "handle": [ { - "body": "Error In range [500 .. 599] from second site", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error In range [500 .. 599] from second site", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest index 46b70c8e3..299abc5f1 100644 --- a/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_range_codes.caddyfiletest @@ -96,8 +96,17 @@ localhost:3010 { { "handle": [ { - "body": "Error in the [400 .. 499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error in the [400 .. 499] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest index 70158830c..9d4d2645c 100644 --- a/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_range_simple_codes.caddyfiletest @@ -116,8 +116,17 @@ localhost:2099 { { "handle": [ { - "body": "Error in the [400 .. 499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error in the [400 .. 499] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -129,8 +138,17 @@ localhost:2099 { { "handle": [ { - "body": "Error code is equal to 500 or in the [300..399] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error code is equal to 500 or in the [300..399] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest index 5ac5863e3..29a51ffad 100644 --- a/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_simple_codes.caddyfiletest @@ -96,8 +96,17 @@ localhost:3010 { { "handle": [ { - "body": "404 or 410 error", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "404 or 410 error", + "handler": "static_response" + } + ] + } + ] } ], "match": [ diff --git a/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest index 63701cccb..1faf9b3bd 100644 --- a/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/error_sort.caddyfiletest @@ -116,8 +116,17 @@ localhost:2099 { { "handle": [ { - "body": "Error in the [400 .. 499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Error in the [400 .. 499] range", + "handler": "static_response" + } + ] + } + ] } ], "match": [ @@ -129,8 +138,17 @@ localhost:2099 { { "handle": [ { - "body": "Fallback route: code outside the [400..499] range", - "handler": "static_response" + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Fallback route: code outside the [400..499] range", + "handler": "static_response" + } + ] + } + ] } ] } diff --git a/caddytest/integration/caddyfile_adapt/error_subhandlers.caddyfiletest b/caddytest/integration/caddyfile_adapt/error_subhandlers.caddyfiletest new file mode 100644 index 000000000..54429a73e --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/error_subhandlers.caddyfiletest @@ -0,0 +1,260 @@ +{ + http_port 2099 +} +localhost:2099 { + root * /var/www/ + file_server + + handle_errors 404 { + handle /en/* { + respond "not found" 404 + } + handle /es/* { + respond "no encontrado" + } + handle { + respond "default not found" + } + } + handle_errors { + handle /en/* { + respond "English error" + } + handle /es/* { + respond "Spanish error" + } + handle { + respond "Default error" + } + } +} +---------- +{ + "apps": { + "http": { + "http_port": 2099, + "servers": { + "srv0": { + "listen": [ + ":2099" + ], + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "vars", + "root": "/var/www/" + }, + { + "handler": "file_server", + "hide": [ + "./Caddyfile" + ] + } + ] + } + ] + } + ], + "terminal": true + } + ], + "errors": { + "routes": [ + { + "match": [ + { + "host": [ + "localhost" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "group": "group3", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "not found", + "handler": "static_response", + "status_code": 404 + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/en/*" + ] + } + ] + }, + { + "group": "group3", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "no encontrado", + "handler": "static_response" + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/es/*" + ] + } + ] + }, + { + "group": "group3", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "default not found", + "handler": "static_response" + } + ] + } + ] + } + ] + } + ] + } + ], + "match": [ + { + "expression": "{http.error.status_code} in [404]" + } + ] + }, + { + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "group": "group8", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "English error", + "handler": "static_response" + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/en/*" + ] + } + ] + }, + { + "group": "group8", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Spanish error", + "handler": "static_response" + } + ] + } + ] + } + ], + "match": [ + { + "path": [ + "/es/*" + ] + } + ] + }, + { + "group": "group8", + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "Default error", + "handler": "static_response" + } + ] + } + ] + } + ] + } + ] + } + ] + } + ] + } + ], + "terminal": true + } + ] + } + } + } + } + } +} + diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go index b340eb8c5..d45d5a5e9 100644 --- a/caddytest/integration/caddyfile_test.go +++ b/caddytest/integration/caddyfile_test.go @@ -782,6 +782,46 @@ func TestHandleErrorRangeAndCodes(t *testing.T) { tester.AssertGetResponse("http://localhost:9080/private", 410, "Error in the [400 .. 499] range") } +func TestHandleErrorSubHandlers(t *testing.T) { + tester := caddytest.NewTester(t) + tester.InitServer(`{ + admin localhost:2999 + http_port 9080 + } + localhost:9080 { + root * /srv + file_server + error /*/internalerr* "Internal Server Error" 500 + + handle_errors 404 { + handle /en/* { + respond "not found" 404 + } + handle /es/* { + respond "no encontrado" 404 + } + handle { + respond "default not found" + } + } + handle_errors { + handle { + respond "Default error" + } + handle /en/* { + respond "English error" + } + } + } + `, "caddyfile") + // act and assert + tester.AssertGetResponse("http://localhost:9080/en/notfound", 404, "not found") + tester.AssertGetResponse("http://localhost:9080/es/notfound", 404, "no encontrado") + tester.AssertGetResponse("http://localhost:9080/notfound", 404, "default not found") + tester.AssertGetResponse("http://localhost:9080/es/internalerr", 500, "Default error") + tester.AssertGetResponse("http://localhost:9080/en/internalerr", 500, "English error") +} + func TestInvalidSiteAddressesAsDirectives(t *testing.T) { type testCase struct { config, expectedError string