diff options
Diffstat (limited to 'modules/caddyhttp/reverseproxy')
| -rw-r--r-- | modules/caddyhttp/reverseproxy/copyresponse.go | 2 | ||||
| -rw-r--r-- | modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go | 47 | ||||
| -rw-r--r-- | modules/caddyhttp/reverseproxy/reverseproxy.go | 71 |
3 files changed, 34 insertions, 86 deletions
diff --git a/modules/caddyhttp/reverseproxy/copyresponse.go b/modules/caddyhttp/reverseproxy/copyresponse.go index 72a55d4..c1c9de9 100644 --- a/modules/caddyhttp/reverseproxy/copyresponse.go +++ b/modules/caddyhttp/reverseproxy/copyresponse.go @@ -80,7 +80,7 @@ func (h CopyResponseHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request hrc.isFinalized = true // write the response - return hrc.handler.finalizeResponse(rw, req, hrc.response, repl, hrc.start, hrc.logger, false) + return hrc.handler.finalizeResponse(rw, req, hrc.response, repl, hrc.start, hrc.logger) } // CopyResponseHeadersHandler is a special HTTP handler which may diff --git a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go index c22ddde..8230216 100644 --- a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go @@ -59,13 +59,6 @@ func init() { // Remote-Email {http.reverse_proxy.header.Remote-Email} // } // } -// -// handle_response { -// copy_response_headers { -// exclude Connection Keep-Alive Te Trailers Transfer-Encoding Upgrade -// } -// copy_response -// } // } // func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) { @@ -217,41 +210,13 @@ func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) }, ) } - rpHandler.HandleResponse = append(rpHandler.HandleResponse, goodResponseHandler) - // set up handler for denial responses; when a response - // has any other status than 2xx, then we copy the response - // back to the client, and terminate handling. - denialResponseHandler := caddyhttp.ResponseHandler{ - Routes: []caddyhttp.Route{ - { - HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( - &reverseproxy.CopyResponseHeadersHandler{ - Exclude: []string{ - "Connection", - "Keep-Alive", - "Te", - "Trailers", - "Transfer-Encoding", - "Upgrade", - }, - }, - "handler", - "copy_response_headers", - nil, - )}, - }, - { - HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( - &reverseproxy.CopyResponseHandler{}, - "handler", - "copy_response", - nil, - )}, - }, - }, - } - rpHandler.HandleResponse = append(rpHandler.HandleResponse, denialResponseHandler) + // note that when a response has any other status than 2xx, then we + // use the reverse proxy's default behaviour of copying the response + // back to the client, so we don't need to explicitly add a response + // handler specifically for that behaviour; we do need the 2xx handler + // though, to make handling fall through to handlers deeper in the chain. + rpHandler.HandleResponse = append(rpHandler.HandleResponse, goodResponseHandler) // the rest of the config is specified by the user // using the reverse_proxy directive syntax diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 1068c23..15e3104 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -784,18 +784,14 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe res.Body = h.bufferedBody(res.Body) } - // the response body may get closed by a response handler, - // and we need to keep track to make sure we don't try to copy - // the response if it was already closed - bodyClosed := false - // see if any response handler is configured for this response from the backend for i, rh := range h.HandleResponse { if rh.Match != nil && !rh.Match.Match(res.StatusCode, res.Header) { continue } - // if configured to only change the status code, do that then continue regular proxy response + // if configured to only change the status code, + // do that then continue regular proxy response if statusCodeStr := rh.StatusCode.String(); statusCodeStr != "" { statusCode, err := strconv.Atoi(repl.ReplaceAll(statusCodeStr, "")) if err != nil { @@ -840,33 +836,29 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe // pass the request through the response handler routes routeErr := rh.Routes.Compile(next).ServeHTTP(rw, origReq.WithContext(ctx)) - // if the response handler routes already finalized the response, - // we can return early. It should be finalized if the routes executed - // included a copy_response handler. If a fresh response was written - // by the routes instead, then we still need to finalize the response - // without copying the body. - if routeErr == nil && hrc.isFinalized { - return nil + // close the response body afterwards, since we don't need it anymore; + // either a route had 'copy_response' which already consumed the body, + // or some other terminal handler ran which doesn't need the response + // body after that point (e.g. 'file_server' for X-Accel-Redirect flow), + // or we fell through to subsequent handlers past this proxy + // (e.g. forward auth's 2xx response flow). + if !hrc.isFinalized { + res.Body.Close() } - // always close the response body afterwards, since it's expected - // that the response handler routes will have written to the - // response writer with a new body, if it wasn't already finalized. - res.Body.Close() - bodyClosed = true - + // wrap any route error in roundtripSucceeded so caller knows that + // the roundtrip was successful and to not retry if routeErr != nil { - // wrap error in roundtripSucceeded so caller knows that - // the roundtrip was successful and to not retry return roundtripSucceeded{routeErr} } - // we've already closed the body, so there's no use allowing - // another response handler to run as well - break + // we're done handling the response, and we don't want to + // fall through to the default finalize/copy behaviour + return nil } - return h.finalizeResponse(rw, req, res, repl, start, logger, bodyClosed) + // copy the response body and headers back to the upstream client + return h.finalizeResponse(rw, req, res, repl, start, logger) } // finalizeResponse prepares and copies the response. @@ -877,7 +869,6 @@ func (h Handler) finalizeResponse( repl *caddy.Replacer, start time.Time, logger *zap.Logger, - bodyClosed bool, ) error { // deal with 101 Switching Protocols responses: (WebSocket, h2c, etc) if res.StatusCode == http.StatusSwitchingProtocols { @@ -891,13 +882,6 @@ func (h Handler) finalizeResponse( res.Header.Del(h) } - // remove the content length if we're not going to be copying - // from the response, because otherwise there'll be a mismatch - // between bytes written and the advertised length - if bodyClosed { - res.Header.Del("Content-Length") - } - // apply any response header operations if h.Headers != nil && h.Headers.Response != nil { if h.Headers.Response.Require == nil || @@ -920,17 +904,16 @@ func (h Handler) finalizeResponse( } rw.WriteHeader(res.StatusCode) - if !bodyClosed { - err := h.copyResponse(rw, res.Body, h.flushInterval(req, res)) - res.Body.Close() // close now, instead of defer, to populate res.Trailer - if err != nil { - // we're streaming the response and we've already written headers, so - // there's nothing an error handler can do to recover at this point; - // the standard lib's proxy panics at this point, but we'll just log - // the error and abort the stream here - h.logger.Error("aborting with incomplete response", zap.Error(err)) - return nil - } + + err := h.copyResponse(rw, res.Body, h.flushInterval(req, res)) + res.Body.Close() // close now, instead of defer, to populate res.Trailer + if err != nil { + // we're streaming the response and we've already written headers, so + // there's nothing an error handler can do to recover at this point; + // the standard lib's proxy panics at this point, but we'll just log + // the error and abort the stream here + h.logger.Error("aborting with incomplete response", zap.Error(err)) + return nil } if len(res.Trailer) > 0 { |