diff --git a/internal/engine/experiment/quicping/quicping.go b/internal/engine/experiment/quicping/quicping.go index f5c5c31..972f68e 100644 --- a/internal/engine/experiment/quicping/quicping.go +++ b/internal/engine/experiment/quicping/quicping.go @@ -18,7 +18,6 @@ import ( _ "crypto/sha256" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/tracex" ) @@ -44,8 +43,8 @@ type Config struct { // Port is the port to test. Port int64 `ooni:"port is the port to test"` - // networkLibrary is the underlying network library. Can be used for testing. - networkLib model.UnderlyingNetworkLibrary + // netListenUDP allows mocking the real net.ListenUDP call + netListenUDP func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) } func (c *Config) repetitions() int64 { @@ -62,11 +61,11 @@ func (c *Config) port() string { return "443" } -func (c *Config) networkLibrary() model.UnderlyingNetworkLibrary { - if c.networkLib != nil { - return c.networkLib +func (c *Config) doListenUDP(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) { + if c.netListenUDP != nil { + return c.netListenUDP(network, laddr) } - return &netxlite.TProxyStdlib{} + return net.ListenUDP(network, laddr) } // TestKeys contains the experiment results. @@ -246,7 +245,7 @@ func (m *Measurer) Run( measurement.TestKeys = tk // create UDP socket - pconn, err := m.config.networkLibrary().ListenUDP("udp", &net.UDPAddr{}) + pconn, err := m.config.doListenUDP("udp", &net.UDPAddr{}) if err != nil { return err } diff --git a/internal/engine/experiment/quicping/quicping_test.go b/internal/engine/experiment/quicping/quicping_test.go index 725d1ea..79a214f 100644 --- a/internal/engine/experiment/quicping/quicping_test.go +++ b/internal/engine/experiment/quicping/quicping_test.go @@ -15,76 +15,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/model/mocks" ) -// FailStdLib is a failing model.UnderlyingNetworkLibrary. -type FailStdLib struct { - conn model.UDPLikeConn - err error - writeErr error - readErr error -} - -// ListenUDP implements model.UnderlyingNetworkLibrary.ListenUDP. -func (f *FailStdLib) ListenUDP(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) { - conn, _ := net.ListenUDP(network, laddr) - f.conn = model.UDPLikeConn(conn) - if f.err != nil { - return nil, f.err - } - if f.writeErr != nil { - return &mocks.UDPLikeConn{ - MockWriteTo: func(p []byte, addr net.Addr) (int, error) { - return 0, f.writeErr - }, - MockReadFrom: func(p []byte) (int, net.Addr, error) { - return f.conn.ReadFrom(p) - }, - MockSetDeadline: func(t time.Time) error { - return f.conn.SetDeadline(t) - }, - MockClose: func() error { - return f.conn.Close() - }, - }, nil - } - if f.readErr != nil { - return &mocks.UDPLikeConn{ - MockWriteTo: func(p []byte, addr net.Addr) (int, error) { - return f.conn.WriteTo(p, addr) - }, - MockReadFrom: func(p []byte) (int, net.Addr, error) { - return 0, nil, f.readErr - }, - MockSetDeadline: func(t time.Time) error { - return f.conn.SetDeadline(t) - }, - MockClose: func() error { - return f.conn.Close() - }, - }, nil - } - return &mocks.UDPLikeConn{}, nil -} - -// DefaultResolver implements model.UnderlyingNetworkLibrary.DefaultResolver. -func (f *FailStdLib) DefaultResolver() model.SimpleResolver { - return f -} - -// LookupHost implements model.SimpleResolver.LookupHost. -func (f *FailStdLib) LookupHost(ctx context.Context, domain string) ([]string, error) { - return nil, f.err -} - -// Network implements model.SimpleResolver.Network. -func (f *FailStdLib) Network() string { - return "fail_stdlib" -} - -// NewSimpleDialer implements UnderlyingNetworkLibrary.NewSimpleDialer. -func (f *FailStdLib) NewSimpleDialer(timeout time.Duration) model.SimpleDialer { - return nil -} - func TestNewExperimentMeasurer(t *testing.T) { measurer := NewExperimentMeasurer(Config{}) if measurer.ExperimentName() != "quicping" { @@ -201,7 +131,9 @@ func TestWithCancelledContext(t *testing.T) { func TestListenFails(t *testing.T) { expected := errors.New("expected") measurer := NewExperimentMeasurer(Config{ - networkLib: &FailStdLib{err: expected, readErr: nil, writeErr: nil}, + netListenUDP: func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) { + return nil, expected + }, }) measurement := new(model.Measurement) measurement.Input = model.MeasurementTarget("google.com") @@ -221,8 +153,30 @@ func TestWriteFails(t *testing.T) { t.Skip("skip test in short mode") } expected := errors.New("expected") + setDeadlineCalled := false + closeCalled := false + pconn := &mocks.UDPLikeConn{ + MockReadFrom: func(p []byte) (int, net.Addr, error) { + source := make([]byte, len(p)) + copy(p, source) + return len(p), &mocks.Addr{}, nil + }, + MockSetDeadline: func(t time.Time) error { + setDeadlineCalled = true + return nil + }, + MockClose: func() error { + closeCalled = true + return nil + }, + MockWriteTo: func(p []byte, addr net.Addr) (int, error) { + return 0, expected + }, + } measurer := NewExperimentMeasurer(Config{ - networkLib: &FailStdLib{err: nil, readErr: nil, writeErr: expected}, + netListenUDP: func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) { + return pconn, nil + }, Repetitions: 1, }) measurement := new(model.Measurement) @@ -245,15 +199,41 @@ func TestWriteFails(t *testing.T) { t.Fatal("ping: unexpected error type", i, *ping.Failure) } } + if !setDeadlineCalled { + t.Fatal("did not call set deadline") + } + if !closeCalled { + t.Fatal("did not call close") + } } func TestReadFails(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } + setDeadlineCalled := false + closeCalled := false expected := errors.New("expected") + pconn := &mocks.UDPLikeConn{ + MockReadFrom: func(p []byte) (int, net.Addr, error) { + return 0, nil, expected + }, + MockSetDeadline: func(t time.Time) error { + setDeadlineCalled = true + return nil + }, + MockClose: func() error { + closeCalled = true + return nil + }, + MockWriteTo: func(p []byte, addr net.Addr) (int, error) { + return len(p), nil + }, + } measurer := NewExperimentMeasurer(Config{ - networkLib: &FailStdLib{err: nil, readErr: expected, writeErr: nil}, + netListenUDP: func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) { + return pconn, nil + }, Repetitions: 1, }) measurement := new(model.Measurement) @@ -273,6 +253,12 @@ func TestReadFails(t *testing.T) { t.Fatal("expected an error here, ping", i) } } + if !setDeadlineCalled { + t.Fatal("did not call set deadline") + } + if !closeCalled { + t.Fatal("did not call close") + } } func TestNoResponse(t *testing.T) { @@ -303,7 +289,7 @@ func TestNoResponse(t *testing.T) { } func TestDissect(t *testing.T) { - // destID--srcID: 040b9649d3fd4c038ab6c073966f3921--44d064031288e97646451f + // destID--srcID: 040b9649d3fd4c038ab6c073966f3921--44d064031288e97646451f versionNegotiationResponse, _ := hex.DecodeString("eb0000000010040b9649d3fd4c038ab6c073966f39210b44d064031288e97646451f00000001ff00001dff00001cff00001b") measurer := NewExperimentMeasurer(Config{}) destID := "040b9649d3fd4c038ab6c073966f3921" diff --git a/internal/model/netx.go b/internal/model/netx.go index ecc9f5e..b38f89c 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -211,9 +211,8 @@ type QUICDialer interface { CloseIdleConnections() } -// SimpleResolver is a simplified resolver that only allows to perform -// an ordinary lookup operation and to know the resolver's name. -type SimpleResolver interface { +// Resolver performs domain name resolutions. +type Resolver interface { // LookupHost behaves like net.Resolver.LookupHost. LookupHost(ctx context.Context, hostname string) (addrs []string, err error) @@ -239,12 +238,6 @@ type SimpleResolver interface { // for an explanation of why it would not be proper to call "netgo" the // resolver we get by default from the standard library. Network() string -} - -// Resolver performs domain name resolutions. -type Resolver interface { - // A Resolver is also a SimpleResolver. - SimpleResolver // Address returns the resolver address (e.g., 8.8.8.8:53). Address() string @@ -484,18 +477,3 @@ type UDPLikeConn interface { // which is also instrumental to setting the read buffer. SyscallConn() (syscall.RawConn, error) } - -// UnderlyingNetworkLibrary defines the basic functionality from -// which the network extensions depend. By changing the default -// implementation of this interface, we can implement a wide array -// of tests, including self censorship tests. -type UnderlyingNetworkLibrary interface { - // ListenUDP creates a new model.UDPLikeConn conn. - ListenUDP(network string, laddr *net.UDPAddr) (UDPLikeConn, error) - - // DefaultResolver returns the default resolver. - DefaultResolver() SimpleResolver - - // NewSimpleDialer returns a new SimpleDialer. - NewSimpleDialer(timeout time.Duration) SimpleDialer -} diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go index b9d0dcd..3ec0cf9 100644 --- a/internal/netxlite/dialer.go +++ b/internal/netxlite/dialer.go @@ -143,7 +143,7 @@ func NewDialerWithoutResolver(dl model.DebugLogger, w ...model.DialerWrapper) mo return NewDialerWithResolver(dl, &NullResolver{}, w...) } -// DialerSystem is a model.Dialer that uses TProxy.NewSimplerDialer +// DialerSystem is a model.Dialer that uses the stdlib's net.Dialer // to construct the new SimpleDialer used for dialing. This dialer has // a fixed timeout for each connect operation equal to 15 seconds. type DialerSystem struct { @@ -160,7 +160,7 @@ func (d *DialerSystem) newUnderlyingDialer() model.SimpleDialer { if t <= 0 { t = dialerDefaultTimeout } - return TProxy.NewSimpleDialer(t) + return &net.Dialer{Timeout: t} } func (d *DialerSystem) DialContext(ctx context.Context, network, address string) (net.Conn, error) { diff --git a/internal/netxlite/dnsovergetaddrinfo.go b/internal/netxlite/dnsovergetaddrinfo.go index 0aca6ed..58b66f2 100644 --- a/internal/netxlite/dnsovergetaddrinfo.go +++ b/internal/netxlite/dnsovergetaddrinfo.go @@ -81,7 +81,7 @@ func (txp *dnsOverGetaddrinfoTransport) lookupfn() func(ctx context.Context, dom if txp.testableLookupHost != nil { return txp.testableLookupHost } - return TProxy.DefaultResolver().LookupHost + return getaddrinfoLookupHost } func (txp *dnsOverGetaddrinfoTransport) RequiresPadding() bool { @@ -89,7 +89,7 @@ func (txp *dnsOverGetaddrinfoTransport) RequiresPadding() bool { } func (txp *dnsOverGetaddrinfoTransport) Network() string { - return TProxy.DefaultResolver().Network() + return getaddrinfoResolverNetwork() } func (txp *dnsOverGetaddrinfoTransport) Address() string { diff --git a/internal/netxlite/dnsovergetaddrinfo_test.go b/internal/netxlite/dnsovergetaddrinfo_test.go index 78ca5dc..37e5deb 100644 --- a/internal/netxlite/dnsovergetaddrinfo_test.go +++ b/internal/netxlite/dnsovergetaddrinfo_test.go @@ -21,7 +21,7 @@ func TestDNSOverGetaddrinfo(t *testing.T) { t.Run("Network", func(t *testing.T) { txp := &dnsOverGetaddrinfoTransport{} - if txp.Network() != TProxy.DefaultResolver().Network() { + if txp.Network() != getaddrinfoResolverNetwork() { t.Fatal("unexpected Network") } }) diff --git a/internal/netxlite/quic.go b/internal/netxlite/quic.go index 682c976..fef805e 100644 --- a/internal/netxlite/quic.go +++ b/internal/netxlite/quic.go @@ -29,7 +29,7 @@ var _ model.QUICListener = &quicListenerStdlib{} // Listen implements QUICListener.Listen. func (qls *quicListenerStdlib) Listen(addr *net.UDPAddr) (model.UDPLikeConn, error) { - return TProxy.ListenUDP("udp", addr) + return net.ListenUDP("udp", addr) } // NewQUICDialerWithResolver is the WrapDialer equivalent for QUIC where diff --git a/internal/netxlite/tproxy.go b/internal/netxlite/tproxy.go deleted file mode 100644 index 98af596..0000000 --- a/internal/netxlite/tproxy.go +++ /dev/null @@ -1,52 +0,0 @@ -package netxlite - -// -// Transparent proxy (for integration testing) -// - -import ( - "context" - "net" - "time" - - "github.com/ooni/probe-cli/v3/internal/model" -) - -// TProxy is the fundamental variable controlling how netxlite creates -// net.Conn and model.UDPLikeConn, as well as how it uses the stdlib -// resolver. By modifying this variable, you can effectively transparently -// proxy netxlite (and hence OONI) activities to other services. This is -// quite convenient when performing quality assurance tests. -var TProxy model.UnderlyingNetworkLibrary = &TProxyStdlib{} - -// TProxyStdlib is the default model.UnderlyingNetworkLibrary using -// the stdlib in the most obvious way for every functionality. -type TProxyStdlib struct{} - -// ListenUDP calls net.ListenUDP. -func (*TProxyStdlib) ListenUDP(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) { - return net.ListenUDP(network, laddr) -} - -// DefaultResolver returns the default resolver. -func (*TProxyStdlib) DefaultResolver() model.SimpleResolver { - return &tproxyDefaultResolver{} -} - -// NewSimpleDialer returns a &net.Dialer{Timeout: timeout} instance. -func (*TProxyStdlib) NewSimpleDialer(timeout time.Duration) model.SimpleDialer { - return &net.Dialer{Timeout: timeout} -} - -// tproxyDefaultResolver is the resolver we use by default. -type tproxyDefaultResolver struct{} - -// LookupHost implements model.SimpleResolver.LookupHost. -func (r *tproxyDefaultResolver) LookupHost(ctx context.Context, domain string) ([]string, error) { - return getaddrinfoLookupHost(ctx, domain) -} - -// Network implements model.SimpleResolver.Network. -func (r *tproxyDefaultResolver) Network() string { - return getaddrinfoResolverNetwork() -}