From 722dde5cfc48f526e97693d2a167d219663145cd Mon Sep 17 00:00:00 2001
From: ginuerzh <ginuerzh@gmail.com>
Date: Sun, 31 May 2026 17:05:21 +0800
Subject: [PATCH] fix(handler/forward): split readTimeout from pipe idleTimeout
 to prevent 15s download abort

The readTimeout field (default 15s) was being applied via xnet.Pipe to both
directions of a bidirectional proxy connection. During asymmetric transfers
(e.g. HTTP file download), the direction reading from the tunnel sees no
data after the initial request is forwarded, causing SetReadDeadline to fire
after 15s and abort the entire transfer.

Fix: add a separate idleTimeout field (default 0=disabled) to the metadata
structs in both forward/local and forward/remote handlers, and switch
xnet.Pipe to use idleTimeout instead of readTimeout. The readTimeout field
now only applies to the initial protocol sniffing/handshake phase.

Also document readTimeout vs idleTimeout semantics across all 24 locations
in the x/ module where these timeouts appear:
- readTimeout: handshake sniffing deadline (handlers), upstream response
  header timeout (http.Transport), or transport-level read deadline
- idleTimeout: idle read deadline per Pipe direction (0=disabled)
- ReadTimeout on Sniffer/SnifferBuilder: upstream response header/TLS
  handshake read timeout during sniffing
---
 handler/dns/metadata.go            |  5 +++++
 handler/forward/local/forward.go   |  2 +-
 handler/forward/local/metadata.go  | 15 +++++++++++++++
 handler/forward/local/sniffing.go  |  3 +++
 handler/forward/remote/forward.go  |  2 +-
 handler/forward/remote/metadata.go | 15 +++++++++++++++
 handler/forward/remote/sniffing.go |  3 +++
 handler/http/connect.go            |  3 +++
 handler/http/metadata.go           |  4 ++--
 handler/http2/metadata.go          |  2 +-
 handler/redirect/tcp/metadata.go   |  5 +++++
 handler/relay/metadata.go          |  6 ++++++
 handler/router/metadata.go         |  5 +++++
 handler/sni/metadata.go            |  6 ++++++
 handler/socks/v4/metadata.go       |  6 ++++++
 handler/socks/v5/metadata.go       |  6 ++++++
 handler/ss/metadata.go             |  6 ++++++
 handler/ss/udp/metadata.go         |  5 +++++
 handler/sshd/metadata.go           |  6 ++++++
 handler/tunnel/entrypoint.go       |  3 +++
 handler/tunnel/metadata.go         |  6 +++++-
 handler/unix/metadata.go           |  6 ++++++
 internal/util/forwarder/sniffer.go |  7 +++++++
 internal/util/sniffing/sniffer.go  |  7 +++++++
 24 files changed, 128 insertions(+), 6 deletions(-)

diff --git a/handler/dns/metadata.go b/handler/dns/metadata.go
index 3acf78a2..5d9c1107 100644
--- a/handler/dns/metadata.go
+++ b/handler/dns/metadata.go
@@ -18,6 +18,11 @@ const (
 
 // metadata holds parsed DNS handler configuration.
 type metadata struct {
+	// readTimeout is the deadline for reading DNS query and writing DNS
+	// response on the client connection. Each DNS exchange (one query +
+	// one response) must complete within this window.
+	// Default: 0 (no timeout set by handler — DNS upstream timeout is
+	// controlled separately by the "timeout" field).
 	readTimeout time.Duration
 	ttl         time.Duration
 	timeout     time.Duration
diff --git a/handler/forward/local/forward.go b/handler/forward/local/forward.go
index 3be86618..a20ab4e3 100644
--- a/handler/forward/local/forward.go
+++ b/handler/forward/local/forward.go
@@ -82,7 +82,7 @@ func (h *forwardHandler) handleRawForwarding(ctx context.Context, conn net.Conn,
 
 	t := time.Now()
 	log.Infof("%s <-> %s", conn.RemoteAddr(), target.Addr)
-	if err := xnet.Pipe(ctx, conn, cc, xnet.WithReadTimeout(h.md.readTimeout)); err != nil {
+	if err := xnet.Pipe(ctx, conn, cc, xnet.WithReadTimeout(h.md.idleTimeout)); err != nil {
 		log.Debugf("pipe: %v", err)
 	}
 	log.WithFields(map[string]any{
diff --git a/handler/forward/local/metadata.go b/handler/forward/local/metadata.go
index 68d2b834..7359bda7 100644
--- a/handler/forward/local/metadata.go
+++ b/handler/forward/local/metadata.go
@@ -13,7 +13,17 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for the initial protocol sniffing phase
+	// (used by SnifferBuilder.ReadTimeout). After sniffing completes and
+	// raw forwarding begins, this timeout no longer applies.
+	// Default: 15s.
 	readTimeout   time.Duration
+	// idleTimeout is the idle read timeout applied to each direction of
+	// the bidirectional pipe (xnet.Pipe). A value of 0 or negative disables
+	// the timeout entirely, relying on TCP keepalives and context
+	// cancellation to detect dead connections.
+	// Default: 0 (disabled).
+	idleTimeout   time.Duration
 	httpKeepalive bool
 	proxyProtocol int
 
@@ -34,6 +44,11 @@ func (h *forwardHandler) parseMetadata(md mdata.Metadata) (err error) {
 		h.md.readTimeout = 15 * time.Second
 	}
 
+	h.md.idleTimeout = mdutil.GetDuration(md, "idleTimeout")
+	if h.md.idleTimeout < 0 {
+		h.md.idleTimeout = 0
+	}
+
 	h.md.httpKeepalive = mdutil.GetBool(md, "http.keepalive")
 	h.md.proxyProtocol = mdutil.GetInt(md, "proxyProtocol")
 
diff --git a/handler/forward/local/sniffing.go b/handler/forward/local/sniffing.go
index 4484bcf3..9276349f 100644
--- a/handler/forward/local/sniffing.go
+++ b/handler/forward/local/sniffing.go
@@ -32,6 +32,9 @@ type SnifferBuilder struct {
 	ALPN                string
 	CertPool            tls_util.CertPool
 	MitmBypass          bypass.Bypass
+	// ReadTimeout is the timeout for reading upstream HTTP response headers
+	// and TLS ServerHello during sniffing. Passed through to forwarder.Sniffer.
+	// See forwarder.Sniffer.ReadTimeout for details.
 	ReadTimeout         time.Duration
 }
 
diff --git a/handler/forward/remote/forward.go b/handler/forward/remote/forward.go
index a20d21ec..3c3a0dba 100644
--- a/handler/forward/remote/forward.go
+++ b/handler/forward/remote/forward.go
@@ -78,7 +78,7 @@ func (h *forwardHandler) handleRawForwarding(ctx context.Context, conn net.Conn,
 
 	t := time.Now()
 	log.Infof("%s <-> %s", conn.RemoteAddr(), target.Addr)
-	if err := xnet.Pipe(ctx, conn, cc, xnet.WithReadTimeout(h.md.readTimeout)); err != nil {
+	if err := xnet.Pipe(ctx, conn, cc, xnet.WithReadTimeout(h.md.idleTimeout)); err != nil {
 		log.Debugf("pipe: %v", err)
 	}
 	log.WithFields(map[string]any{
diff --git a/handler/forward/remote/metadata.go b/handler/forward/remote/metadata.go
index ce237269..79a2728c 100644
--- a/handler/forward/remote/metadata.go
+++ b/handler/forward/remote/metadata.go
@@ -13,7 +13,17 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for the initial protocol sniffing phase
+	// (used by SnifferBuilder.ReadTimeout). After sniffing completes and
+	// raw forwarding begins, this timeout no longer applies.
+	// Default: 15s.
 	readTimeout   time.Duration
+	// idleTimeout is the idle read timeout applied to each direction of
+	// the bidirectional pipe (xnet.Pipe). A value of 0 or negative disables
+	// the timeout entirely, relying on TCP keepalives and context
+	// cancellation to detect dead connections.
+	// Default: 0 (disabled).
+	idleTimeout   time.Duration
 	httpKeepalive bool
 	proxyProtocol int
 
@@ -34,6 +44,11 @@ func (h *forwardHandler) parseMetadata(md mdata.Metadata) (err error) {
 		h.md.readTimeout = 15 * time.Second
 	}
 
+	h.md.idleTimeout = mdutil.GetDuration(md, "idleTimeout")
+	if h.md.idleTimeout < 0 {
+		h.md.idleTimeout = 0
+	}
+
 	h.md.httpKeepalive = mdutil.GetBool(md, "http.keepalive")
 	h.md.proxyProtocol = mdutil.GetInt(md, "proxyProtocol")
 
diff --git a/handler/forward/remote/sniffing.go b/handler/forward/remote/sniffing.go
index 5b89d73b..bc518dfa 100644
--- a/handler/forward/remote/sniffing.go
+++ b/handler/forward/remote/sniffing.go
@@ -32,6 +32,9 @@ type SnifferBuilder struct {
 	ALPN                string
 	CertPool            tls_util.CertPool
 	MitmBypass          bypass.Bypass
+	// ReadTimeout is the timeout for reading upstream HTTP response headers
+	// and TLS ServerHello during sniffing. Passed through to forwarder.Sniffer.
+	// See forwarder.Sniffer.ReadTimeout for details.
 	ReadTimeout         time.Duration
 }
 
diff --git a/handler/http/connect.go b/handler/http/connect.go
index 3edfa32a..8f9942aa 100644
--- a/handler/http/connect.go
+++ b/handler/http/connect.go
@@ -230,6 +230,9 @@ type SnifferBuilder struct {
 	ALPN                string
 	CertPool            tls_util.CertPool
 	MitmBypass          bypass.Bypass
+	// ReadTimeout is the timeout for reading upstream HTTP response headers
+	// and TLS ServerHello during sniffing. Passed through to sniffing.Sniffer.
+	// See sniffing.Sniffer.ReadTimeout for details.
 	ReadTimeout         time.Duration
 }
 
diff --git a/handler/http/metadata.go b/handler/http/metadata.go
index c327b2d0..41f03626 100644
--- a/handler/http/metadata.go
+++ b/handler/http/metadata.go
@@ -23,8 +23,8 @@ const (
 // Fields are populated by parseMetadata and read throughout the handler's
 // request-processing methods.
 type metadata struct {
-	readTimeout time.Duration // read timeout for upstream responses; default 15s, negative disables
-	idleTimeout time.Duration // idle timeout for pipe forwarding; 0 means disabled
+	readTimeout time.Duration // deadline for upstream response headers (http.Transport.ResponseHeaderTimeout); 0=15s default, negative=disabled
+	idleTimeout time.Duration // idle read deadline per Pipe direction during CONNECT/forwarding; 0 or negative = disabled
 	keepalive   bool          // enable HTTP keep-alive on the upstream transport
 	compression bool          // enable HTTP compression on the upstream transport
 
diff --git a/handler/http2/metadata.go b/handler/http2/metadata.go
index 49597a63..216a05ca 100644
--- a/handler/http2/metadata.go
+++ b/handler/http2/metadata.go
@@ -20,7 +20,7 @@ type metadata struct {
 	authBasicRealm       string
 	observerPeriod       time.Duration
 	observerResetTraffic bool
-	idleTimeout          time.Duration
+	idleTimeout time.Duration // idle read deadline per Pipe direction during forwarding; read from "readTimeout" or "read.timeout" metadata key. 0 = disabled.
 
 	limiterRefreshInterval time.Duration
 	limiterCleanupInterval time.Duration
diff --git a/handler/redirect/tcp/metadata.go b/handler/redirect/tcp/metadata.go
index 77865c0e..d0045a22 100644
--- a/handler/redirect/tcp/metadata.go
+++ b/handler/redirect/tcp/metadata.go
@@ -13,6 +13,11 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is passed to SnifferBuilder as the timeout for reading
+	// upstream response headers during HTTP/TLS sniffing. It is NOT used
+	// as a deadline on the initial client connection (unlike socks/ss
+	// handlers), because redirect/tcp has no protocol handshake.
+	// 0 or negative defaults to 15s.
 	readTimeout time.Duration
 	tproxy      bool
 
diff --git a/handler/relay/metadata.go b/handler/relay/metadata.go
index c7d09838..e61b927b 100644
--- a/handler/relay/metadata.go
+++ b/handler/relay/metadata.go
@@ -14,6 +14,12 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for reading the initial relay protocol
+	// handshake (relay.Request) from the client connection. The deadline
+	// is cleared immediately after the handshake completes, so it does
+	// not affect subsequent data transfer. Also passed to SnifferBuilder
+	// for the upstream response header read timeout.
+	// 0 or negative defaults to 15s.
 	readTimeout   time.Duration
 	udpBufferSize int
 	enableBind    bool
diff --git a/handler/router/metadata.go b/handler/router/metadata.go
index e3c125b0..c663ebc4 100644
--- a/handler/router/metadata.go
+++ b/handler/router/metadata.go
@@ -18,6 +18,11 @@ const (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for reading the initial relay protocol
+	// handshake from the client connection. The deadline is cleared
+	// after the handshake, so it does not affect subsequent data
+	// transfer.
+	// 0 or negative means no timeout is applied.
 	readTimeout time.Duration
 	bufferSize  int
 
diff --git a/handler/sni/metadata.go b/handler/sni/metadata.go
index 06852bc4..5f00e62d 100644
--- a/handler/sni/metadata.go
+++ b/handler/sni/metadata.go
@@ -13,6 +13,12 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is passed to SnifferBuilder as the timeout for reading
+	// upstream response headers during HTTP/TLS sniffing. It is NOT used
+	// as a deadline on the initial client connection (unlike socks/ss
+	// handlers), because SNI routing has no protocol handshake beyond
+	// the TLS ClientHello which is parsed during sniffing.
+	// 0 or negative defaults to 15s.
 	readTimeout time.Duration
 	hash        string
 
diff --git a/handler/socks/v4/metadata.go b/handler/socks/v4/metadata.go
index b3e7550c..8cfe04e2 100644
--- a/handler/socks/v4/metadata.go
+++ b/handler/socks/v4/metadata.go
@@ -13,6 +13,12 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for reading the initial SOCKS4/4A
+	// handshake (connect/bind request) from the client connection. The
+	// deadline is cleared after the handshake, so it does not affect
+	// subsequent data transfer. Also passed to SnifferBuilder for the
+	// upstream response header read timeout.
+	// 0 or negative defaults to 15s.
 	readTimeout            time.Duration
 	hash                   string
 
diff --git a/handler/socks/v5/metadata.go b/handler/socks/v5/metadata.go
index 2c75645c..59aa2fb0 100644
--- a/handler/socks/v5/metadata.go
+++ b/handler/socks/v5/metadata.go
@@ -15,6 +15,12 @@ import (
 
 type metadata struct {
 	publicAddr        string
+	// readTimeout is the deadline for reading the initial SOCKS5
+	// handshake (auth + connect/associate/udp request) from the client
+	// connection. The deadline is cleared after the handshake, so it
+	// does not affect subsequent data transfer. Also passed to
+	// SnifferBuilder for the upstream response header read timeout.
+	// 0 or negative defaults to 15s.
 	readTimeout       time.Duration
 	noTLS             bool
 	enableBind        bool
diff --git a/handler/ss/metadata.go b/handler/ss/metadata.go
index 2683f482..1e9a00e2 100644
--- a/handler/ss/metadata.go
+++ b/handler/ss/metadata.go
@@ -16,6 +16,12 @@ import (
 type metadata struct {
 	key         string
 	hash        string
+	// readTimeout is the deadline for reading the initial Shadowsocks
+	// handshake from the client connection. The deadline is cleared
+	// after the handshake, so it does not affect subsequent data
+	// transfer. Also passed to SnifferBuilder for the upstream response
+	// header read timeout.
+	// 0 or negative defaults to 15s.
 	readTimeout time.Duration
 
 	sniffing                    bool
diff --git a/handler/ss/udp/metadata.go b/handler/ss/udp/metadata.go
index 03a6db8a..22b22212 100644
--- a/handler/ss/udp/metadata.go
+++ b/handler/ss/udp/metadata.go
@@ -14,6 +14,11 @@ const (
 
 type metadata struct {
 	key           string
+	// readTimeout is the deadline for reading a single UDP datagram from
+	// the client. Since UDP is connectionless, this timeout applies to
+	// each individual datagram read. Default: 1 minute (longer than the
+	// typical 15s used by TCP handlers to accommodate the stateless
+	// nature of UDP).
 	readTimeout   time.Duration
 	udpBufferSize int
 
diff --git a/handler/sshd/metadata.go b/handler/sshd/metadata.go
index 01cbab57..7ebccc89 100644
--- a/handler/sshd/metadata.go
+++ b/handler/sshd/metadata.go
@@ -13,6 +13,12 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for reading the initial SSH handshake
+	// from the client connection. The deadline is cleared after the
+	// handshake, so it does not affect subsequent data transfer. Also
+	// passed to SnifferBuilder for the upstream response header read
+	// timeout.
+	// 0 or negative defaults to 15s.
 	readTimeout time.Duration
 
 	sniffing                    bool
diff --git a/handler/tunnel/entrypoint.go b/handler/tunnel/entrypoint.go
index abd2f7d6..3811bfd1 100644
--- a/handler/tunnel/entrypoint.go
+++ b/handler/tunnel/entrypoint.go
@@ -63,6 +63,9 @@ type entrypoint struct {
 	sniffingWebsocket   bool
 	websocketSampleRate float64
 
+	// readTimeout is applied as SetReadDeadline on the upstream connection
+	// before sniffing HTTP/TLS reads. It mirrors entryPointReadTimeout
+	// from the handler metadata.
 	readTimeout time.Duration
 }
 
diff --git a/handler/tunnel/metadata.go b/handler/tunnel/metadata.go
index c6310eda..4b0fa75d 100644
--- a/handler/tunnel/metadata.go
+++ b/handler/tunnel/metadata.go
@@ -21,6 +21,10 @@ const (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for reading the initial relay protocol
+	// handshake from the client connection. The deadline is cleared
+	// after the handshake, so it does not affect subsequent data
+	// transfer. 0 or negative means no timeout is applied.
 	readTimeout time.Duration
 
 	entrypoints []entrypointConfig
@@ -30,7 +34,7 @@ type metadata struct {
 	entryPointProxyProtocol     int
 	entryPointKeepalive         bool
 	entryPointCompression       bool
-	entryPointReadTimeout       time.Duration
+	entryPointReadTimeout       time.Duration // deadline for reading upstream HTTP response headers in the entrypoint's http.Transport.ResponseHeaderTimeout. Also passed as readTimeout to entrypoint dialer for SetReadDeadline on upstream conn. 0 or negative defaults to 15s.
 	sniffingWebsocket           bool
 	sniffingWebsocketSampleRate float64
 
diff --git a/handler/unix/metadata.go b/handler/unix/metadata.go
index 4430f8bb..c0bfd558 100644
--- a/handler/unix/metadata.go
+++ b/handler/unix/metadata.go
@@ -13,6 +13,12 @@ import (
 )
 
 type metadata struct {
+	// readTimeout is the deadline for reading data from the upstream
+	// (the target Unix socket or pipe). It is set on the upstream conn
+	// via SetReadDeadline before each sniffing HTTP/TLS read. Also
+	// passed to SnifferBuilder for the upstream response header read
+	// timeout.
+	// 0 or negative defaults to 15s.
 	readTimeout time.Duration
 
 	sniffing        bool
diff --git a/internal/util/forwarder/sniffer.go b/internal/util/forwarder/sniffer.go
index 42a3e5e3..d115a56d 100644
--- a/internal/util/forwarder/sniffer.go
+++ b/internal/util/forwarder/sniffer.go
@@ -122,6 +122,13 @@ type Sniffer struct {
 	CertPool           tls_util.CertPool
 	MitmBypass         bypass.Bypass
 
+	// ReadTimeout is the deadline for reading the upstream response
+	// headers during HTTP sniffing (http.ReadResponse) and the TLS
+	// ServerHello during TLS sniffing. This timeout is applied once
+	// per request/response pair in the HTTP keep-alive loop and cleared
+	// after each response is received. It does NOT affect the client
+	// connection or the response body transfer.
+	// Default: DefaultReadTimeout (30s) if not set.
 	ReadTimeout time.Duration
 }
 
diff --git a/internal/util/sniffing/sniffer.go b/internal/util/sniffing/sniffer.go
index 2e44be11..da5ddcea 100644
--- a/internal/util/sniffing/sniffer.go
+++ b/internal/util/sniffing/sniffer.go
@@ -103,6 +103,13 @@ type Sniffer struct {
 	CertPool           tls_util.CertPool
 	MitmBypass         bypass.Bypass
 
+	// ReadTimeout is the deadline for reading the upstream response
+	// headers during HTTP sniffing (http.ReadResponse) and the TLS
+	// ServerHello during TLS sniffing. This timeout is applied once
+	// per request/response pair in the HTTP keep-alive loop and cleared
+	// after each response is received. It does NOT affect the client
+	// connection or the response body transfer.
+	// Default: DefaultReadTimeout (30s) if not set.
 	ReadTimeout time.Duration
 }