diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go index d265cd3f726..e5c06a89032 100644 --- a/src/net/http/export_test.go +++ b/src/net/http/export_test.go @@ -208,6 +208,30 @@ func (t *Transport) PutIdleTestConn(scheme, addr string) bool { }) == nil } +// PutIdleTestConnH2 reports whether it was able to insert a fresh +// HTTP/2 persistConn for scheme, addr into the idle connection pool. +func (t *Transport) PutIdleTestConnH2(scheme, addr string, alt RoundTripper) bool { + key := connectMethodKey{"", scheme, addr, false} + + if t.MaxConnsPerHost > 0 { + // Transport is tracking conns-per-host. + // Increment connection count to account + // for new persistConn created below. + t.connsPerHostMu.Lock() + if t.connsPerHost == nil { + t.connsPerHost = make(map[connectMethodKey]int) + } + t.connsPerHost[key]++ + t.connsPerHostMu.Unlock() + } + + return t.tryPutIdleConn(&persistConn{ + t: t, + alt: alt, + cacheKey: key, + }) == nil +} + // All test hooks must be non-nil so they can be called directly, // but the tests use nil to mean hook disabled. func unnilTestHook(f *func()) { diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 44dbbef43fe..209414d85b1 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1195,7 +1195,9 @@ func (t *Transport) getConn(treq *transportRequest, cm connectMethod) (pc *persi // Queue for idle connection. if delivered := t.queueForIdleConn(w); delivered { pc := w.pc - if trace != nil && trace.GotConn != nil { + // Trace only for HTTP/1. + // HTTP/2 calls trace.GotConn itself. + if pc.alt == nil && trace != nil && trace.GotConn != nil { trace.GotConn(pc.gotIdleConnTrace(pc.idleAt)) } // set request canceler to some non-nil function so we diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 2ea26f7f666..b456e5823f3 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -3562,6 +3562,38 @@ func TestTransportCloseIdleConnsThenReturn(t *testing.T) { wantIdle("after final put", 1) } +// Test for issue 34282 +// Ensure that getConn doesn't call the GotConn trace hook on a HTTP/2 idle conn +func TestTransportTraceGotConnH2IdleConns(t *testing.T) { + tr := &Transport{} + wantIdle := func(when string, n int) bool { + got := tr.IdleConnCountForTesting("https", "example.com:443") // key used by PutIdleTestConnH2 + if got == n { + return true + } + t.Errorf("%s: idle conns = %d; want %d", when, got, n) + return false + } + wantIdle("start", 0) + alt := funcRoundTripper(func() {}) + if !tr.PutIdleTestConnH2("https", "example.com:443", alt) { + t.Fatal("put failed") + } + wantIdle("after put", 1) + ctx := httptrace.WithClientTrace(context.Background(), &httptrace.ClientTrace{ + GotConn: func(httptrace.GotConnInfo) { + // tr.getConn should leave it for the HTTP/2 alt to call GotConn. + t.Error("GotConn called") + }, + }) + req, _ := NewRequestWithContext(ctx, MethodGet, "https://example.com", nil) + _, err := tr.RoundTrip(req) + if err != errFakeRoundTrip { + t.Errorf("got error: %v; want %q", err, errFakeRoundTrip) + } + wantIdle("after round trip", 1) +} + // This tests that an client requesting a content range won't also // implicitly ask for gzip support. If they want that, they need to do it // on their own.