From 09332743ad6d5a9eb1137adaade2810c583d38ca Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Tue, 30 Aug 2022 16:19:13 -0700
Subject: [PATCH] net/http: don't time out idle server connections after
 ReadHeaderTimeout

Consistently wait for idle connections to become readable before
starting the ReadHeaderTimeout timer. Previously, connections with no
idle timeout skipped directly to reading headers, so the
ReadHeaderTimeout also included time spent idle.

Fixes #54784

Change-Id: Ifba48f16c9a747647086ea9da6b6dcd54c4672e7
---
 src/net/http/serve_test.go | 52 ++++++++++++++++++++++++++++++++++++++
 src/net/http/server.go     | 14 +++++++---
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index d28bfba759..143874d70a 100644
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -5843,6 +5843,58 @@ func TestServerCancelsReadTimeoutWhenIdle(t *testing.T) {
 	})
 }
 
+// Issue 54784: test that the Server's ReadHeaderTimeout only starts once the
+// beginning of a request has been received, rather than including time the
+// connection spent idle.
+func TestServerCancelsReadHeaderTimeoutWhenIdle(t *testing.T) {
+	setParallel(t)
+	defer afterTest(t)
+	runTimeSensitiveTest(t, []time.Duration{
+		10 * time.Millisecond,
+		50 * time.Millisecond,
+		250 * time.Millisecond,
+		time.Second,
+		2 * time.Second,
+	}, func(t *testing.T, timeout time.Duration) error {
+		ts := httptest.NewUnstartedServer(serve(200))
+		ts.Config.ReadHeaderTimeout = timeout
+		ts.Config.IdleTimeout = 0 // disable idle timeout
+		ts.Start()
+		defer ts.Close()
+
+		// rather than using an http.Client, create a single connection, so that
+		// we can ensure this connection is not closed.
+		conn, err := net.Dial("tcp", ts.Listener.Addr().String())
+		if err != nil {
+			t.Fatalf("dial failed: %v", err)
+		}
+		br := bufio.NewReader(conn)
+		defer conn.Close()
+
+		if _, err := conn.Write([]byte("GET / HTTP/1.1\r\nHost: e.com\r\n\r\n")); err != nil {
+			t.Fatalf("writing first request failed: %v", err)
+		}
+
+		if _, err := ReadResponse(br, nil); err != nil {
+			t.Fatalf("first response (before timeout) failed: %v", err)
+		}
+
+		// wait for longer than the server's ReadHeaderTimeout, and then send
+		// another request
+		time.Sleep(timeout + 10*time.Millisecond)
+
+		if _, err := conn.Write([]byte("GET / HTTP/1.1\r\nHost: e.com\r\n\r\n")); err != nil {
+			t.Fatalf("writing second request failed: %v", err)
+		}
+
+		if _, err := ReadResponse(br, nil); err != nil {
+			t.Fatalf("second response (after timeout) failed: %v", err)
+		}
+
+		return nil
+	})
+}
+
 // runTimeSensitiveTest runs test with the provided durations until one passes.
 // If they all fail, t.Fatal is called with the last one's duration and error value.
 func runTimeSensitiveTest(t *testing.T, durations []time.Duration, test func(t *testing.T, d time.Duration) error) {
diff --git a/src/net/http/server.go b/src/net/http/server.go
index 9aea1b8002..3d427e5ae4 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2002,10 +2002,18 @@ func (c *conn) serve(ctx context.Context) {
 
 		if d := c.server.idleTimeout(); d != 0 {
 			c.rwc.SetReadDeadline(time.Now().Add(d))
-			if _, err := c.bufr.Peek(4); err != nil {
-				return
-			}
+		} else {
+			c.rwc.SetReadDeadline(time.Time{})
 		}
+
+		// Wait for the connection to become readable again before trying to
+		// read the next request. This prevents a ReadHeaderTimeout or
+		// ReadTimeout from starting until the first bytes of the next request
+		// have been received.
+		if _, err := c.bufr.Peek(4); err != nil {
+			return
+		}
+
 		c.rwc.SetReadDeadline(time.Time{})
 	}
 }