fix(handler/serial,sni): 5 code-review fixes - xnet.Pipe errors, (nil,nil) warning, readTimeout comment, defer deadline, clean close

- serial/handler.go: capture xnet.Pipe return value in Handle() and forwardSerial()
- serial/handler.go: log Warnf when chain dial returns (nil,nil) before falling back
- sni/handler.go: use defer for read deadline reset to prevent stale deadline
- sni/handler.go: unrecognized protocol returns nil (clean close) instead of error
- sni/metadata.go: update readTimeout comment to reflect dual usage (deadline + SnifferBuilder)
This commit is contained in:
ginuerzh
2026-06-05 21:09:10 +08:00
parent f09a7a69b3
commit 4d4690b535
3 changed files with 21 additions and 11 deletions
+13 -2
View File
@@ -179,7 +179,10 @@ func (h *serialHandler) Handle(ctx context.Context, conn net.Conn, opts ...handl
t := time.Now()
log.Infof("%s <-> %s", conn.LocalAddr(), "@")
// xnet.Transport(conn, cc)
xnet.Pipe(ctx, conn, cc)
if err := xnet.Pipe(ctx, conn, cc); err != nil {
log.Errorf("pipe: %v", err)
return err
}
log.WithFields(map[string]any{
"duration": time.Since(t),
}).Infof("%s >-< %s", conn.LocalAddr(), "@")
@@ -236,6 +239,11 @@ func (h *serialHandler) forwardSerial(ctx context.Context, conn net.Conn, target
// If the router chain returned an actual error, we skip the fallback
// and propagate the error — a configured chain is authoritative.
if port == nil && err == nil {
if h.options.Router != nil {
if opts := h.options.Router.Options(); opts != nil && opts.Chain != nil {
log.Warnf("chain dial returned no connection and no error, falling back to direct serial port")
}
}
cfg.ReadTimeout = h.md.timeout
port, err = serial.OpenPort(cfg)
}
@@ -251,7 +259,10 @@ func (h *serialHandler) forwardSerial(ctx context.Context, conn net.Conn, target
// Pipe bidirectionally between the client connection and the serial port.
// xnet.Pipe spawns two goroutines and returns on first error or when
// both directions complete.
xnet.Pipe(ctx, conn, port)
if err := xnet.Pipe(ctx, conn, port); err != nil {
log.Errorf("pipe: %v", err)
return err
}
log.WithFields(map[string]any{
"duration": time.Since(t),
}).Infof("%s >-< %s", conn.LocalAddr(), target.Addr)
+3 -4
View File
@@ -122,12 +122,10 @@ func (h *sniHandler) Handle(ctx context.Context, conn net.Conn, opts ...handler.
if h.md.readTimeout > 0 {
conn.SetReadDeadline(time.Now().Add(h.md.readTimeout))
defer conn.SetReadDeadline(time.Time{})
}
br := bufio.NewReader(conn)
proto, sniffErr := sniffing.Sniff(ctx, br)
if h.md.readTimeout > 0 {
conn.SetReadDeadline(time.Time{})
}
if sniffErr != nil {
log.Debugf("sniff: %v", sniffErr)
}
@@ -189,7 +187,8 @@ func (h *sniHandler) Handle(ctx context.Context, conn net.Conn, opts ...handler.
sniffing.WithLog(log),
)
default:
return errors.New("unknown traffic")
log.Debugf("unrecognized traffic from %s", conn.RemoteAddr())
return nil
}
}
+5 -5
View File
@@ -13,11 +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 SNI routing has no protocol handshake beyond
// the TLS ClientHello which is parsed during sniffing.
// readTimeout controls two behaviors:
// 1. A read deadline set on the client connection before sniffing the
// TLS ClientHello / HTTP request (see handler.go Handle).
// 2. Passed to SnifferBuilder as the timeout for reading upstream
// response headers during HTTP/TLS sniffing.
// 0 or negative defaults to 15s.
readTimeout time.Duration
hash string