From a6da1acdc86199a7f99fa2347d5f91cd59ff90d8 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Tue, 18 Nov 2025 00:51:37 +0800 Subject: [PATCH] reverse_proxy: use interfaces to modify the behaviors of the transports (#7353) --- modules/caddyhttp/reverseproxy/caddyfile.go | 7 ++- .../caddyhttp/reverseproxy/fastcgi/fastcgi.go | 19 +++++++- .../caddyhttp/reverseproxy/healthchecks.go | 12 ++--- .../caddyhttp/reverseproxy/httptransport.go | 31 ++++++++++-- .../caddyhttp/reverseproxy/reverseproxy.go | 48 ++++++++++++++----- 5 files changed, 88 insertions(+), 29 deletions(-) diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 8439d1d51..12d610800 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -888,8 +888,11 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if commonScheme == "http" && te.TLSEnabled() { return d.Errf("upstream address scheme is HTTP but transport is configured for HTTP+TLS (HTTPS)") } - if te, ok := transport.(*HTTPTransport); ok && commonScheme == "h2c" { - te.Versions = []string{"h2c", "2"} + if h2ct, ok := transport.(H2CTransport); ok && commonScheme == "h2c" { + err := h2ct.EnableH2C() + if err != nil { + return err + } } } else if commonScheme == "https" { return d.Errf("upstreams are configured for HTTPS but transport module does not support TLS: %T", transport) diff --git a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go index d451dd380..5c68c3ad5 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go @@ -112,6 +112,20 @@ func (t *Transport) Provision(ctx caddy.Context) error { return nil } +// DefaultBufferSizes enables request buffering for fastcgi if not configured. +// This is because most fastcgi servers are php-fpm that require the content length to be set to read the body, golang +// std has fastcgi implementation that doesn't need this value to process the body, but we can safely assume that's +// not used. +// http3 requests have a negative content length for GET and HEAD requests, if that header is not sent. +// see: https://github.com/caddyserver/caddy/issues/6678#issuecomment-2472224182 +// Though it appears even if CONTENT_LENGTH is invalid, php-fpm can handle just fine if the body is empty (no Stdin records sent). +// php-fpm will hang if there is any data in the body though, https://github.com/caddyserver/caddy/issues/5420#issuecomment-2415943516 + +// TODO: better default buffering for fastcgi requests without content length, in theory a value of 1 should be enough, make it bigger anyway +func (t Transport) DefaultBufferSizes() (int64, int64) { + return 4096, 0 +} + // RoundTrip implements http.RoundTripper. func (t Transport) RoundTrip(r *http.Request) (*http.Response, error) { server := r.Context().Value(caddyhttp.ServerCtxKey).(*caddyhttp.Server) @@ -427,6 +441,7 @@ var headerNameReplacer = strings.NewReplacer(" ", "_", "-", "_") var ( _ zapcore.ObjectMarshaler = (*loggableEnv)(nil) - _ caddy.Provisioner = (*Transport)(nil) - _ http.RoundTripper = (*Transport)(nil) + _ caddy.Provisioner = (*Transport)(nil) + _ http.RoundTripper = (*Transport)(nil) + _ reverseproxy.BufferedTransport = (*Transport)(nil) ) diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index ac42570b2..b72e723e0 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -23,7 +23,6 @@ import ( "net/url" "regexp" "runtime/debug" - "slices" "strconv" "strings" "time" @@ -405,14 +404,9 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, networ u.Host = net.JoinHostPort(host, port) } - // this is kind of a hacky way to know if we should use HTTPS, but whatever - if tt, ok := h.Transport.(TLSTransport); ok && tt.TLSEnabled() { - u.Scheme = "https" - - // if the port is in the except list, flip back to HTTP - if ht, ok := h.Transport.(*HTTPTransport); ok && slices.Contains(ht.TLS.ExceptPorts, port) { - u.Scheme = "http" - } + // override health check schemes if applicable + if hcsot, ok := h.Transport.(HealthCheckSchemeOverriderTransport); ok { + hcsot.OverrideHealthCheckScheme(u, port) } // if we have a provisioned uri, use that, otherwise use diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 1e4cfa743..8edc585e7 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -564,6 +564,26 @@ func (h *HTTPTransport) EnableTLS(base *TLSConfig) error { return nil } +// EnableH2C enables H2C (HTTP/2 over Cleartext) on the transport. +func (h *HTTPTransport) EnableH2C() error { + h.Versions = []string{"h2c", "2"} + return nil +} + +// OverrideHealthCheckScheme overrides the scheme of the given URL +// used for health checks. +func (h HTTPTransport) OverrideHealthCheckScheme(base *url.URL, port string) { + // if tls is enabled and the port isn't in the except list, use HTTPs + if h.TLSEnabled() && !slices.Contains(h.TLS.ExceptPorts, port) { + base.Scheme = "https" + } +} + +// ProxyProtocolEnabled returns true if proxy protocol is enabled. +func (h HTTPTransport) ProxyProtocolEnabled() bool { + return h.ProxyProtocol != "" +} + // Cleanup implements caddy.CleanerUpper and closes any idle connections. func (h HTTPTransport) Cleanup() error { if h.Transport == nil { @@ -820,8 +840,11 @@ func decodeBase64DERCert(certStr string) (*x509.Certificate, error) { // Interface guards var ( - _ caddy.Provisioner = (*HTTPTransport)(nil) - _ http.RoundTripper = (*HTTPTransport)(nil) - _ caddy.CleanerUpper = (*HTTPTransport)(nil) - _ TLSTransport = (*HTTPTransport)(nil) + _ caddy.Provisioner = (*HTTPTransport)(nil) + _ http.RoundTripper = (*HTTPTransport)(nil) + _ caddy.CleanerUpper = (*HTTPTransport)(nil) + _ TLSTransport = (*HTTPTransport)(nil) + _ H2CTransport = (*HTTPTransport)(nil) + _ HealthCheckSchemeOverriderTransport = (*HTTPTransport)(nil) + _ ProxyProtocolTransport = (*HTTPTransport)(nil) ) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 794860d8e..d207e240e 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -243,18 +243,16 @@ func (h *Handler) Provision(ctx caddy.Context) error { return fmt.Errorf("loading transport: %v", err) } h.Transport = mod.(http.RoundTripper) - // enable request buffering for fastcgi if not configured - // This is because most fastcgi servers are php-fpm that require the content length to be set to read the body, golang - // std has fastcgi implementation that doesn't need this value to process the body, but we can safely assume that's - // not used. - // http3 requests have a negative content length for GET and HEAD requests, if that header is not sent. - // see: https://github.com/caddyserver/caddy/issues/6678#issuecomment-2472224182 - // Though it appears even if CONTENT_LENGTH is invalid, php-fpm can handle just fine if the body is empty (no Stdin records sent). - // php-fpm will hang if there is any data in the body though, https://github.com/caddyserver/caddy/issues/5420#issuecomment-2415943516 - // TODO: better default buffering for fastcgi requests without content length, in theory a value of 1 should be enough, make it bigger anyway - if module, ok := h.Transport.(caddy.Module); ok && module.CaddyModule().ID.Name() == "fastcgi" && h.RequestBuffers == 0 { - h.RequestBuffers = 4096 + // set default buffer sizes if applicable + if bt, ok := h.Transport.(BufferedTransport); ok { + reqBuffers, respBuffers := bt.DefaultBufferSizes() + if h.RequestBuffers == 0 { + h.RequestBuffers = reqBuffers + } + if h.ResponseBuffers == 0 { + h.ResponseBuffers = respBuffers + } } } if h.LoadBalancing != nil && h.LoadBalancing.SelectionPolicyRaw != nil { @@ -1210,7 +1208,7 @@ func (h *Handler) directRequest(req *http.Request, di DialInfo) { } // add client address to the host to let transport differentiate requests from different clients - if ht, ok := h.Transport.(*HTTPTransport); ok && ht.ProxyProtocol != "" { + if ppt, ok := h.Transport.(ProxyProtocolTransport); ok && ppt.ProxyProtocolEnabled() { if proxyProtocolInfo, ok := caddyhttp.GetVar(req.Context(), proxyProtocolInfoVarKey).(ProxyProtocolInfo); ok { reqHost = proxyProtocolInfo.AddrPort.String() + "->" + reqHost } @@ -1501,6 +1499,32 @@ type TLSTransport interface { EnableTLS(base *TLSConfig) error } +// H2CTransport is implemented by transports +// that are capable of using h2c. +type H2CTransport interface { + EnableH2C() error +} + +// ProxyProtocolTransport is implemented by transports +// that are capable of using proxy protocol. +type ProxyProtocolTransport interface { + ProxyProtocolEnabled() bool +} + +// HealthCheckSchemeOverriderTransport is implemented by transports +// that can override the scheme used for health checks. +type HealthCheckSchemeOverriderTransport interface { + OverrideHealthCheckScheme(base *url.URL, port string) +} + +// BufferedTransport is implemented by transports +// that needs to buffer requests and/or responses. +type BufferedTransport interface { + // DefaultBufferSizes returns the default buffer sizes + // for requests and responses, respectively if buffering isn't enabled. + DefaultBufferSizes() (int64, int64) +} + // roundtripSucceededError is an error type that is returned if the // roundtrip succeeded, but an error occurred after-the-fact. type roundtripSucceededError struct{ error }