From 741a8bc4c2bed890560667643be415338612847f Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Mon, 27 Sep 2021 12:00:43 +0200 Subject: [PATCH] feat(netxlite): introduce wrapping constructors (#507) This diff has been extracted from https://github.com/ooni/probe-cli/pull/506. In it, we introduce wrapping constructors for types and we update the docs. These new constructures are used by the code in https://github.com/ooni/probe-cli/pull/506. In itself, this work is part of https://github.com/ooni/probe/issues/1733. --- internal/netxlite/dialer.go | 14 +++++++++---- internal/netxlite/http.go | 39 +++++++++++++++++++++-------------- internal/netxlite/resolver.go | 17 +++++++++------ 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go index df61e6d..d143b92 100644 --- a/internal/netxlite/dialer.go +++ b/internal/netxlite/dialer.go @@ -19,8 +19,14 @@ type Dialer interface { CloseIdleConnections() } -// NewDialerWithResolver creates a new Dialer. The returned Dialer -// has the following properties: +// NewDialerWithResolver is a convenience factory that calls +// WrapDialer for a stdlib dialer type. +func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer { + return WrapDialer(logger, resolver, &dialerSystem{}) +} + +// WrapDialer creates a new Dialer that wraps the given +// Dialer. The returned Dialer has the following properties: // // 1. logs events using the given logger; // @@ -45,12 +51,12 @@ type Dialer interface { // 6. if a dialer wraps a resolver, the dialer will forward // the CloseIdleConnection call to its resolver (which is // instrumental to manage a DoH resolver connections properly). -func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer { +func WrapDialer(logger Logger, resolver Resolver, dialer Dialer) Dialer { return &dialerLogger{ Dialer: &dialerResolver{ Dialer: &dialerLogger{ Dialer: &dialerErrWrapper{ - Dialer: &dialerSystem{}, + Dialer: dialer, }, Logger: logger, operationSuffix: "_address", diff --git a/internal/netxlite/http.go b/internal/netxlite/http.go index 7ac528e..35aaf59 100644 --- a/internal/netxlite/http.go +++ b/internal/netxlite/http.go @@ -82,11 +82,15 @@ func (txp *httpTransportConnectionsCloser) CloseIdleConnections() { txp.TLSDialer.CloseIdleConnections() } -// NewHTTPTransport creates a new HTTP transport using the given +// NewHTTPTransport combines NewOOHTTPBaseTransport and +// WrapHTTPTransport to construct a new HTTPTransport. +func NewHTTPTransport(logger Logger, dialer Dialer, tlsDialer TLSDialer) HTTPTransport { + return WrapHTTPTransport(logger, NewOOHTTPBaseTransport(dialer, tlsDialer)) +} + +// NewOOHTTPBaseTransport creates a new HTTP transport using the given // dialer and TLS dialer to create connections. // -// The returned transport will use the given Logger for logging. -// // The returned transport will gracefully handle TLS connections // created using gitlab.com/yawning/utls.git. // @@ -106,10 +110,7 @@ func (txp *httpTransportConnectionsCloser) CloseIdleConnections() { // necessary to perform sane measurements with tracing. We will be // able to possibly relax this requirement after we change the // way in which we perform measurements. -// -// The returned transport will set a default user agent if the -// request has not already set a user agent. -func NewHTTPTransport(logger Logger, dialer Dialer, tlsDialer TLSDialer) HTTPTransport { +func NewOOHTTPBaseTransport(dialer Dialer, tlsDialer TLSDialer) HTTPTransport { // Using oohttp to support any TLS library. txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone() @@ -138,16 +139,24 @@ func NewHTTPTransport(logger Logger, dialer Dialer, tlsDialer TLSDialer) HTTPTra // upon us when we are using TLS parroting). txp.ForceAttemptHTTP2 = true - // Ensure we correctly forward CloseIdleConnections and compose - // with a logging transport thus enabling logging. + // Ensure we correctly forward CloseIdleConnections. + return &httpTransportConnectionsCloser{ + HTTPTransport: &oohttp.StdlibTransport{Transport: txp}, + Dialer: dialer, + TLSDialer: tlsDialer, + } +} + +// WrapHTTPTransport creates a new HTTP transport using +// the given logger for logging. +// +// The returned transport will set a default user agent if the +// request has not already set a user agent. +func WrapHTTPTransport(logger Logger, txp HTTPTransport) HTTPTransport { return &httpUserAgentTransport{ HTTPTransport: &httpTransportLogger{ - HTTPTransport: &httpTransportConnectionsCloser{ - HTTPTransport: &oohttp.StdlibTransport{Transport: txp}, - Dialer: dialer, - TLSDialer: tlsDialer, - }, - Logger: logger, + HTTPTransport: txp, + Logger: logger, }, } } diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go index a85e966..e43a176 100644 --- a/internal/netxlite/resolver.go +++ b/internal/netxlite/resolver.go @@ -25,10 +25,15 @@ type Resolver interface { CloseIdleConnections() } -// NewResolverStdlib creates a new resolver using system -// facilities for resolving domain names (e.g., getaddrinfo). -// -// The resolver will provide the following guarantees: +// NewResolverStdlib creates a new Resolver by combining +// WrapResolver with an internal "system" resolver type that +// adds extra functionality to net.Resolver. +func NewResolverStdlib(logger Logger) Resolver { + return WrapResolver(logger, &resolverSystem{}) +} + +// WrapResolver creates a new resolver that wraps an +// existing resolver to add these properties: // // 1. handles IDNA; // @@ -41,12 +46,12 @@ type Resolver interface { // // 5. enforces reasonable timeouts ( // see https://github.com/ooni/probe/issues/1726). -func NewResolverStdlib(logger Logger) Resolver { +func WrapResolver(logger Logger, resolver Resolver) Resolver { return &resolverIDNA{ Resolver: &resolverLogger{ Resolver: &resolverShortCircuitIPAddr{ Resolver: &resolverErrWrapper{ - Resolver: &resolverSystem{}, + Resolver: resolver, }, }, Logger: logger,