From 87a1f228b4c48db7bdab694cf0fd1c76243f29f3 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Tue, 1 Mar 2022 16:12:43 -0500 Subject: [PATCH] reverseproxy: Move status replacement intercept to `replace_status` (#4300) --- .../reverse_proxy_handle_response.txt | 22 +++--- modules/caddyhttp/reverseproxy/caddyfile.go | 79 +++++++++++-------- 2 files changed, 56 insertions(+), 45 deletions(-) diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt index 35d9631a..88ecbc21 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt @@ -1,6 +1,9 @@ :8884 reverse_proxy 127.0.0.1:65535 { + @changeStatus status 500 + replace_status @changeStatus 400 + @accel header X-Accel-Redirect * handle_response @accel { respond "Header X-Accel-Redirect!" @@ -38,9 +41,6 @@ reverse_proxy 127.0.0.1:65535 { handle_response @multi { respond "Headers Foo, Bar AND statuses 401, 403 and 404!" } - - @changeStatus status 500 - handle_response @changeStatus 400 } ---------- { @@ -56,6 +56,14 @@ reverse_proxy 127.0.0.1:65535 { "handle": [ { "handle_response": [ + { + "match": { + "status_code": [ + 500 + ] + }, + "status_code": 400 + }, { "match": { "headers": { @@ -155,14 +163,6 @@ reverse_proxy 127.0.0.1:65535 { } ] }, - { - "match": { - "status_code": [ - 500 - ] - }, - "status_code": 400 - }, { "routes": [ { diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 52282f7b..d642d04e 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -91,12 +91,13 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // ... // } // -// # handle responses +// # intercepting responses // @name { // status // header [] // } -// handle_response [] [status_code] { +// replace_status +// handle_response [] { // // } // } @@ -651,6 +652,39 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // See h.FinalizeUnmarshalCaddyfile h.handleResponseSegments = append(h.handleResponseSegments, d.NewFromNextSegment()) + case "replace_status": + args := d.RemainingArgs() + if len(args) != 2 { + return d.Errf("must have two arguments: a response matcher and a status code") + } + + if !strings.HasPrefix(args[0], matcherPrefix) { + return d.Errf("must use a named response matcher, starting with '@'") + } + + foundMatcher, ok := h.responseMatchers[args[0]] + if !ok { + return d.Errf("no named response matcher defined with name '%s'", args[0][1:]) + } + + _, err := strconv.Atoi(args[1]) + if err != nil { + return d.Errf("bad integer value '%s': %v", args[1], err) + } + + // make sure there's no block, cause it doesn't make sense + if d.NextBlock(1) { + return d.Errf("cannot define routes for 'replace_status', use 'handle_response' instead.") + } + + h.HandleResponse = append( + h.HandleResponse, + caddyhttp.ResponseHandler{ + Match: &foundMatcher, + StatusCode: caddyhttp.WeakString(args[1]), + }, + ) + default: return d.Errf("unrecognized subdirective %s", d.Val()) } @@ -699,20 +733,20 @@ func (h *Handler) FinalizeUnmarshalCaddyfile(helper httpcaddyfile.Helper) error for _, d := range h.handleResponseSegments { // consume the "handle_response" token d.Next() - - var matcher *caddyhttp.ResponseMatcher args := d.RemainingArgs() - // the first arg should be a matcher (optional) - // the second arg should be a status code (optional) - // any more than that isn't currently supported - if len(args) > 2 { + // TODO: Remove this check at some point in the future + if len(args) == 2 { + return d.Errf("configuring 'handle_response' for status code replacement is no longer supported. Use 'replace_status' instead.") + } + + if len(args) > 1 { return d.Errf("too many arguments for 'handle_response': %s", args) } - // the first arg should always be a matcher. - // it doesn't really make sense to support status code without a matcher. - if len(args) > 0 { + var matcher *caddyhttp.ResponseMatcher + if len(args) == 1 { + // the first arg should always be a matcher. if !strings.HasPrefix(args[0], matcherPrefix) { return d.Errf("must use a named response matcher, starting with '@'") } @@ -724,29 +758,6 @@ func (h *Handler) FinalizeUnmarshalCaddyfile(helper httpcaddyfile.Helper) error matcher = &foundMatcher } - // a second arg should be a status code, in which case - // we skip parsing the block for routes - if len(args) == 2 { - _, err := strconv.Atoi(args[1]) - if err != nil { - return d.Errf("bad integer value '%s': %v", args[1], err) - } - - // make sure there's no block, cause it doesn't make sense - if d.NextBlock(1) { - return d.Errf("cannot define routes for 'handle_response' when changing the status code") - } - - h.HandleResponse = append( - h.HandleResponse, - caddyhttp.ResponseHandler{ - Match: matcher, - StatusCode: caddyhttp.WeakString(args[1]), - }, - ) - continue - } - // parse the block as routes handler, err := httpcaddyfile.ParseSegmentAsSubroute(helper.WithDispenser(d.NewFromNextSegment())) if err != nil {