http2: don't sniff first Request.Body byte in Transport until we have a conn

bodyAndLength mutates Request.Body if Request.ContentLength == 0,
reading the first byte to determine whether it's actually empty or
just undeclared. But we did that before we checked whether our
connection was overloaded, which meant the caller could retry the
request on an new or lesser-loaded connection, but then lose the first
byte of the request.

Updates golang/go#17071 (needs bundle into std before fixed)

Change-Id: I996a92ad037b45bc49e7cf0801a2027bbbb3c920
Reviewed-on: https://go-review.googlesource.com/29074
Reviewed-by: Gleb Stepanov <[email protected]>
Reviewed-by: Chris Broadfoot <[email protected]>
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

Author: bradfitz

http2/transport.go

@@ -656,16 +656,16 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
 	}
 	hasTrailers := trailers != ""
 
-	body, contentLen := bodyAndLength(req)
-	hasBody := body != nil
-
 	cc.mu.Lock()
 	cc.lastActive = time.Now()
 	if cc.closed || !cc.canTakeNewRequestLocked() {
 		cc.mu.Unlock()
 		return nil, errClientConnUnusable
 	}
 
+	body, contentLen := bodyAndLength(req)
+	hasBody := body != nil
+
 	// TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere?
 	var requestedGzip bool
 	if !cc.t.disableCompression() &&

http2/transport_test.go

@@ -2597,3 +2597,24 @@ func TestTransportRequestPathPseudo(t *testing.T) {
 	}
 
 }
+
+// golang.org/issue/17071 -- don't sniff the first byte of the request body
+// before we've determined that the ClientConn is usable.
+func TestRoundTripDoesntConsumeRequestBodyEarly(t *testing.T) {
+	const body = "foo"
+	req, _ := http.NewRequest("POST", "http://foo.com/", ioutil.NopCloser(strings.NewReader(body)))
+	cc := &ClientConn{
+		closed: true,
+	}
+	_, err := cc.RoundTrip(req)
+	if err != errClientConnUnusable {
+		t.Fatalf("RoundTrip = %v; want errClientConnUnusable", err)
+	}
+	slurp, err := ioutil.ReadAll(req.Body)
+	if err != nil {
+		t.Errorf("ReadAll = %v", err)
+	}
+	if string(slurp) != body {
+		t.Errorf("Body = %q; want %q", slurp, body)
+	}
+}