net/http/internal/http2: don't reuse ClientRequest streams

The ClientRequest type (unlike http.Request) is not reusable
across RoundTrip attempts because it includes a single-use
clientStream. (It's possible for RoundTrip to return while
some goroutines are still accessing the clientStream.)

Always clone the ClientRequest on retries.

Fixes #78202
Fixes #78187

Change-Id: I4012bb4e017a9516278c873ec5a589086a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/761301
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>

1 files changed, 7 insertions, 8 deletions

diff --git a/src/net/http/internal/http2/transport.go b/src/net/http/internal/http2/transport.go
index 1e6f4474e2..36423b22df 100644
--- a/src/net/http/internal/http2/transport.go
+++ b/src/net/http/internal/http2/transport.go
@@ -644,16 +644,16 @@ var (
 
 // shouldRetryRequest is called by RoundTrip when a request fails to get
 // response headers. It is always called with a non-nil error.
-// It returns either a request to retry (either the same request, or a
-// modified clone), or an error if the request can't be replayed.
+// It returns either a request to retry or an error if the request can't be replayed.
+// If the request is retried, it always clones the request (since requests
+// contain an unreusable clientStream).
 func shouldRetryRequest(req *ClientRequest, err error) (*ClientRequest, error) {
 	if !canRetryError(err) {
 		return nil, err
 	}
-	// If the Body is nil (or http.NoBody), it's safe to reuse
-	// this request and its Body.
+	// If the Body is nil (or http.NoBody), it's safe to reuse this request's Body.
 	if req.Body == nil || req.Body == NoBody {
-		return req, nil
+		return req.Clone(), nil
 	}
 
 	// If the request body can be reset back to its original
@@ -669,10 +669,9 @@ func shouldRetryRequest(req *ClientRequest, err error) (*ClientRequest, error) {
 	}
 
 	// The Request.Body can't reset back to the beginning, but we
-	// don't seem to have started to read from it yet, so reuse
-	// the request directly.
+	// don't seem to have started to read from it yet, so reuse the body.
 	if err == errClientConnUnusable {
-		return req, nil
+		return req.Clone(), nil
 	}
 
 	return nil, fmt.Errorf("http2: Transport: cannot retry err [%v] after Request.Body was written; define Request.GetBody to avoid this error", err)