Skip to content

Commit

Permalink
[release-branch.go1.17] net/http: do not cancel request context on re…
Browse files Browse the repository at this point in the history
…sponse 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).

Fixes #49559.
For #49366.

Change-Id: Ieaed866116916261d9079f71d8fea7a7b303b8fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/361919
Trust: Damien Neil <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
(cherry picked from commit 76fbd61)
Reviewed-on: https://go-review.googlesource.com/c/go/+/368085
Reviewed-by: Michael Knyszek <[email protected]>
  • Loading branch information
neild authored and mknyszek committed Dec 1, 2021
1 parent f0ee7c6 commit ab17593
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
1 change: 0 additions & 1 deletion src/net/http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,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
}
Expand Down
27 changes: 27 additions & 0 deletions src/net/http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit ab17593

Please sign in to comment.