From f58efda04ad5cadf4f62f3276b5d336d5c3ccb36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20Soul=C3=A9?= Date: Sun, 6 Sep 2020 18:07:46 +0200 Subject: [PATCH 1/9] reverseproxy: Add `handle_response` blocks to `reverse_proxy` (#3710) --- caddyconfig/httpcaddyfile/directives.go | 7 ++ .../reverse_proxy_handle_response.txt | 90 +++++++++++++++++++ modules/caddyhttp/reverseproxy/caddyfile.go | 53 ++++++++++- .../reverseproxy/fastcgi/caddyfile.go | 2 +- 4 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go index b4a8407d658..5e194741a6f 100644 --- a/caddyconfig/httpcaddyfile/directives.go +++ b/caddyconfig/httpcaddyfile/directives.go @@ -265,6 +265,13 @@ func (h Helper) NewBindAddresses(addrs []string) []ConfigValue { return []ConfigValue{{Class: "bind", Value: addrs}} } +// WithDispenser returns a new instance based on d. All others Helper +// fields are copied, so typically maps are shared with this new instance. +func (h Helper) WithDispenser(d *caddyfile.Dispenser) Helper { + h.Dispenser = d + return h +} + // ParseSegmentAsSubroute parses the segment such that its subdirectives // are themselves treated as directives, from which a subroute is built // and returned. diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt new file mode 100644 index 00000000000..7fee0bdf9de --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt @@ -0,0 +1,90 @@ +:8884 + +reverse_proxy 127.0.0.1:65535 { + handle_response header X-Accel-Redirect { + respond "Header!" + } + handle_response status 401 { + respond "Status!" + } + handle_response { + respond "Any!" + } +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":8884" + ], + "routes": [ + { + "handle": [ + { + "handle_response": [ + { + "match": { + "headers": { + "X-Accel-Redirect": [] + } + }, + "routes": [ + { + "handle": [ + { + "body": "Header!", + "handler": "static_response" + } + ] + } + ] + }, + { + "match": { + "status_code": [ + 401 + ] + }, + "routes": [ + { + "handle": [ + { + "body": "Status!", + "handler": "static_response" + } + ] + } + ] + }, + { + "match": {}, + "routes": [ + { + "handle": [ + { + "body": "Any!", + "handler": "static_response" + } + ] + } + ] + } + ], + "handler": "reverse_proxy", + "upstreams": [ + { + "dial": "127.0.0.1:65535" + } + ] + } + ] + } + ] + } + } + } + } +} diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index dbadef600f6..ef366de2cb1 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -38,14 +38,14 @@ func init() { func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { rp := new(Handler) - err := rp.UnmarshalCaddyfile(h.Dispenser) + err := rp.ParseCaddyfileReverseProxy(h) if err != nil { return nil, err } return rp, nil } -// UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax: +// ParseCaddyfileReverseProxy sets up the handler from Caddyfile tokens. Syntax: // // reverse_proxy [] [] { // # upstreams @@ -86,13 +86,20 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // transport { // ... // } +// +// # handle responses +// handle_response [header |status ] { +// ... +// } // } // // Proxy upstream addresses should be network dial addresses such // as `host:port`, or a URL such as `scheme://host:port`. Scheme // and port may be inferred from other parts of the address/URL; if // either are missing, defaults to HTTP. -func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { +func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error { + d := helper.Dispenser + // currently, all backends must use the same scheme/protocol (the // underlying JSON does not yet support per-backend transports) var commonScheme string @@ -617,6 +624,45 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } transport = rt + case "handle_response": + var rm caddyhttp.ResponseMatcher + args := d.RemainingArgs() + switch len(args) { + case 2: + switch args[0] { + case "header": + rm = caddyhttp.ResponseMatcher{ + Headers: http.Header{args[1]: []string{}}, + } + case "status": + statusNum, err := strconv.Atoi(args[1]) + if err != nil { + return d.Errf("bad status value '%s': %v", args[1], err) + } + rm = caddyhttp.ResponseMatcher{ + StatusCode: []int{statusNum}, + } + default: + return d.Err("handle_response only accepts header|status") + } + case 0: // Any status or header + default: + return d.ArgErr() + } + handler, err := httpcaddyfile.ParseSegmentAsSubroute(helper.WithDispenser(d.NewFromNextSegment())) + if err != nil { + return err + } + subroute, ok := handler.(*caddyhttp.Subroute) + if !ok { + return helper.Errf("segment was not parsed as a subroute") + } + h.HandleResponse = append(h.HandleResponse, + caddyhttp.ResponseHandler{ + Match: &rm, + Routes: subroute.Routes, + }) + default: return d.Errf("unrecognized subdirective %s", d.Val()) } @@ -894,6 +940,5 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // Interface guards var ( - _ caddyfile.Unmarshaler = (*Handler)(nil) _ caddyfile.Unmarshaler = (*HTTPTransport)(nil) ) diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 4d0b23b29d9..486fdcf2d65 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -355,7 +355,7 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error // using the reverse_proxy directive syntax // TODO: this can overwrite our fcgiTransport that we encoded and // set on the rpHandler... even with a non-fastcgi transport! - err = rpHandler.UnmarshalCaddyfile(dispenser) + err = rpHandler.ParseCaddyfileReverseProxy(h.WithDispenser(dispenser)) if err != nil { return nil, err } From f24b67d177de1ab81e39d642ee94f31d4a819fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20Soul=C3=A9?= Date: Sat, 19 Sep 2020 12:35:45 +0200 Subject: [PATCH 2/9] reverseproxy: complete handle_response test --- .../reverse_proxy_handle_response.txt | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt index 7fee0bdf9de..7085c7e81be 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt @@ -2,10 +2,16 @@ reverse_proxy 127.0.0.1:65535 { handle_response header X-Accel-Redirect { - respond "Header!" + respond "Header X-Accel-Redirect!" + } + handle_response header X-Another { + respond "Header X-Another!" } handle_response status 401 { - respond "Status!" + respond "Status 401!" + } + handle_response status 403 { + respond "Status 403!" } handle_response { respond "Any!" @@ -35,7 +41,24 @@ reverse_proxy 127.0.0.1:65535 { { "handle": [ { - "body": "Header!", + "body": "Header X-Accel-Redirect!", + "handler": "static_response" + } + ] + } + ] + }, + { + "match": { + "headers": { + "X-Another": [] + } + }, + "routes": [ + { + "handle": [ + { + "body": "Header X-Another!", "handler": "static_response" } ] @@ -52,7 +75,24 @@ reverse_proxy 127.0.0.1:65535 { { "handle": [ { - "body": "Status!", + "body": "Status 401!", + "handler": "static_response" + } + ] + } + ] + }, + { + "match": { + "status_code": [ + 403 + ] + }, + "routes": [ + { + "handle": [ + { + "body": "Status 403!", "handler": "static_response" } ] From 56647994b92a4695108434e649653679ca55abc6 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 15 Feb 2021 22:10:39 -0500 Subject: [PATCH 3/9] reverseproxy: Change handle_response matchers to use named matchers reverseproxy: Add support for changing status code --- .../reverse_proxy_handle_response.txt | 83 ++++++++- modules/caddyhttp/reverseproxy/caddyfile.go | 175 +++++++++++++++--- 2 files changed, 222 insertions(+), 36 deletions(-) diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt index 7085c7e81be..35d9631a78c 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_handle_response.txt @@ -1,21 +1,46 @@ :8884 reverse_proxy 127.0.0.1:65535 { - handle_response header X-Accel-Redirect { + @accel header X-Accel-Redirect * + handle_response @accel { respond "Header X-Accel-Redirect!" } - handle_response header X-Another { + + @another { + header X-Another * + } + handle_response @another { respond "Header X-Another!" } - handle_response status 401 { + + @401 status 401 + handle_response @401 { respond "Status 401!" } - handle_response status 403 { + + handle_response { + respond "Any! This should be last in the JSON!" + } + + @403 { + status 403 + } + handle_response @403 { respond "Status 403!" } - handle_response { - respond "Any!" + + @multi { + status 401 403 + status 404 + header Foo * + header Bar * } + handle_response @multi { + respond "Headers Foo, Bar AND statuses 401, 403 and 404!" + } + + @changeStatus status 500 + handle_response @changeStatus 400 } ---------- { @@ -34,7 +59,9 @@ reverse_proxy 127.0.0.1:65535 { { "match": { "headers": { - "X-Accel-Redirect": [] + "X-Accel-Redirect": [ + "*" + ] } }, "routes": [ @@ -51,7 +78,9 @@ reverse_proxy 127.0.0.1:65535 { { "match": { "headers": { - "X-Another": [] + "X-Another": [ + "*" + ] } }, "routes": [ @@ -100,12 +129,46 @@ reverse_proxy 127.0.0.1:65535 { ] }, { - "match": {}, + "match": { + "headers": { + "Bar": [ + "*" + ], + "Foo": [ + "*" + ] + }, + "status_code": [ + 401, + 403, + 404 + ] + }, + "routes": [ + { + "handle": [ + { + "body": "Headers Foo, Bar AND statuses 401, 403 and 404!", + "handler": "static_response" + } + ] + } + ] + }, + { + "match": { + "status_code": [ + 500 + ] + }, + "status_code": 400 + }, + { "routes": [ { "handle": [ { - "body": "Any!", + "body": "Any! This should be last in the JSON!", "handler": "static_response" } ] diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index ef366de2cb1..1592a5aa00d 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -88,8 +88,12 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // } // // # handle responses -// handle_response [header |status ] { -// ... +// @name { +// status +// header [] +// } +// handle_response [] [status_code] { +// // } // } // @@ -109,6 +113,10 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error var transport http.RoundTripper var transportModuleName string + // collect the response matchers defined as subdirectives prefixed with "@" + // for use with "handle_response" blocks + responseMatchers := map[string]caddyhttp.ResponseMatcher{} + // TODO: the logic in this function is kind of sensitive, we need // to write tests before making any more changes to it upstreamDialAddress := func(upstreamAddr string) (string, error) { @@ -234,6 +242,16 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error } for d.NextBlock(0) { + // if the subdirective has an "@" prefix then we + // parse it as a response matcher for use with "handle_response" + if strings.HasPrefix(d.Val(), matcherPrefix) { + err := h.parseNamedResponseMatcher(d.NewFromNextSegment(), responseMatchers) + if err != nil { + return err + } + continue + } + switch d.Val() { case "to": args := d.RemainingArgs() @@ -625,30 +643,54 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error transport = rt case "handle_response": - var rm caddyhttp.ResponseMatcher + var matcher *caddyhttp.ResponseMatcher args := d.RemainingArgs() - switch len(args) { - case 2: - switch args[0] { - case "header": - rm = caddyhttp.ResponseMatcher{ - Headers: http.Header{args[1]: []string{}}, - } - case "status": - statusNum, err := strconv.Atoi(args[1]) - if err != nil { - return d.Errf("bad status value '%s': %v", args[1], err) - } - rm = caddyhttp.ResponseMatcher{ - StatusCode: []int{statusNum}, - } - default: - return d.Err("handle_response only accepts header|status") + + // 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 { + 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 { + if !strings.HasPrefix(args[0], matcherPrefix) { + return d.Errf("must use a named response matcher, starting with '@'") } - case 0: // Any status or header - default: - return d.ArgErr() + + foundMatcher, ok := responseMatchers[args[0]] + if !ok { + return d.Errf("no named response matcher defined with name '%s'", args[0][1:]) + } + 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 { return err @@ -657,11 +699,13 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error if !ok { return helper.Errf("segment was not parsed as a subroute") } - h.HandleResponse = append(h.HandleResponse, + h.HandleResponse = append( + h.HandleResponse, caddyhttp.ResponseHandler{ - Match: &rm, + Match: matcher, Routes: subroute.Routes, - }) + }, + ) default: return d.Errf("unrecognized subdirective %s", d.Val()) @@ -702,6 +746,21 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error } } + // move the handle_response entries without a matcher to the end. + // we can't use sort.SliceStable because it will reorder the rest of the + // entries which may be undesirable because we don't have a good + // heuristic to use for sorting. + withoutMatchers := []caddyhttp.ResponseHandler{} + withMatchers := []caddyhttp.ResponseHandler{} + for _, hr := range h.HandleResponse { + if hr.Match == nil { + withoutMatchers = append(withoutMatchers, hr) + } else { + withMatchers = append(withMatchers, hr) + } + } + h.HandleResponse = append(withMatchers, withoutMatchers...) + return nil } @@ -938,6 +997,70 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } +// Parse the tokens of a named response matcher. +// +// @name { +// header [] +// status +// } +// +// Or, single line syntax: +// +// @name [header []] | [status ] +// +func (h *Handler) parseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map[string]caddyhttp.ResponseMatcher) error { + for d.Next() { + definitionName := d.Val() + + if _, ok := matchers[definitionName]; ok { + return d.Errf("matcher is defined more than once: %s", definitionName) + } + + matcher := caddyhttp.ResponseMatcher{} + for nesting := d.Nesting(); d.NextArg() || d.NextBlock(nesting); { + switch d.Val() { + case "header": + if matcher.Headers == nil { + matcher.Headers = http.Header{} + } + + // reuse the header request matcher's unmarshaler + headerMatcher := caddyhttp.MatchHeader(matcher.Headers) + err := headerMatcher.UnmarshalCaddyfile(d.NewFromNextSegment()) + if err != nil { + return err + } + + matcher.Headers = http.Header(headerMatcher) + case "status": + if matcher.StatusCode == nil { + matcher.StatusCode = []int{} + } + + args := d.RemainingArgs() + if len(args) == 0 { + return d.ArgErr() + } + + for _, arg := range args { + statusNum, err := strconv.Atoi(arg) + if err != nil { + return d.Errf("bad status value '%s': %v", arg, err) + } + matcher.StatusCode = append(matcher.StatusCode, statusNum) + } + default: + return d.Errf("unrecognized response matcher %s", d.Val()) + } + } + + matchers[definitionName] = matcher + } + return nil +} + +const matcherPrefix = "@" + // Interface guards var ( _ caddyfile.Unmarshaler = (*HTTPTransport)(nil) From a59ea1c05ac534ae0c35fb2c767833c7220f6f3f Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 15 Feb 2021 21:06:56 -0500 Subject: [PATCH 4/9] fastcgi: Remove obsolete TODO We already have d.Err("transport already specified") in the reverse_proxy parsing code which covers this case --- modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 486fdcf2d65..8ae170d921b 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -353,8 +353,6 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error // the rest of the config is specified by the user // using the reverse_proxy directive syntax - // TODO: this can overwrite our fcgiTransport that we encoded and - // set on the rpHandler... even with a non-fastcgi transport! err = rpHandler.ParseCaddyfileReverseProxy(h.WithDispenser(dispenser)) if err != nil { return nil, err From 696c75aa4619d1c0df37c92b133292b2d26bdd5a Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 17 Feb 2021 19:20:37 -0500 Subject: [PATCH 5/9] reverseproxy: Fix support for "4xx" type status codes --- modules/caddyhttp/reverseproxy/caddyfile.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 1592a5aa00d..2835a1d2e4e 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -1032,6 +1032,7 @@ func (h *Handler) parseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map } matcher.Headers = http.Header(headerMatcher) + case "status": if matcher.StatusCode == nil { matcher.StatusCode = []int{} @@ -1043,12 +1044,16 @@ func (h *Handler) parseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map } for _, arg := range args { + if len(arg) == 3 && strings.HasSuffix(arg, "xx") { + arg = arg[:1] + } statusNum, err := strconv.Atoi(arg) if err != nil { return d.Errf("bad status value '%s': %v", arg, err) } matcher.StatusCode = append(matcher.StatusCode, statusNum) } + default: return d.Errf("unrecognized response matcher %s", d.Val()) } From 677ec82898c40ed32b623af44e017d798d406d22 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 29 Apr 2021 17:54:03 -0400 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: Matt Holt --- modules/caddyhttp/reverseproxy/caddyfile.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 2835a1d2e4e..4afd61ef0d8 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -115,7 +115,7 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error // collect the response matchers defined as subdirectives prefixed with "@" // for use with "handle_response" blocks - responseMatchers := map[string]caddyhttp.ResponseMatcher{} + responseMatchers := make(map[string]caddyhttp.ResponseMatcher) // TODO: the logic in this function is kind of sensitive, we need // to write tests before making any more changes to it @@ -1008,7 +1008,7 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // // @name [header []] | [status ] // -func (h *Handler) parseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map[string]caddyhttp.ResponseMatcher) error { +func (Handler) parseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map[string]caddyhttp.ResponseMatcher) error { for d.Next() { definitionName := d.Val() From 599709df423608cc7ded1fbf554b6e311cac47ef Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Thu, 29 Apr 2021 18:17:33 -0400 Subject: [PATCH 7/9] caddyhttp: Reorganize response matchers --- modules/caddyhttp/encode/caddyfile.go | 69 +------- modules/caddyhttp/matchers.go | 34 ---- modules/caddyhttp/matchers_test.go | 149 ----------------- modules/caddyhttp/responsematchers.go | 122 ++++++++++++++ modules/caddyhttp/responsematchers_test.go | 169 ++++++++++++++++++++ modules/caddyhttp/reverseproxy/caddyfile.go | 69 +------- 6 files changed, 293 insertions(+), 319 deletions(-) create mode 100644 modules/caddyhttp/responsematchers.go create mode 100644 modules/caddyhttp/responsematchers_test.go diff --git a/modules/caddyhttp/encode/caddyfile.go b/modules/caddyhttp/encode/caddyfile.go index c45f159e66c..b236520e70c 100644 --- a/modules/caddyhttp/encode/caddyfile.go +++ b/modules/caddyhttp/encode/caddyfile.go @@ -15,9 +15,7 @@ package encode import ( - "net/http" "strconv" - "strings" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig" @@ -95,7 +93,7 @@ func (enc *Encode) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } enc.Prefer = encs case "match": - err := enc.parseNamedResponseMatcher(d.NewFromNextSegment(), responseMatchers) + err := caddyhttp.ParseNamedResponseMatcher(d.NewFromNextSegment(), responseMatchers) if err != nil { return err } @@ -123,70 +121,5 @@ func (enc *Encode) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } -// Parse the tokens of a named response matcher. -// -// match { -// header [] -// status -// } -// -// Or, single line syntax: -// -// match [header []] | [status ] -// -func (enc *Encode) parseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map[string]caddyhttp.ResponseMatcher) error { - for d.Next() { - definitionName := d.Val() - - if _, ok := matchers[definitionName]; ok { - return d.Errf("matcher is defined more than once: %s", definitionName) - } - - matcher := caddyhttp.ResponseMatcher{} - for nesting := d.Nesting(); d.NextArg() || d.NextBlock(nesting); { - switch d.Val() { - case "header": - if matcher.Headers == nil { - matcher.Headers = http.Header{} - } - - // reuse the header request matcher's unmarshaler - headerMatcher := caddyhttp.MatchHeader(matcher.Headers) - err := headerMatcher.UnmarshalCaddyfile(d.NewFromNextSegment()) - if err != nil { - return err - } - - matcher.Headers = http.Header(headerMatcher) - case "status": - if matcher.StatusCode == nil { - matcher.StatusCode = []int{} - } - - args := d.RemainingArgs() - if len(args) == 0 { - return d.ArgErr() - } - - for _, arg := range args { - if len(arg) == 3 && strings.HasSuffix(arg, "xx") { - arg = arg[:1] - } - statusNum, err := strconv.Atoi(arg) - if err != nil { - return d.Errf("bad status value '%s': %v", arg, err) - } - matcher.StatusCode = append(matcher.StatusCode, statusNum) - } - default: - return d.Errf("unrecognized response matcher %s", d.Val()) - } - } - - matchers[definitionName] = matcher - } - return nil -} - // Interface guard var _ caddyfile.Unmarshaler = (*Encode)(nil) diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index eaf43e9a329..9b127dbc216 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -971,40 +971,6 @@ func (mre *MatchRegexp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } -// ResponseMatcher is a type which can determine if an -// HTTP response matches some criteria. -type ResponseMatcher struct { - // If set, one of these status codes would be required. - // A one-digit status can be used to represent all codes - // in that class (e.g. 3 for all 3xx codes). - StatusCode []int `json:"status_code,omitempty"` - - // If set, each header specified must be one of the - // specified values, with the same logic used by the - // request header matcher. - Headers http.Header `json:"headers,omitempty"` -} - -// Match returns true if the given statusCode and hdr match rm. -func (rm ResponseMatcher) Match(statusCode int, hdr http.Header) bool { - if !rm.matchStatusCode(statusCode) { - return false - } - return matchHeaders(hdr, rm.Headers, "", nil) -} - -func (rm ResponseMatcher) matchStatusCode(statusCode int) bool { - if rm.StatusCode == nil { - return true - } - for _, code := range rm.StatusCode { - if StatusCodeMatches(statusCode, code) { - return true - } - } - return false -} - var wordRE = regexp.MustCompile(`\w+`) const regexpPlaceholderPrefix = "http.regexp" diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go index 950020f94e4..2ec7039276b 100644 --- a/modules/caddyhttp/matchers_test.go +++ b/modules/caddyhttp/matchers_test.go @@ -804,155 +804,6 @@ func TestVarREMatcher(t *testing.T) { } } -func TestResponseMatcher(t *testing.T) { - for i, tc := range []struct { - require ResponseMatcher - status int - hdr http.Header // make sure these are canonical cased (std lib will do that in a real request) - expect bool - }{ - { - require: ResponseMatcher{}, - status: 200, - expect: true, - }, - { - require: ResponseMatcher{ - StatusCode: []int{200}, - }, - status: 200, - expect: true, - }, - { - require: ResponseMatcher{ - StatusCode: []int{2}, - }, - status: 200, - expect: true, - }, - { - require: ResponseMatcher{ - StatusCode: []int{201}, - }, - status: 200, - expect: false, - }, - { - require: ResponseMatcher{ - StatusCode: []int{2}, - }, - status: 301, - expect: false, - }, - { - require: ResponseMatcher{ - StatusCode: []int{3}, - }, - status: 301, - expect: true, - }, - { - require: ResponseMatcher{ - StatusCode: []int{3}, - }, - status: 399, - expect: true, - }, - { - require: ResponseMatcher{ - StatusCode: []int{3}, - }, - status: 400, - expect: false, - }, - { - require: ResponseMatcher{ - StatusCode: []int{3, 4}, - }, - status: 400, - expect: true, - }, - { - require: ResponseMatcher{ - StatusCode: []int{3, 401}, - }, - status: 401, - expect: true, - }, - { - require: ResponseMatcher{ - Headers: http.Header{ - "Foo": []string{"bar"}, - }, - }, - hdr: http.Header{"Foo": []string{"bar"}}, - expect: true, - }, - { - require: ResponseMatcher{ - Headers: http.Header{ - "Foo2": []string{"bar"}, - }, - }, - hdr: http.Header{"Foo": []string{"bar"}}, - expect: false, - }, - { - require: ResponseMatcher{ - Headers: http.Header{ - "Foo": []string{"bar", "baz"}, - }, - }, - hdr: http.Header{"Foo": []string{"baz"}}, - expect: true, - }, - { - require: ResponseMatcher{ - Headers: http.Header{ - "Foo": []string{"bar"}, - "Foo2": []string{"baz"}, - }, - }, - hdr: http.Header{"Foo": []string{"baz"}}, - expect: false, - }, - { - require: ResponseMatcher{ - Headers: http.Header{ - "Foo": []string{"bar"}, - "Foo2": []string{"baz"}, - }, - }, - hdr: http.Header{"Foo": []string{"bar"}, "Foo2": []string{"baz"}}, - expect: true, - }, - { - require: ResponseMatcher{ - Headers: http.Header{ - "Foo": []string{"foo*"}, - }, - }, - hdr: http.Header{"Foo": []string{"foobar"}}, - expect: true, - }, - { - require: ResponseMatcher{ - Headers: http.Header{ - "Foo": []string{"foo*"}, - }, - }, - hdr: http.Header{"Foo": []string{"foobar"}}, - expect: true, - }, - } { - actual := tc.require.Match(tc.status, tc.hdr) - if actual != tc.expect { - t.Errorf("Test %d %v: Expected %t, got %t for HTTP %d %v", i, tc.require, tc.expect, actual, tc.status, tc.hdr) - continue - } - } -} - func TestNotMatcher(t *testing.T) { for i, tc := range []struct { host, path string diff --git a/modules/caddyhttp/responsematchers.go b/modules/caddyhttp/responsematchers.go new file mode 100644 index 00000000000..d9ad8480b5e --- /dev/null +++ b/modules/caddyhttp/responsematchers.go @@ -0,0 +1,122 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package caddyhttp + +import ( + "net/http" + "strconv" + "strings" + + "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" +) + +// ResponseMatcher is a type which can determine if an +// HTTP response matches some criteria. +type ResponseMatcher struct { + // If set, one of these status codes would be required. + // A one-digit status can be used to represent all codes + // in that class (e.g. 3 for all 3xx codes). + StatusCode []int `json:"status_code,omitempty"` + + // If set, each header specified must be one of the + // specified values, with the same logic used by the + // request header matcher. + Headers http.Header `json:"headers,omitempty"` +} + +// Match returns true if the given statusCode and hdr match rm. +func (rm ResponseMatcher) Match(statusCode int, hdr http.Header) bool { + if !rm.matchStatusCode(statusCode) { + return false + } + return matchHeaders(hdr, rm.Headers, "", nil) +} + +func (rm ResponseMatcher) matchStatusCode(statusCode int) bool { + if rm.StatusCode == nil { + return true + } + for _, code := range rm.StatusCode { + if StatusCodeMatches(statusCode, code) { + return true + } + } + return false +} + +// ParseNamedResponseMatcher parses the tokens of a named response matcher. +// +// @name { +// header [] +// status +// } +// +// Or, single line syntax: +// +// @name [header []] | [status ] +// +func ParseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map[string]ResponseMatcher) error { + for d.Next() { + definitionName := d.Val() + + if _, ok := matchers[definitionName]; ok { + return d.Errf("matcher is defined more than once: %s", definitionName) + } + + matcher := ResponseMatcher{} + for nesting := d.Nesting(); d.NextArg() || d.NextBlock(nesting); { + switch d.Val() { + case "header": + if matcher.Headers == nil { + matcher.Headers = http.Header{} + } + + // reuse the header request matcher's unmarshaler + headerMatcher := MatchHeader(matcher.Headers) + err := headerMatcher.UnmarshalCaddyfile(d.NewFromNextSegment()) + if err != nil { + return err + } + + matcher.Headers = http.Header(headerMatcher) + case "status": + if matcher.StatusCode == nil { + matcher.StatusCode = []int{} + } + + args := d.RemainingArgs() + if len(args) == 0 { + return d.ArgErr() + } + + for _, arg := range args { + if len(arg) == 3 && strings.HasSuffix(arg, "xx") { + arg = arg[:1] + } + statusNum, err := strconv.Atoi(arg) + if err != nil { + return d.Errf("bad status value '%s': %v", arg, err) + } + matcher.StatusCode = append(matcher.StatusCode, statusNum) + } + default: + return d.Errf("unrecognized response matcher %s", d.Val()) + } + } + + matchers[definitionName] = matcher + } + return nil +} diff --git a/modules/caddyhttp/responsematchers_test.go b/modules/caddyhttp/responsematchers_test.go new file mode 100644 index 00000000000..f5bb6f18fbc --- /dev/null +++ b/modules/caddyhttp/responsematchers_test.go @@ -0,0 +1,169 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package caddyhttp + +import ( + "net/http" + "testing" +) + +func TestResponseMatcher(t *testing.T) { + for i, tc := range []struct { + require ResponseMatcher + status int + hdr http.Header // make sure these are canonical cased (std lib will do that in a real request) + expect bool + }{ + { + require: ResponseMatcher{}, + status: 200, + expect: true, + }, + { + require: ResponseMatcher{ + StatusCode: []int{200}, + }, + status: 200, + expect: true, + }, + { + require: ResponseMatcher{ + StatusCode: []int{2}, + }, + status: 200, + expect: true, + }, + { + require: ResponseMatcher{ + StatusCode: []int{201}, + }, + status: 200, + expect: false, + }, + { + require: ResponseMatcher{ + StatusCode: []int{2}, + }, + status: 301, + expect: false, + }, + { + require: ResponseMatcher{ + StatusCode: []int{3}, + }, + status: 301, + expect: true, + }, + { + require: ResponseMatcher{ + StatusCode: []int{3}, + }, + status: 399, + expect: true, + }, + { + require: ResponseMatcher{ + StatusCode: []int{3}, + }, + status: 400, + expect: false, + }, + { + require: ResponseMatcher{ + StatusCode: []int{3, 4}, + }, + status: 400, + expect: true, + }, + { + require: ResponseMatcher{ + StatusCode: []int{3, 401}, + }, + status: 401, + expect: true, + }, + { + require: ResponseMatcher{ + Headers: http.Header{ + "Foo": []string{"bar"}, + }, + }, + hdr: http.Header{"Foo": []string{"bar"}}, + expect: true, + }, + { + require: ResponseMatcher{ + Headers: http.Header{ + "Foo2": []string{"bar"}, + }, + }, + hdr: http.Header{"Foo": []string{"bar"}}, + expect: false, + }, + { + require: ResponseMatcher{ + Headers: http.Header{ + "Foo": []string{"bar", "baz"}, + }, + }, + hdr: http.Header{"Foo": []string{"baz"}}, + expect: true, + }, + { + require: ResponseMatcher{ + Headers: http.Header{ + "Foo": []string{"bar"}, + "Foo2": []string{"baz"}, + }, + }, + hdr: http.Header{"Foo": []string{"baz"}}, + expect: false, + }, + { + require: ResponseMatcher{ + Headers: http.Header{ + "Foo": []string{"bar"}, + "Foo2": []string{"baz"}, + }, + }, + hdr: http.Header{"Foo": []string{"bar"}, "Foo2": []string{"baz"}}, + expect: true, + }, + { + require: ResponseMatcher{ + Headers: http.Header{ + "Foo": []string{"foo*"}, + }, + }, + hdr: http.Header{"Foo": []string{"foobar"}}, + expect: true, + }, + { + require: ResponseMatcher{ + Headers: http.Header{ + "Foo": []string{"foo*"}, + }, + }, + hdr: http.Header{"Foo": []string{"foobar"}}, + expect: true, + }, + } { + actual := tc.require.Match(tc.status, tc.hdr) + if actual != tc.expect { + t.Errorf("Test %d %v: Expected %t, got %t for HTTP %d %v", i, tc.require, tc.expect, actual, tc.status, tc.hdr) + continue + } + } +} diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 4afd61ef0d8..7ee8bc5298a 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -245,7 +245,7 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error // if the subdirective has an "@" prefix then we // parse it as a response matcher for use with "handle_response" if strings.HasPrefix(d.Val(), matcherPrefix) { - err := h.parseNamedResponseMatcher(d.NewFromNextSegment(), responseMatchers) + err := caddyhttp.ParseNamedResponseMatcher(d.NewFromNextSegment(), responseMatchers) if err != nil { return err } @@ -997,73 +997,6 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } -// Parse the tokens of a named response matcher. -// -// @name { -// header [] -// status -// } -// -// Or, single line syntax: -// -// @name [header []] | [status ] -// -func (Handler) parseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map[string]caddyhttp.ResponseMatcher) error { - for d.Next() { - definitionName := d.Val() - - if _, ok := matchers[definitionName]; ok { - return d.Errf("matcher is defined more than once: %s", definitionName) - } - - matcher := caddyhttp.ResponseMatcher{} - for nesting := d.Nesting(); d.NextArg() || d.NextBlock(nesting); { - switch d.Val() { - case "header": - if matcher.Headers == nil { - matcher.Headers = http.Header{} - } - - // reuse the header request matcher's unmarshaler - headerMatcher := caddyhttp.MatchHeader(matcher.Headers) - err := headerMatcher.UnmarshalCaddyfile(d.NewFromNextSegment()) - if err != nil { - return err - } - - matcher.Headers = http.Header(headerMatcher) - - case "status": - if matcher.StatusCode == nil { - matcher.StatusCode = []int{} - } - - args := d.RemainingArgs() - if len(args) == 0 { - return d.ArgErr() - } - - for _, arg := range args { - if len(arg) == 3 && strings.HasSuffix(arg, "xx") { - arg = arg[:1] - } - statusNum, err := strconv.Atoi(arg) - if err != nil { - return d.Errf("bad status value '%s': %v", arg, err) - } - matcher.StatusCode = append(matcher.StatusCode, statusNum) - } - - default: - return d.Errf("unrecognized response matcher %s", d.Val()) - } - } - - matchers[definitionName] = matcher - } - return nil -} - const matcherPrefix = "@" // Interface guards From f8768c8b307efc5c7156bd419a05563603f4d474 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 30 Apr 2021 21:57:53 -0400 Subject: [PATCH 8/9] reverseproxy: Reintroduce caddyfile.Unmarshaler --- modules/caddyhttp/reverseproxy/caddyfile.go | 167 ++++++++++-------- .../reverseproxy/fastcgi/caddyfile.go | 6 +- .../caddyhttp/reverseproxy/reverseproxy.go | 7 + 3 files changed, 107 insertions(+), 73 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 7ee8bc5298a..1275b26bb1f 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -38,14 +38,18 @@ func init() { func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { rp := new(Handler) - err := rp.ParseCaddyfileReverseProxy(h) + err := rp.UnmarshalCaddyfile(h.Dispenser) + if err != nil { + return nil, err + } + err = rp.FinalizeUnmarshalCaddyfile(h) if err != nil { return nil, err } return rp, nil } -// ParseCaddyfileReverseProxy sets up the handler from Caddyfile tokens. Syntax: +// UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax: // // reverse_proxy [] [] { // # upstreams @@ -101,9 +105,7 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // as `host:port`, or a URL such as `scheme://host:port`. Scheme // and port may be inferred from other parts of the address/URL; if // either are missing, defaults to HTTP. -func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error { - d := helper.Dispenser - +func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // currently, all backends must use the same scheme/protocol (the // underlying JSON does not yet support per-backend transports) var commonScheme string @@ -113,9 +115,9 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error var transport http.RoundTripper var transportModuleName string - // collect the response matchers defined as subdirectives prefixed with "@" - // for use with "handle_response" blocks - responseMatchers := make(map[string]caddyhttp.ResponseMatcher) + // collect the response matchers defined as subdirectives + // prefixed with "@" for use with "handle_response" blocks + h.responseMatchers = make(map[string]caddyhttp.ResponseMatcher) // TODO: the logic in this function is kind of sensitive, we need // to write tests before making any more changes to it @@ -245,7 +247,7 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error // if the subdirective has an "@" prefix then we // parse it as a response matcher for use with "handle_response" if strings.HasPrefix(d.Val(), matcherPrefix) { - err := caddyhttp.ParseNamedResponseMatcher(d.NewFromNextSegment(), responseMatchers) + err := caddyhttp.ParseNamedResponseMatcher(d.NewFromNextSegment(), h.responseMatchers) if err != nil { return err } @@ -643,69 +645,10 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error transport = rt case "handle_response": - 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 { - 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 { - if !strings.HasPrefix(args[0], matcherPrefix) { - return d.Errf("must use a named response matcher, starting with '@'") - } - - foundMatcher, ok := responseMatchers[args[0]] - if !ok { - return d.Errf("no named response matcher defined with name '%s'", args[0][1:]) - } - 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 { - return err - } - subroute, ok := handler.(*caddyhttp.Subroute) - if !ok { - return helper.Errf("segment was not parsed as a subroute") - } - h.HandleResponse = append( - h.HandleResponse, - caddyhttp.ResponseHandler{ - Match: matcher, - Routes: subroute.Routes, - }, - ) + // delegate the parsing of handle_response to the caller, + // since we need the httpcaddyfile.Helper to parse subroutes. + // See h.FinalizeUnmarshalCaddyfile + h.handleResponseSegments = append(h.handleResponseSegments, d.NewFromNextSegment()) default: return d.Errf("unrecognized subdirective %s", d.Val()) @@ -746,6 +689,81 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error } } + return nil +} + +// FinalizeUnmarshalCaddyfile finalizes the Caddyfile parsing which +// requires having an httpcaddyfile.Helper to function, to parse subroutes. +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 { + 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 { + 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:]) + } + 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 { + return err + } + subroute, ok := handler.(*caddyhttp.Subroute) + if !ok { + return helper.Errf("segment was not parsed as a subroute") + } + h.HandleResponse = append( + h.HandleResponse, + caddyhttp.ResponseHandler{ + Match: matcher, + Routes: subroute.Routes, + }, + ) + } + // move the handle_response entries without a matcher to the end. // we can't use sort.SliceStable because it will reorder the rest of the // entries which may be undesirable because we don't have a good @@ -761,6 +779,10 @@ func (h *Handler) ParseCaddyfileReverseProxy(helper httpcaddyfile.Helper) error } h.HandleResponse = append(withMatchers, withoutMatchers...) + // clean up the bits we only needed for adapting + h.handleResponseSegments = nil + h.responseMatchers = nil + return nil } @@ -1001,5 +1023,6 @@ const matcherPrefix = "@" // Interface guards var ( + _ caddyfile.Unmarshaler = (*Handler)(nil) _ caddyfile.Unmarshaler = (*HTTPTransport)(nil) ) diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index 8ae170d921b..0ccd9fec720 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -353,7 +353,11 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error // the rest of the config is specified by the user // using the reverse_proxy directive syntax - err = rpHandler.ParseCaddyfileReverseProxy(h.WithDispenser(dispenser)) + err = rpHandler.UnmarshalCaddyfile(dispenser) + if err != nil { + return nil, err + } + err = rpHandler.FinalizeUnmarshalCaddyfile(h) if err != nil { return nil, err } diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index b552d965da1..b6c24f3c2f7 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -31,6 +31,7 @@ import ( "time" "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/caddyserver/caddy/v2/modules/caddyhttp/headers" "go.uber.org/zap" @@ -127,6 +128,12 @@ type Handler struct { Transport http.RoundTripper `json:"-"` CB CircuitBreaker `json:"-"` + // Holds the named response matchers from the Caddyfile while adapting + responseMatchers map[string]caddyhttp.ResponseMatcher + + // Holds the handle_response Caddyfile tokens while adapting + handleResponseSegments []*caddyfile.Dispenser + ctx caddy.Context logger *zap.Logger } From 14eb7089b102b4a85a73ffe0e20caefa97e77c61 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Sun, 2 May 2021 14:19:03 -0400 Subject: [PATCH 9/9] reverseproxy: Add comment mentioning Finalize should be called --- modules/caddyhttp/reverseproxy/caddyfile.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 1275b26bb1f..61eac7ed8aa 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -105,6 +105,9 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // as `host:port`, or a URL such as `scheme://host:port`. Scheme // and port may be inferred from other parts of the address/URL; if // either are missing, defaults to HTTP. +// +// The FinalizeUnmarshalCaddyfile method should be called after this +// to finalize parsing of "handle_response" blocks, if possible. func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // currently, all backends must use the same scheme/protocol (the // underlying JSON does not yet support per-backend transports)