From 1578f70a2f2d423bd8fab666caec569a88a7e1a0 Mon Sep 17 00:00:00 2001 From: mmmray <142015632+mmmray@users.noreply.github.com> Date: Thu, 1 Aug 2024 21:23:13 -0500 Subject: [PATCH 1/4] SplitHTTP client: Raise idle timeout to 5 minutes Copy the settings from QUIC transport. See https://github.com/XTLS/Xray-core/pull/3565#issuecomment-2264356688 -- Tested with CDN and it seems better than before (fewer reconnections) keep-alive packets are still turned off, so the original concern should be resolved. --- transport/internet/splithttp/dialer.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/transport/internet/splithttp/dialer.go b/transport/internet/splithttp/dialer.go index 07e5d5d1cb85..476c4b37e967 100644 --- a/transport/internet/splithttp/dialer.go +++ b/transport/internet/splithttp/dialer.go @@ -89,7 +89,13 @@ func getHTTPClient(ctx context.Context, dest net.Destination, streamSettings *in var uploadTransport http.RoundTripper if isH3 { + quicConfig := &quic.Config{ + KeepAlivePeriod: 0, + HandshakeIdleTimeout: time.Second * 8, + MaxIdleTimeout: time.Second * 300, + } roundTripper := &http3.RoundTripper{ + QUICConfig: quicConfig, TLSClientConfig: gotlsConfig, Dial: func(ctx context.Context, addr string, tlsCfg *gotls.Config, cfg *quic.Config) (quic.EarlyConnection, error) { conn, err := internet.DialSystem(ctx, dest, streamSettings.SocketSettings) From 45abf902a1939fa2fd54c0331916f6210802737c Mon Sep 17 00:00:00 2001 From: mmmray <142015632+mmmray@users.noreply.github.com> Date: Fri, 2 Aug 2024 12:21:17 -0500 Subject: [PATCH 2/4] remove unnecessary parameters --- transport/internet/splithttp/dialer.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/transport/internet/splithttp/dialer.go b/transport/internet/splithttp/dialer.go index 476c4b37e967..5a1b1743fde5 100644 --- a/transport/internet/splithttp/dialer.go +++ b/transport/internet/splithttp/dialer.go @@ -90,9 +90,7 @@ func getHTTPClient(ctx context.Context, dest net.Destination, streamSettings *in if isH3 { quicConfig := &quic.Config{ - KeepAlivePeriod: 0, - HandshakeIdleTimeout: time.Second * 8, - MaxIdleTimeout: time.Second * 300, + MaxIdleTimeout: time.Second * 300, } roundTripper := &http3.RoundTripper{ QUICConfig: quicConfig, From 3317cb874df3f634fc87aba24bbe67f80da4386d Mon Sep 17 00:00:00 2001 From: mmmray <142015632+mmmray@users.noreply.github.com> Date: Sun, 4 Aug 2024 18:39:32 -0500 Subject: [PATCH 3/4] address some feedback --- transport/internet/splithttp/dialer.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/transport/internet/splithttp/dialer.go b/transport/internet/splithttp/dialer.go index 5a1b1743fde5..f684339cb42e 100644 --- a/transport/internet/splithttp/dialer.go +++ b/transport/internet/splithttp/dialer.go @@ -26,6 +26,9 @@ import ( "golang.org/x/net/http2" ) +// this is consistent with other xray transports +const connIdleTimeout = 300 * time.Second + type dialerConf struct { net.Destination *internet.MemoryStreamConfig @@ -90,7 +93,13 @@ func getHTTPClient(ctx context.Context, dest net.Destination, streamSettings *in if isH3 { quicConfig := &quic.Config{ - MaxIdleTimeout: time.Second * 300, + MaxIdleTimeout: connIdleTimeout, + + // these two are defaults of quic-go/http3. the default of quic-go (no + // http3) is different, so it is hardcoded here for clarity. + // https://github.com/quic-go/quic-go/blob/b8ea5c798155950fb5bbfdd06cad1939c9355878/http3/client.go#L36-L39 + MaxIncomingStreams: -1, + KeepAlivePeriod: time.Second * 10, } roundTripper := &http3.RoundTripper{ QUICConfig: quicConfig, @@ -135,7 +144,7 @@ func getHTTPClient(ctx context.Context, dest net.Destination, streamSettings *in DialTLSContext: func(ctxInner context.Context, network string, addr string, cfg *gotls.Config) (net.Conn, error) { return dialContext(ctxInner) }, - IdleConnTimeout: 90 * time.Second, + IdleConnTimeout: connIdleTimeout, } uploadTransport = downloadTransport } else { @@ -146,7 +155,7 @@ func getHTTPClient(ctx context.Context, dest net.Destination, streamSettings *in downloadTransport = &http.Transport{ DialTLSContext: httpDialContext, DialContext: httpDialContext, - IdleConnTimeout: 90 * time.Second, + IdleConnTimeout: connIdleTimeout, // chunked transfer download with keepalives is buggy with // http.Client and our custom dial context. DisableKeepAlives: true, From f2512c4b4c14006d7f2d73387402b1c91cf5cde7 Mon Sep 17 00:00:00 2001 From: mmmray <142015632+mmmray@users.noreply.github.com> Date: Mon, 5 Aug 2024 16:10:12 -0500 Subject: [PATCH 4/4] also tweak keepalive --- transport/internet/splithttp/dialer.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/transport/internet/splithttp/dialer.go b/transport/internet/splithttp/dialer.go index f684339cb42e..d50514e4ceb6 100644 --- a/transport/internet/splithttp/dialer.go +++ b/transport/internet/splithttp/dialer.go @@ -26,9 +26,16 @@ import ( "golang.org/x/net/http2" ) -// this is consistent with other xray transports +// defines the maximum time an idle TCP session can survive in the tunnel, so +// it should be consistent across HTTP versions and with other transports. const connIdleTimeout = 300 * time.Second +// consistent with quic-go +const h3KeepalivePeriod = 10 * time.Second + +// consistent with chrome +const h2KeepalivePeriod = 45 * time.Second + type dialerConf struct { net.Destination *internet.MemoryStreamConfig @@ -99,7 +106,7 @@ func getHTTPClient(ctx context.Context, dest net.Destination, streamSettings *in // http3) is different, so it is hardcoded here for clarity. // https://github.com/quic-go/quic-go/blob/b8ea5c798155950fb5bbfdd06cad1939c9355878/http3/client.go#L36-L39 MaxIncomingStreams: -1, - KeepAlivePeriod: time.Second * 10, + KeepAlivePeriod: h3KeepalivePeriod, } roundTripper := &http3.RoundTripper{ QUICConfig: quicConfig, @@ -145,6 +152,7 @@ func getHTTPClient(ctx context.Context, dest net.Destination, streamSettings *in return dialContext(ctxInner) }, IdleConnTimeout: connIdleTimeout, + ReadIdleTimeout: h2KeepalivePeriod, } uploadTransport = downloadTransport } else {