Commit bb4356db authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

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/87298Reviewed-by: 's avatarTom Bergan <tombergan@google.com>
parent 1b89dada
......@@ -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 {
......
......@@ -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.
......
......@@ -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{},
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment