Skip to content

Commit

Permalink
refactor(MeasuringNetwork): define dialers without resolver (ooni#1259)
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
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent 3a90f76 commit 31f2edc
Showing 5 changed files with 62 additions and 40 deletions.
16 changes: 8 additions & 8 deletions internal/mocks/measuringnetwork.go
Original file line number Diff line number Diff line change
@@ -7,13 +7,13 @@ import (

// MeasuringNetwork allows mocking [model.MeasuringNetwork].
type MeasuringNetwork struct {
MockNewDialerWithResolver func(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer
MockNewDialerWithoutResolver func(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer

MockNewParallelDNSOverHTTPSResolver func(logger model.DebugLogger, URL string) model.Resolver

MockNewParallelUDPResolver func(logger model.DebugLogger, dialer model.Dialer, address string) model.Resolver

MockNewQUICDialerWithResolver func(listener model.UDPListener, logger model.DebugLogger, resolver model.Resolver, w ...model.QUICDialerWrapper) model.QUICDialer
MockNewQUICDialerWithoutResolver func(listener model.UDPListener, logger model.DebugLogger, w ...model.QUICDialerWrapper) model.QUICDialer

MockNewStdlibResolver func(logger model.DebugLogger) model.Resolver

@@ -26,9 +26,9 @@ type MeasuringNetwork struct {

var _ model.MeasuringNetwork = &MeasuringNetwork{}

// NewDialerWithResolver implements model.MeasuringNetwork.
func (mn *MeasuringNetwork) NewDialerWithResolver(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer {
return mn.MockNewDialerWithResolver(dl, r, w...)
// NewDialerWithoutResolver implements model.MeasuringNetwork.
func (mn *MeasuringNetwork) NewDialerWithoutResolver(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer {
return mn.MockNewDialerWithoutResolver(dl, w...)
}

// NewParallelDNSOverHTTPSResolver implements model.MeasuringNetwork.
@@ -41,9 +41,9 @@ func (mn *MeasuringNetwork) NewParallelUDPResolver(logger model.DebugLogger, dia
return mn.MockNewParallelUDPResolver(logger, dialer, address)
}

// NewQUICDialerWithResolver implements model.MeasuringNetwork.
func (mn *MeasuringNetwork) NewQUICDialerWithResolver(listener model.UDPListener, logger model.DebugLogger, resolver model.Resolver, w ...model.QUICDialerWrapper) model.QUICDialer {
return mn.MockNewQUICDialerWithResolver(listener, logger, resolver, w...)
// NewQUICDialerWithoutResolver implements model.MeasuringNetwork.
func (mn *MeasuringNetwork) NewQUICDialerWithoutResolver(listener model.UDPListener, logger model.DebugLogger, w ...model.QUICDialerWrapper) model.QUICDialer {
return mn.MockNewQUICDialerWithoutResolver(listener, logger, w...)
}

// NewStdlibResolver implements model.MeasuringNetwork.
12 changes: 6 additions & 6 deletions internal/mocks/measuringnetwork_test.go
Original file line number Diff line number Diff line change
@@ -8,14 +8,14 @@ import (
)

func TestMeasuringN(t *testing.T) {
t.Run("MockNewDialerWithResolver", func(t *testing.T) {
t.Run("MockNewDialerWithoutResolver", func(t *testing.T) {
expected := &Dialer{}
mn := &MeasuringNetwork{
MockNewDialerWithResolver: func(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer {
MockNewDialerWithoutResolver: func(dl model.DebugLogger, w ...model.DialerWrapper) model.Dialer {
return expected
},
}
got := mn.NewDialerWithResolver(nil, nil)
got := mn.NewDialerWithoutResolver(nil, nil)
if expected != got {
t.Fatal("unexpected result")
}
@@ -47,14 +47,14 @@ func TestMeasuringN(t *testing.T) {
}
})

t.Run("MockNewQUICDialerWithResolver", func(t *testing.T) {
t.Run("MockNewQUICDialerWithoutResolver", func(t *testing.T) {
expected := &QUICDialer{}
mn := &MeasuringNetwork{
MockNewQUICDialerWithResolver: func(listener model.UDPListener, logger model.DebugLogger, resolver model.Resolver, w ...model.QUICDialerWrapper) model.QUICDialer {
MockNewQUICDialerWithoutResolver: func(listener model.UDPListener, logger model.DebugLogger, w ...model.QUICDialerWrapper) model.QUICDialer {
return expected
},
}
got := mn.NewQUICDialerWithResolver(nil, nil, nil)
got := mn.NewQUICDialerWithoutResolver(nil, nil, nil)
if expected != got {
t.Fatal("unexpected result")
}
28 changes: 10 additions & 18 deletions internal/model/netx.go
Original file line number Diff line number Diff line change
@@ -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 wraps 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
// The [QUICDialerWrapper] arguments wraps 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`.
23 changes: 19 additions & 4 deletions internal/netxlite/dialer.go
Original file line number Diff line number Diff line change
@@ -23,7 +23,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...)
}
@@ -143,10 +152,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
23 changes: 19 additions & 4 deletions internal/netxlite/quic.go
Original file line number Diff line number Diff line change
@@ -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 wraps 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.

0 comments on commit 31f2edc

Please sign in to comment.