diff --git a/cmd/common-main.go b/cmd/common-main.go index 393ab17d0..c127759fc 100644 --- a/cmd/common-main.go +++ b/cmd/common-main.go @@ -445,6 +445,7 @@ func buildServerCtxt(ctx *cli.Context, ctxt *serverCtxt) (err error) { ctxt.SendBufSize = ctx.Int("send-buf-size") ctxt.RecvBufSize = ctx.Int("recv-buf-size") ctxt.IdleTimeout = ctx.Duration("idle-timeout") + ctxt.ReadHeaderTimeout = ctx.Duration("read-header-timeout") ctxt.UserTimeout = ctx.Duration("conn-user-timeout") if conf := ctx.String("config"); len(conf) > 0 { diff --git a/cmd/server-main.go b/cmd/server-main.go index 48ed0f87d..aa450faee 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -905,7 +905,11 @@ func serverMain(ctx *cli.Context) { UseHandler(setCriticalErrorHandler(corsHandler(handler))). UseTLSConfig(newTLSConfig(getCert)). UseIdleTimeout(globalServerCtxt.IdleTimeout). - UseReadTimeout(globalServerCtxt.IdleTimeout). + // WriteTimeout only resets the activity-based write deadline + // that deadlineconn enforces per response write. ReadTimeout is + // left at zero: with native read deadlines honored again it would + // cap the whole request including large uploads; request bodies + // are bounded per-read by xhttp instead. UseWriteTimeout(globalServerCtxt.IdleTimeout). UseReadHeaderTimeout(globalServerCtxt.ReadHeaderTimeout). UseBaseContext(GlobalContext). diff --git a/internal/http/listener.go b/internal/http/listener.go index bc6de3af9..79345420d 100644 --- a/internal/http/listener.go +++ b/internal/http/listener.go @@ -70,7 +70,12 @@ func (listener *httpListener) Accept() (conn net.Conn, err error) { if result.err != nil { return nil, result.err } - return deadlineconn.New(result.conn).WithReadDeadline(listener.opts.IdleTimeout).WithWriteDeadline(listener.opts.IdleTimeout), result.err + // The read side must stay on native deadlines: an activity-based + // read deadline would extend the absolute ReadHeaderTimeout set by + // net/http on every partial read, letting slow-header clients keep + // connections open indefinitely (slowloris). Request bodies are + // bounded per-read by idleTimeoutBody in the server instead. + return deadlineconn.New(result.conn).WithWriteDeadline(listener.opts.IdleTimeout), result.err case <-listener.ctxDoneCh: } return nil, syscall.EINVAL diff --git a/internal/http/server-deadline_test.go b/internal/http/server-deadline_test.go new file mode 100644 index 000000000..8bfb7bc53 --- /dev/null +++ b/internal/http/server-deadline_test.go @@ -0,0 +1,157 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-or-later + +package http + +import ( + "context" + "io" + "net" + "net/http" + "strings" + "testing" + "time" +) + +// startDeadlineTestServer starts a full xhttp server (listener + wrapped +// handler, mirroring the server-main timeout layout) on an ephemeral port. +func startDeadlineTestServer(t *testing.T, handler http.Handler, idle, readHeaderTimeout time.Duration) string { + t.Helper() + + srv := NewServer([]string{"127.0.0.1:0"}). + UseHandler(handler). + UseIdleTimeout(idle). + UseWriteTimeout(idle). + UseReadHeaderTimeout(readHeaderTimeout). + // Mirror server-main, which routes the idle timeout to the listener + // (and from there to deadlineconn) through TCPOptions. + UseTCPOptions(TCPOptions{IdleTimeout: idle}) + + serveFn, err := srv.Init(context.Background(), func(listenAddr string, err error) { + t.Fatalf("listen %s: %v", listenAddr, err) + }) + if err != nil { + t.Fatalf("Init: %v", err) + } + go serveFn() + t.Cleanup(func() { srv.Shutdown() }) + + srv.listenerMutex.Lock() + defer srv.listenerMutex.Unlock() + return srv.listener.Addr().String() +} + +// A client that keeps trickling header bytes must have its connection cut +// once ReadHeaderTimeout elapses in total, not per byte. +func TestServerSlowHeaderConnectionKilled(t *testing.T) { + addr := startDeadlineTestServer(t, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), + 5*time.Second, 1*time.Second) + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + // Send a partial header line, then dribble one byte at a time well + // within the idle timeout, so only the absolute deadline can kill it. + if _, err := conn.Write([]byte("GET / HTTP/1.1\r\nHost: 127.0.0.1\r\nX-Slow: ")); err != nil { + t.Fatalf("write header: %v", err) + } + go func() { + for i := 0; ; i++ { + if _, err := conn.Write([]byte{byte('a' + i%26)}); err != nil { + return + } + time.Sleep(100 * time.Millisecond) + } + }() + + conn.SetReadDeadline(time.Now().Add(4 * time.Second)) + start := time.Now() + buf := make([]byte, 1) + for { + _, err := conn.Read(buf) + if err != nil { + break + } + } + if elapsed := time.Since(start); elapsed > 2*time.Second { + t.Fatalf("slow-header connection survived %v, want it closed within ReadHeaderTimeout+slack", elapsed) + } +} + +// A request whose body stalls mid-transfer must be cut off by the per-read +// idle deadline, while the handler observes the read error. +func TestServerStalledRequestBodyKilled(t *testing.T) { + bodyResult := make(chan error, 1) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, err := io.Copy(io.Discard, r.Body) + bodyResult <- err + }) + addr := startDeadlineTestServer(t, handler, 1*time.Second, 1*time.Second) + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + head := "POST / HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: 100\r\n\r\n" + if _, err := conn.Write([]byte(head + strings.Repeat("x", 10))); err != nil { + t.Fatalf("write: %v", err) + } + // Stall: no further body bytes are ever sent. + + select { + case err := <-bodyResult: + if err == nil { + t.Fatal("handler completed a request whose body stalled forever") + } + case <-time.After(3 * time.Second): + t.Fatal("stalled body kept the handler blocked past the idle deadline") + } +} + +// A body that keeps making progress, even slower than the idle timeout in +// total, must complete: the deadline is per-read activity, never absolute. +func TestServerSlowProgressingBodyAccepted(t *testing.T) { + bodyResult := make(chan int, 1) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n, _ := io.Copy(io.Discard, r.Body) + bodyResult <- int(n) + }) + addr := startDeadlineTestServer(t, handler, 500*time.Millisecond, 1*time.Second) + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + const total = 100 + head := "POST / HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: 100\r\n\r\n" + if _, err := conn.Write([]byte(head)); err != nil { + t.Fatalf("write: %v", err) + } + // 20ms per byte: the whole body takes 2s, four times the idle timeout, + // but every inter-byte gap stays far below it. + go func() { + for i := 0; i < total; i++ { + if _, err := conn.Write([]byte{'y'}); err != nil { + return + } + time.Sleep(20 * time.Millisecond) + } + }() + + select { + case n := <-bodyResult: + if n != total { + t.Fatalf("handler read %d bytes, want %d", n, total) + } + case <-time.After(5 * time.Second): + t.Fatal("progressing slow body was terminated") + } +} diff --git a/internal/http/server.go b/internal/http/server.go index 2934fda6c..c63b59645 100644 --- a/internal/http/server.go +++ b/internal/http/server.go @@ -21,6 +21,7 @@ import ( "context" "crypto/tls" "errors" + "io" "log" "net" "net/http" @@ -114,6 +115,19 @@ func (srv *Server) Init(listenCtx context.Context, listenErrCallback func(listen atomic.AddInt32(&srv.requestCount, 1) defer atomic.AddInt32(&srv.requestCount, -1) + // Bound request bodies by read activity: refresh the connection + // read deadline ahead of every body read so a stalled upload is + // cut off while a slow but progressing one is never terminated. + // This replaces the connection-level read idle timeout that had + // to be dropped to keep ReadHeaderTimeout absolute (slowloris). + if idle := srv.IdleTimeout; idle > 0 && r.Body != nil && r.Body != http.NoBody { + r.Body = &idleTimeoutBody{ + rc: http.NewResponseController(w), + body: r.Body, + idle: idle, + } + } + // Handle request using passed handler. handler.ServeHTTP(w, r) }) @@ -217,6 +231,28 @@ func (srv *Server) UseTCPOptions(opts TCPOptions) *Server { return srv } +// idleTimeoutBody wraps a request body and refreshes the connection read +// deadline ahead of every read, giving bodies an activity-based timeout: +// reads that stall longer than idle fail with a timeout error while reads +// that keep making progress extend the deadline indefinitely. +type idleTimeoutBody struct { + rc *http.ResponseController + body io.ReadCloser + idle time.Duration +} + +func (b *idleTimeoutBody) Read(p []byte) (n int, err error) { + // Best effort: on connections where deadline control is unavailable + // (e.g. hijacked) the call is a no-op and reads stay unbounded, as + // they were before. + _ = b.rc.SetReadDeadline(time.Now().Add(b.idle)) + return b.body.Read(p) +} + +func (b *idleTimeoutBody) Close() error { + return b.body.Close() +} + // NewServer - creates new HTTP server using given arguments. func NewServer(addrs []string) *Server { httpServer := &Server{