Skip to content

Commit 8aca108

Browse files
authored
reverseproxy: do not disable keepalive if proxy protocol is used (#7300)
1 parent 156ce99 commit 8aca108

3 files changed

Lines changed: 31 additions & 18 deletions

File tree

modules/caddyhttp/reverseproxy/hosts.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,3 +285,6 @@ type ProxyProtocolInfo struct {
285285
// tlsH1OnlyVarKey is the key used that indicates the connection will use h1 only for TLS.
286286
// https://github.com/caddyserver/caddy/issues/7292
287287
const tlsH1OnlyVarKey = "reverse_proxy.tls_h1_only"
288+
289+
// proxyVarKey is the key used that indicates the proxy server used for a request.
290+
const proxyVarKey = "reverse_proxy.proxy"

modules/caddyhttp/reverseproxy/httptransport.go

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
weakrand "math/rand"
2525
"net"
2626
"net/http"
27+
"net/url"
2728
"os"
2829
"reflect"
2930
"slices"
@@ -267,15 +268,15 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
267268
}
268269

269270
dialContext := func(ctx context.Context, network, address string) (net.Conn, error) {
270-
// For unix socket upstreams, we need to recover the dial info from
271-
// the request's context, because the Host on the request's URL
272-
// will have been modified by directing the request, overwriting
273-
// the unix socket filename.
274-
// Also, we need to avoid overwriting the address at this point
275-
// when not necessary, because http.ProxyFromEnvironment may have
276-
// modified the address according to the user's env proxy config.
271+
// The network is usually tcp, and the address is the host in http.Request.URL.Host
272+
// and that's been overwritten in directRequest
273+
// However, if proxy is used according to http.ProxyFromEnvironment or proxy providers,
274+
// address will be the address of the proxy server.
275+
276+
// This means we can safely use the address in dialInfo if proxy is not used (the address and network will be same any way)
277+
// or if the upstream is unix (because there is no way socks or http proxy can be used for unix address).
277278
if dialInfo, ok := GetDialInfo(ctx); ok {
278-
if strings.HasPrefix(dialInfo.Network, "unix") {
279+
if caddyhttp.GetVar(ctx, proxyVarKey) == nil || strings.HasPrefix(dialInfo.Network, "unix") {
279280
network = dialInfo.Network
280281
address = dialInfo.Address
281282
}
@@ -376,9 +377,19 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
376377
return nil, fmt.Errorf("network_proxy module is not `(func(*http.Request) (*url.URL, error))``")
377378
}
378379
}
380+
// we need to keep track if a proxy is used for a request
381+
proxyWrapper := func(req *http.Request) (*url.URL, error) {
382+
u, err := proxy(req)
383+
if u == nil || err != nil {
384+
return u, err
385+
}
386+
// there must be a proxy for this request
387+
caddyhttp.SetVar(req.Context(), proxyVarKey, u)
388+
return u, nil
389+
}
379390

380391
rt := &http.Transport{
381-
Proxy: proxy,
392+
Proxy: proxyWrapper,
382393
DialContext: dialContext,
383394
MaxConnsPerHost: h.MaxConnsPerHost,
384395
ResponseHeaderTimeout: time.Duration(h.ResponseHeaderTimeout),
@@ -457,14 +468,6 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
457468
rt.IdleConnTimeout = time.Duration(h.KeepAlive.IdleConnTimeout)
458469
}
459470

460-
// The proxy protocol header can only be sent once right after opening the connection.
461-
// So single connection must not be used for multiple requests, which can potentially
462-
// come from different clients.
463-
if !rt.DisableKeepAlives && h.ProxyProtocol != "" {
464-
caddyCtx.Logger().Warn("disabling keepalives, they are incompatible with using PROXY protocol")
465-
rt.DisableKeepAlives = true
466-
}
467-
468471
if h.Compression != nil {
469472
rt.DisableCompression = !*h.Compression
470473
}

modules/caddyhttp/reverseproxy/reverseproxy.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,7 @@ func (lb LoadBalancing) tryAgain(ctx caddy.Context, start time.Time, retries int
11981198

11991199
// directRequest modifies only req.URL so that it points to the upstream
12001200
// in the given DialInfo. It must modify ONLY the request URL.
1201-
func (Handler) directRequest(req *http.Request, di DialInfo) {
1201+
func (h *Handler) directRequest(req *http.Request, di DialInfo) {
12021202
// we need a host, so set the upstream's host address
12031203
reqHost := di.Address
12041204

@@ -1209,6 +1209,13 @@ func (Handler) directRequest(req *http.Request, di DialInfo) {
12091209
reqHost = di.Host
12101210
}
12111211

1212+
// add client address to the host to let transport differentiate requests from different clients
1213+
if ht, ok := h.Transport.(*HTTPTransport); ok && ht.ProxyProtocol != "" {
1214+
if proxyProtocolInfo, ok := caddyhttp.GetVar(req.Context(), proxyProtocolInfoVarKey).(ProxyProtocolInfo); ok {
1215+
reqHost = proxyProtocolInfo.AddrPort.String() + "->" + reqHost
1216+
}
1217+
}
1218+
12121219
req.URL.Host = reqHost
12131220
}
12141221

0 commit comments

Comments
 (0)