From 5a93142cec6c4da528e12939398a615869361d5a Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Mon, 8 Nov 2021 11:23:27 -0800 Subject: [release-branch.go1.16] net/http: do not cancel request context on response body read 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 Run-TryBot: Damien Neil TryBot-Result: Go Bot Reviewed-by: Brad Fitzpatrick (cherry picked from commit 76fbd6167364fb98e3ebe946cfc16b5b84d4240e) Reviewed-on: https://go-review.googlesource.com/c/go/+/368084 Reviewed-by: Michael Knyszek --- src/net/http/client.go | 1 - src/net/http/client_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/net/http/client.go b/src/net/http/client.go index 88e2028bc3..5cb39f1c9a 100644 --- 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 d90b4841c6..a182743d52 100644 --- 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) { -- cgit v1.2.3-54-g00ecf