net/http: fix "http2: no cached connection..." error with x/net/http2

The net/http Transport was testing for a sentinel x/net/http2 error
value with ==, which meant it was only testing the bundled version. If
a user enabled http2 via golang.org/x/net/http2, the error value had a
different name.

This also updates the bundled x/net/http2 to git rev ab555f36 for:

    http2: add internal function isNoCachedConnError to test for ErrNoCachedConn
    https://golang.org/cl/87297

Fixes #22091

Change-Id: I3fb85e2b7ba7d145dd66767e1795a56de633958c
Reviewed-on: https://go-review.googlesource.com/87298
Reviewed-by: Tom Bergan <tombergan@google.com>

3 files changed, 40 insertions, 8 deletions

diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go
index 161a1ed137..7a1564f755 100644
--- a/src/net/http/h2_bundle.go
+++ b/src/net/http/h2_bundle.go
@@ -989,7 +989,7 @@ type http2noDialH2RoundTripper struct{ t *http2Transport }
 
 func (rt http2noDialH2RoundTripper) RoundTrip(req *Request) (*Response, error) {
 	res, err := rt.t.RoundTrip(req)
-	if err == http2ErrNoCachedConn {
+	if http2isNoCachedConnError(err) {
 		return nil, ErrSkipAltProtocol
 	}
 	return res, err
@@ -6856,7 +6856,27 @@ func (sew http2stickyErrWriter) Write(p []byte) (n int, err error) {
 	return
 }
 
-var http2ErrNoCachedConn = errors.New("http2: no cached connection was available")
+// noCachedConnError is the concrete type of ErrNoCachedConn, needs to be detected
+// by net/http regardless of whether it's its bundled version (in h2_bundle.go with a rewritten type name)
+// or from a user's x/net/http2. As such, as it has a unique method name (IsHTTP2NoCachedConnError) that
+// net/http sniffs for via func isNoCachedConnError.
+type http2noCachedConnError struct{}
+
+func (http2noCachedConnError) IsHTTP2NoCachedConnError() {}
+
+func (http2noCachedConnError) Error() string { return "http2: no cached connection was available" }
+
+// isNoCachedConnError reports whether err is of type noCachedConnError
+// or its equivalent renamed type in net/http2's h2_bundle.go. Both types
+// may coexist in the same running program.
+func http2isNoCachedConnError(err error) bool {
+	_, ok := err.(interface {
+		IsHTTP2NoCachedConnError()
+	})
+	return ok
+}
+
+var http2ErrNoCachedConn error = http2noCachedConnError{}
 
 // RoundTripOpt are options for the Transport.RoundTripOpt method.
 type http2RoundTripOpt struct {
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index c9758e9b38..7ef8f0147b 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -452,7 +452,7 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 // HTTP request on a new connection. The non-nil input error is the
 // error from roundTrip.
 func (pc *persistConn) shouldRetryRequest(req *Request, err error) bool {
-	if err == http2ErrNoCachedConn {
+	if http2isNoCachedConnError(err) {
 		// Issue 16582: if the user started a bunch of
 		// requests at once, they can all pick the same conn
 		// and violate the server's max concurrent streams.
diff --git a/src/net/http/transport_internal_test.go b/src/net/http/transport_internal_test.go
index 594bf6e2c8..a5f29c97a9 100644
--- a/src/net/http/transport_internal_test.go
+++ b/src/net/http/transport_internal_test.go
@@ -96,6 +96,12 @@ func dummyRequestWithBodyNoGetBody(method string) *Request {
 	return req
 }
 
+// issue22091Error acts like a golang.org/x/net/http2.ErrNoCachedConn.
+type issue22091Error struct{}
+
+func (issue22091Error) IsHTTP2NoCachedConnError() {}
+func (issue22091Error) Error() string             { return "issue22091Error" }
+
 func TestTransportShouldRetryRequest(t *testing.T) {
 	tests := []struct {
 		pc  *persistConn
@@ -123,36 +129,42 @@ func TestTransportShouldRetryRequest(t *testing.T) {
 			want: true,
 		},
 		3: {
+			pc:   nil,
+			req:  nil,
+			err:  issue22091Error{}, // like an external http2ErrNoCachedConn
+			want: true,
+		},
+		4: {
 			pc:   &persistConn{reused: true},
 			req:  dummyRequest("POST"),
 			err:  errMissingHost,
 			want: false,
 		},
-		4: {
+		5: {
 			pc:   &persistConn{reused: true},
 			req:  dummyRequest("POST"),
 			err:  transportReadFromServerError{},
 			want: false,
 		},
-		5: {
+		6: {
 			pc:   &persistConn{reused: true},
 			req:  dummyRequest("GET"),
 			err:  transportReadFromServerError{},
 			want: true,
 		},
-		6: {
+		7: {
 			pc:   &persistConn{reused: true},
 			req:  dummyRequest("GET"),
 			err:  errServerClosedIdle,
 			want: true,
 		},
-		7: {
+		8: {
 			pc:   &persistConn{reused: true},
 			req:  dummyRequestWithBody("POST"),
 			err:  nothingWrittenError{},
 			want: true,
 		},
-		8: {
+		9: {
 			pc:   &persistConn{reused: true},
 			req:  dummyRequestWithBodyNoGetBody("POST"),
 			err:  nothingWrittenError{},