net/http: fix double-close of req.Body

Add a test and fix for the request body being closed twice.

Fixes #19186

Change-Id: I1e35ad4aebfef68e6099c1dba7986883afdef4d7
Reviewed-on: https://go-review.googlesource.com/37298
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/client.go b/src/net/http/client.go
index fbdc41bdf2cb073ea0d335c81ef2c03d1b3048ab..4c9084ae512df9c89a487639072ac3489fb95d5f 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -494,17 +494,21 @@ func (c *Client) Do(req *Request) (*Response, error) {
        }
 
        var (
-               deadline    = c.deadline()
-               reqs        []*Request
-               resp        *Response
-               copyHeaders = c.makeHeadersCopier(req)
+               deadline      = c.deadline()
+               reqs          []*Request
+               resp          *Response
+               copyHeaders   = c.makeHeadersCopier(req)
+               reqBodyClosed = false // have we closed the current req.Body?
 
                // Redirect behavior:
                redirectMethod string
                includeBody    bool
        )
        uerr := func(err error) error {
-               req.closeBody()
+               // the body may have been closed already by c.send()
+               if !reqBodyClosed {
+                       req.closeBody()
+               }
                method := valueOrDefault(reqs[0].Method, "GET")
                var urlStr string
                if resp != nil && resp.Request != nil {
@@ -596,6 +600,8 @@ func (c *Client) Do(req *Request) (*Response, error) {
                var err error
                var didTimeout func() bool
                if resp, didTimeout, err = c.send(req, deadline); err != nil {
+                       // c.send() always closes req.Body
+                       reqBodyClosed = true
                        if !deadline.IsZero() && didTimeout() {
                                err = &httpError{
                                        err:     err.Error() + " (Client.Timeout exceeded while awaiting headers)",

diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go
index 53556a1107d8a14f98e033abff084de474c6ea4e..8738c8ff7c19e3419dae7bee4bb3e0614b5ab210 100644 (file)
--- a/src/net/http/clientserver_test.go
+++ b/src/net/http/clientserver_test.go
@@ -1381,3 +1381,30 @@ func testServerUndeclaredTrailers(t *testing.T, h2 bool) {
                t.Errorf("Trailer = %#v; want %#v", res.Trailer, want)
        }
 }
+
+func TestBadResponseAfterReadingBody(t *testing.T) {
+       defer afterTest(t)
+       cst := newClientServerTest(t, false, HandlerFunc(func(w ResponseWriter, r *Request) {
+               _, err := io.Copy(ioutil.Discard, r.Body)
+               if err != nil {
+                       t.Fatal(err)
+               }
+               c, _, err := w.(Hijacker).Hijack()
+               if err != nil {
+                       t.Fatal(err)
+               }
+               defer c.Close()
+               fmt.Fprintln(c, "some bogus crap")
+       }))
+       defer cst.close()
+
+       closes := 0
+       res, err := cst.c.Post(cst.ts.URL, "text/plain", countCloseReader{&closes, strings.NewReader("hello")})
+       if err == nil {
+               res.Body.Close()
+               t.Fatal("expected an error to be returned from Post")
+       }
+       if closes != 1 {
+               t.Errorf("closes = %d; want 1", closes)
+       }
+}