From 806681366d40e213fcde130f63261e854b18ab53 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Sep 2023 13:58:21 +0200 Subject: [PATCH] refactor(MeasuringNetwork): define dialers without resolver I initially planned on defining only the dialers with resolver and explicitly passing &NullResolver{} to them. But I have just realized that, instead, defining the dialers without resolvers (which is what measurexlite needs anyway) will allow me to rewrite the without-resolver case to be very simple and completely ignore resolvers. Once I have done this rewrite, we can keep the existing implementation for legacy code, but we can also have: 1. the above-mentioned simple dialing implementation without resolvers that fullfills measurexlite needs; 2. another implementation tailored to the needs of beacons. So, voila, I think I now have a reasonable way forward to introduce beacons support in netxlite. Part of https://github.com/ooni/probe/issues/2531 --- internal/model/netx.go | 26 +++++++++----------------- internal/netxlite/dialer.go | 23 +++++++++++++++++++---- internal/netxlite/quic.go | 23 +++++++++++++++++++---- 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/internal/model/netx.go b/internal/model/netx.go index 264559a25f..a5785c88f9 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -183,17 +183,12 @@ type HTTPSSvc struct { // implementation of this interface. This interface SHOULD always be implemented in terms of // an [UnderlyingNetwork] that allows to switch between the host network and [netemx]. type MeasuringNetwork interface { - // NewDialerWithResolver creates a [Dialer] with error wrapping. + // NewDialerWithoutResolver creates a [Dialer] with error wrapping and without an attached + // resolver, meaning that you MUST pass TCP or UDP endpoint addresses to this dialer. // - // This dialer will try to connect to each of the resolved IP address - // sequentially. In case of failure, such a resolver will return the first - // error that occurred. This implementation strategy is a QUIRK that is - // documented at TODO(https://github.com/ooni/probe/issues/1779). - // - // The [DialerWrapper] arguments wrap the returned dialer in such a way - // that we can implement the legacy [netx] package. New code MUST NOT - // use this functionality, which we'd like to remove ASAP. - NewDialerWithResolver(dl DebugLogger, r Resolver, w ...DialerWrapper) Dialer + // The [DialerWrapper] arguments wrap the returned dialer in such a way that we can implement + // the legacy [netx] package. New code MUST NOT use this functionality, which we'd like to remove ASAP. + NewDialerWithoutResolver(dl DebugLogger, w ...DialerWrapper) Dialer // NewParallelDNSOverHTTPSResolver creates a new DNS-over-HTTPS resolver with error wrapping. NewParallelDNSOverHTTPSResolver(logger DebugLogger, URL string) Resolver @@ -204,17 +199,14 @@ type MeasuringNetwork interface { // The address argument is the UDP endpoint address (e.g., 1.1.1.1:53, [::1]:53). NewParallelUDPResolver(logger DebugLogger, dialer Dialer, address string) Resolver - // NewQUICDialerWithResolver creates a QUICDialer with error wrapping. - // - // Unlike the dialer returned by NewDialerWithResolver, this dialer MAY attempt - // happy eyeballs, perform parallel dial attempts, and return an error - // that aggregates all the errors that occurred. + // NewQUICDialerWithoutResolver creates a [QUICDialer] with error wrapping and without an attached + // resolver, meaning that you MUST pass UDP endpoint addresses to this dialer. // // The [QUICDialerWrapper] arguments wrap the returned dialer in such a way // that we can implement the legacy [netx] package. New code MUST NOT // use this functionality, which we'd like to remove ASAP. - NewQUICDialerWithResolver( - listener UDPListener, logger DebugLogger, resolver Resolver, w ...QUICDialerWrapper) QUICDialer + NewQUICDialerWithoutResolver( + listener UDPListener, logger DebugLogger, w ...QUICDialerWrapper) QUICDialer // NewStdlibResolver creates a new Resolver with error wrapping using // getaddrinfo or &net.Resolver{} depending on `-tags netgo`. diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go index e30e344514..6a0ff6133b 100644 --- a/internal/netxlite/dialer.go +++ b/internal/netxlite/dialer.go @@ -22,7 +22,16 @@ func NewDialerWithStdlibResolver(dl model.DebugLogger) model.Dialer { return NewDialerWithResolver(dl, reso) } -// NewDialerWithResolver implements [model.MeasuringNetwork]. +// NewDialerWithResolver creates a [Dialer] with error wrapping. +// +// This dialer will try to connect to each of the resolved IP address +// sequentially. In case of failure, such a resolver will return the first +// error that occurred. This implementation strategy is a QUIRK that is +// documented at TODO(https://github.com/ooni/probe/issues/1779). +// +// The [model.DialerWrapper] arguments wrap the returned dialer in such a way +// that we can implement the legacy [netx] package. New code MUST NOT +// use this functionality, which we'd like to remove ASAP. func (netx *Netx) NewDialerWithResolver(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer { return WrapDialer(dl, r, &dialerSystem{provider: netx.maybeCustomUnderlyingNetwork()}, w...) } @@ -142,10 +151,16 @@ func WrapDialer(logger model.DebugLogger, resolver model.Resolver, } } -// NewDialerWithoutResolver is equivalent to calling NewDialerWithResolver -// with the resolver argument being &NullResolver{}. +// NewDialerWithoutResolver implements [model.MeasuringNetwork]. +func (netx *Netx) NewDialerWithoutResolver(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer { + return netx.NewDialerWithResolver(dl, &NullResolver{}, w...) +} + +// NewDialerWithoutResolver is equivalent to creating an empty [*Netx] +// and calling its NewDialerWithoutResolver method. func NewDialerWithoutResolver(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer { - return NewDialerWithResolver(dl, &NullResolver{}, w...) + netx := &Netx{Underlying: nil} + return netx.NewDialerWithoutResolver(dl, w...) } // dialerSystem is a model.Dialer that uses the stdlib's net.Dialer diff --git a/internal/netxlite/quic.go b/internal/netxlite/quic.go index df5ac80e9c..ce8af96e67 100644 --- a/internal/netxlite/quic.go +++ b/internal/netxlite/quic.go @@ -16,7 +16,15 @@ import ( "github.com/quic-go/quic-go" ) -// NewQUICDialerWithResolver implements [model.MeasuringNetwork]. +// NewQUICDialerWithResolver creates a QUICDialer with error wrapping. +// +// Unlike the dialer returned by NewDialerWithResolver, this dialer MAY attempt +// happy eyeballs, perform parallel dial attempts, and return an error +// that aggregates all the errors that occurred. +// +// The [model.QUICDialerWrapper] arguments wrap the returned dialer in such a way +// that we can implement the legacy [netx] package. New code MUST NOT +// use this functionality, which we'd like to remove ASAP. func (netx *Netx) NewQUICDialerWithResolver(listener model.UDPListener, logger model.DebugLogger, resolver model.Resolver, wrappers ...model.QUICDialerWrapper) (outDialer model.QUICDialer) { baseDialer := &quicDialerQUICGo{ @@ -62,11 +70,18 @@ func wrapQUICDialer(logger model.DebugLogger, resolver model.Resolver, } } -// NewQUICDialerWithoutResolver is equivalent to calling NewQUICDialerWithResolver -// with the resolver argument set to &NullResolver{}. +// NewQUICDialerWithoutResolver implements [model.MeasuringNetwork]. +func (netx *Netx) NewQUICDialerWithoutResolver(listener model.UDPListener, + logger model.DebugLogger, wrappers ...model.QUICDialerWrapper) model.QUICDialer { + return netx.NewQUICDialerWithResolver(listener, logger, &NullResolver{}, wrappers...) +} + +// NewQUICDialerWithoutResolver is equivalent to creating an empty [*Netx] +// and calling its NewQUICDialerWithoutResolver method. func NewQUICDialerWithoutResolver(listener model.UDPListener, logger model.DebugLogger, wrappers ...model.QUICDialerWrapper) model.QUICDialer { - return NewQUICDialerWithResolver(listener, logger, &NullResolver{}, wrappers...) + netx := &Netx{Underlying: nil} + return netx.NewQUICDialerWithoutResolver(listener, logger, wrappers...) } // quicDialerQUICGo dials using the quic-go/quic-go library.