diff options
| -rw-r--r-- | modules/caddyhttp/reverseproxy/reverseproxy.go | 32 | ||||
| -rw-r--r-- | modules/caddyhttp/reverseproxy/streaming.go | 20 |
2 files changed, 37 insertions, 15 deletions
diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 6b217e1ae..633cc657f 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -23,6 +23,7 @@ import ( "io" "net" "net/http" + "net/textproto" "net/url" "regexp" "strconv" @@ -80,10 +81,13 @@ type Handler struct { // Upstreams is the list of backends to proxy to. Upstreams UpstreamPool `json:"upstreams,omitempty"` - // Adjusts how often to flush the response buffer. A - // negative value disables response buffering. - // TODO: figure out good defaults and write docs for this - // (see https://github.com/caddyserver/caddy/issues/1460) + // Adjusts how often to flush the response buffer. By default, + // no periodic flushing is done. A negative value disables + // response buffering, and flushes immediately after each + // write to the client. This option is ignored when the upstream's + // response is recognized as a streaming response, or if its + // content length is -1; for such responses, writes are flushed + // to the client immediately. FlushInterval caddy.Duration `json:"flush_interval,omitempty"` // Headers manipulates headers between Caddy and the backend. @@ -528,13 +532,19 @@ func (h Handler) prepareRequest(req *http.Request) error { // If we aren't the first proxy retain prior // X-Forwarded-For information as a comma+space // separated list and fold multiple headers into one. - if prior, ok := req.Header["X-Forwarded-For"]; ok { + prior, ok := req.Header["X-Forwarded-For"] + omit := ok && prior == nil // Issue 38079: nil now means don't populate the header + if len(prior) > 0 { clientIP = strings.Join(prior, ", ") + ", " + clientIP } - req.Header.Set("X-Forwarded-For", clientIP) + if !omit { + req.Header.Set("X-Forwarded-For", clientIP) + } } - if req.Header.Get("X-Forwarded-Proto") == "" { + prior, ok := req.Header["X-Forwarded-Proto"] + omit := ok && prior == nil + if len(prior) == 0 && !omit { // set X-Forwarded-Proto; many backend apps expect this too proto := "https" if req.TLS == nil { @@ -827,10 +837,10 @@ func upgradeType(h http.Header) string { // removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h. // See RFC 7230, section 6.1 func removeConnectionHeaders(h http.Header) { - if c := h.Get("Connection"); c != "" { - for _, f := range strings.Split(c, ",") { - if f = strings.TrimSpace(f); f != "" { - h.Del(f) + for _, f := range h["Connection"] { + for _, sf := range strings.Split(f, ",") { + if sf = textproto.TrimString(sf); sf != "" { + h.Del(sf) } } } diff --git a/modules/caddyhttp/reverseproxy/streaming.go b/modules/caddyhttp/reverseproxy/streaming.go index 285c04b78..1db352b70 100644 --- a/modules/caddyhttp/reverseproxy/streaming.go +++ b/modules/caddyhttp/reverseproxy/streaming.go @@ -32,6 +32,9 @@ import ( func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWriter, req *http.Request, res *http.Response) { reqUpType := upgradeType(req.Header) resUpType := upgradeType(res.Header) + // TODO: Update to use "net/http/internal/ascii" once we bumped + // the minimum Go version to 1.17. + // See https://github.com/golang/go/commit/5c489514bc5e61ad9b5b07bd7d8ec65d66a0512a if reqUpType != resUpType { h.logger.Debug("backend tried to switch to unexpected protocol via Upgrade header", zap.String("backend_upgrade", resUpType), @@ -39,8 +42,6 @@ func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrite return } - copyHeader(res.Header, rw.Header()) - hj, ok := rw.(http.Hijacker) if !ok { h.logger.Sugar().Errorf("can't switch protocols using non-Hijacker ResponseWriter type %T", rw) @@ -78,6 +79,9 @@ func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrite logger.Debug("connection closed", zap.Duration("duration", time.Since(start))) }() + copyHeader(rw.Header(), res.Header) + + res.Header = rw.Header() res.Body = nil // so res.Write only writes the headers; we have res.Body in backConn above if err := res.Write(brw); err != nil { h.logger.Debug("response write", zap.Error(err)) @@ -107,13 +111,16 @@ func (h Handler) flushInterval(req *http.Request, res *http.Response) time.Durat return -1 // negative means immediately } + // We might have the case of streaming for which Content-Length might be unset. + if res.ContentLength == -1 { + return -1 + } + // for h2 and h2c upstream streaming data to client (issues #3556 and #3606) if h.isBidirectionalStream(req, res) { return -1 } - // TODO: more specific cases? e.g. res.ContentLength == -1? (this TODO is from the std lib, but - // strangely similar to our isBidirectionalStream function that we implemented ourselves) return time.Duration(h.FlushInterval) } @@ -142,6 +149,11 @@ func (h Handler) copyResponse(dst io.Writer, src io.Reader, flushInterval time.D latency: flushInterval, } defer mlw.stop() + + // set up initial timer so headers get flushed even if body writes are delayed + mlw.flushPending = true + mlw.t = time.AfterFunc(flushInterval, mlw.delayedFlush) + dst = mlw } } |