From b9c4ad0b2b1c79b19f619bc90ba6d4aaf6dc1260 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Mon, 6 Sep 2021 21:52:00 +0200 Subject: [PATCH] fix(netxlite): http3 propagates CloseIdleConnections to its dialer (#471) With this change, we are now able to change more dependent code to simplify the way in which we create and manage resolvers. See https://github.com/ooni/probe/issues/1591 --- .../cmd/oohelperd/internal/websteps/explore.go | 3 ++- .../engine/netx/httptransport/http3transport.go | 4 +++- internal/netxlite/http3.go | 9 ++++++--- internal/netxlite/http3_test.go | 16 ++++++++++++++++ 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/internal/cmd/oohelperd/internal/websteps/explore.go b/internal/cmd/oohelperd/internal/websteps/explore.go index 5f52b76..d55cb29 100644 --- a/internal/cmd/oohelperd/internal/websteps/explore.go +++ b/internal/cmd/oohelperd/internal/websteps/explore.go @@ -135,7 +135,8 @@ func (e *DefaultExplorer) getH3(h3URL *h3URL, headers map[string][]string) (*htt tlsConf := &tls.Config{ NextProtos: []string{h3URL.proto}, } - transport := netxlite.NewHTTP3Transport(dialer, tlsConf) + transport := netxlite.NewHTTP3Transport( + netxlite.NewQUICDialerFromContextDialerAdapter(dialer), tlsConf) // TODO(bassosimone): here we should use runtimex.PanicOnError jarjar, _ := cookiejar.New(nil) clnt := &http.Client{ diff --git a/internal/engine/netx/httptransport/http3transport.go b/internal/engine/netx/httptransport/http3transport.go index 95427ac..63ce659 100644 --- a/internal/engine/netx/httptransport/http3transport.go +++ b/internal/engine/netx/httptransport/http3transport.go @@ -10,5 +10,7 @@ import ( // // New code should use netxlite.NewHTTP3Transport instead. func NewHTTP3Transport(config Config) RoundTripper { - return netxlite.NewHTTP3Transport(config.QUICDialer, config.TLSConfig) + return netxlite.NewHTTP3Transport( + netxlite.NewQUICDialerFromContextDialerAdapter(config.QUICDialer), + config.TLSConfig) } diff --git a/internal/netxlite/http3.go b/internal/netxlite/http3.go index cec7b40..d502146 100644 --- a/internal/netxlite/http3.go +++ b/internal/netxlite/http3.go @@ -13,7 +13,7 @@ import ( // an http3.RoundTripper. This is necessary because the // http3.RoundTripper does not support DialContext. type http3Dialer struct { - Dialer QUICContextDialer + Dialer QUICDialer } // dial is like QUICContextDialer.DialContext but without context. @@ -25,7 +25,8 @@ func (d *http3Dialer) dial(network, address string, tlsConfig *tls.Config, // http3Transport is an HTTPTransport using the http3 protocol. type http3Transport struct { - child *http3.RoundTripper + child *http3.RoundTripper + dialer QUICDialer } var _ HTTPTransport = &http3Transport{} @@ -38,13 +39,14 @@ func (txp *http3Transport) RoundTrip(req *http.Request) (*http.Response, error) // CloseIdleConnections implements HTTPTransport.CloseIdleConnections. func (txp *http3Transport) CloseIdleConnections() { txp.child.Close() + txp.dialer.CloseIdleConnections() } // NewHTTP3Transport creates a new HTTPTransport using http3. The // dialer argument MUST NOT be nil. If the tlsConfig argument is nil, // then the code will use the default TLS configuration. func NewHTTP3Transport( - dialer QUICContextDialer, tlsConfig *tls.Config) HTTPTransport { + dialer QUICDialer, tlsConfig *tls.Config) HTTPTransport { return &http3Transport{ child: &http3.RoundTripper{ Dial: (&http3Dialer{dialer}).dial, @@ -55,5 +57,6 @@ func NewHTTP3Transport( DisableCompression: true, TLSClientConfig: tlsConfig, }, + dialer: dialer, } } diff --git a/internal/netxlite/http3_test.go b/internal/netxlite/http3_test.go index 9a2e464..234e4f3 100644 --- a/internal/netxlite/http3_test.go +++ b/internal/netxlite/http3_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" ) func TestHTTP3TransportWorks(t *testing.T) { @@ -24,3 +25,18 @@ func TestHTTP3TransportWorks(t *testing.T) { resp.Body.Close() txp.CloseIdleConnections() } + +func TestHTTP3TransportClosesIdleConnections(t *testing.T) { + var called bool + d := &mocks.QUICDialer{ + MockCloseIdleConnections: func() { + called = true + }, + } + txp := NewHTTP3Transport(d, &tls.Config{}) + client := &http.Client{Transport: txp} + client.CloseIdleConnections() + if !called { + t.Fatal("not called") + } +}