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 <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

Author: nerdatmath

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)",

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)
+	}
+}