Skip to content

Commit

Permalink
refactor(MeasuringNetwork): define dialers without resolver
Browse files Browse the repository at this point in the history
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 ooni/probe#2531
  • Loading branch information
bassosimone committed Sep 12, 2023
1 parent 8bf9d88 commit 8066813
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 25 deletions.
26 changes: 9 additions & 17 deletions internal/model/netx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`.
Expand Down
23 changes: 19 additions & 4 deletions internal/netxlite/dialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
Expand Down Expand Up @@ -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
Expand Down
23 changes: 19 additions & 4 deletions internal/netxlite/quic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 8066813

Please sign in to comment.