[release-branch.go1.16] net/http: do not cancel request context on response body...

When sending a Request with a non-context deadline, we create a
context with a timeout. This context is canceled when closing the
response body, and also if a read from the response body returns
an error (including io.EOF).

Cancelling the context in Response.Body.Read interferes with the
HTTP/2 client cleaning up after a request is completed, and is
unnecessary: The user should always close the body, the impact
from not canceling the context is minor (the context timer leaks
until it fires).

For #49366.
Fixes #49558.

Change-Id: Ieaed866116916261d9079f71d8fea7a7b303b8fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/361919
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 76fbd6167364fb98e3ebe946cfc16b5b84d4240e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/368084
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 88e2028bc34904e85629ad8cfa3ac92083ca6529..5cb39f1c9af8c0cf9661d5bbacf32d67eb688807 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -940,7 +940,6 @@ func (b *cancelTimerBody) Read(p []byte) (n int, err error) {
        if err == nil {
                return n, nil
        }
-       b.stop()
        if err == io.EOF {
                return n, err
        }

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index d90b4841c66cb661065bc67589c8b986e9d64bf0..a182743d52b768fd9f3cd88f5bcde5d1d14e6777 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -1353,6 +1353,33 @@ func TestClientTimeoutCancel(t *testing.T) {
        }
 }
 
+func TestClientTimeoutDoesNotExpire_h1(t *testing.T) { testClientTimeoutDoesNotExpire(t, h1Mode) }
+func TestClientTimeoutDoesNotExpire_h2(t *testing.T) { testClientTimeoutDoesNotExpire(t, h2Mode) }
+
+// Issue 49366: if Client.Timeout is set but not hit, no error should be returned.
+func testClientTimeoutDoesNotExpire(t *testing.T, h2 bool) {
+       setParallel(t)
+       defer afterTest(t)
+
+       cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
+               w.Write([]byte("body"))
+       }))
+       defer cst.close()
+
+       cst.c.Timeout = 1 * time.Hour
+       req, _ := NewRequest("GET", cst.ts.URL, nil)
+       res, err := cst.c.Do(req)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if _, err = io.Copy(io.Discard, res.Body); err != nil {
+               t.Fatalf("io.Copy(io.Discard, res.Body) = %v, want nil", err)
+       }
+       if err = res.Body.Close(); err != nil {
+               t.Fatalf("res.Body.Close() = %v, want nil", err)
+       }
+}
+
 func TestClientRedirectEatsBody_h1(t *testing.T) { testClientRedirectEatsBody(t, h1Mode) }
 func TestClientRedirectEatsBody_h2(t *testing.T) { testClientRedirectEatsBody(t, h2Mode) }
 func testClientRedirectEatsBody(t *testing.T, h2 bool) {