diff --git a/internal/cmd/oohelperd/internal/internal_test.go b/internal/cmd/oohelperd/internal/internal_test.go index 34cdffd..04d71e8 100644 --- a/internal/cmd/oohelperd/internal/internal_test.go +++ b/internal/cmd/oohelperd/internal/internal_test.go @@ -11,8 +11,8 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal" - "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" "github.com/ooni/probe-cli/v3/internal/iox" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) const simplerequest = `{ @@ -56,7 +56,7 @@ func TestWorkingAsIntended(t *testing.T) { Client: http.DefaultClient, Dialer: new(net.Dialer), MaxAcceptableBody: 1 << 24, - Resolver: resolver.SystemResolver{}, + Resolver: netxlite.ResolverSystem{}, } srv := httptest.NewServer(handler) defer srv.Close() diff --git a/internal/engine/experiment/ndt7/dial.go b/internal/engine/experiment/ndt7/dial.go index 31645d7..8a89b1a 100644 --- a/internal/engine/experiment/ndt7/dial.go +++ b/internal/engine/experiment/ndt7/dial.go @@ -10,6 +10,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) type dialManager struct { @@ -33,8 +34,8 @@ func newDialManager(ndt7URL string, logger model.Logger, userAgent string) dialM } func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*websocket.Conn, error) { - var reso resolver.Resolver = resolver.SystemResolver{} - reso = resolver.LoggingResolver{Resolver: reso, Logger: mgr.logger} + var reso resolver.Resolver = netxlite.ResolverSystem{} + reso = netxlite.ResolverLogger{Resolver: reso, Logger: mgr.logger} dlr := dialer.New(&dialer.Config{ ContextByteCounting: true, Logger: mgr.logger, diff --git a/internal/engine/legacy/netx/resolver.go b/internal/engine/legacy/netx/resolver.go index 0b685bf..7b5c574 100644 --- a/internal/engine/legacy/netx/resolver.go +++ b/internal/engine/legacy/netx/resolver.go @@ -12,6 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/handlers" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) var ( @@ -158,7 +159,7 @@ func resolverWrapTransport(txp resolver.RoundTripper) resolver.EmitterResolver { } func newResolverSystem() resolver.EmitterResolver { - return resolverWrapResolver(resolver.SystemResolver{}) + return resolverWrapResolver(netxlite.ResolverSystem{}) } func newResolverUDP(dialer resolver.Dialer, address string) resolver.EmitterResolver { diff --git a/internal/engine/netx/dialer/dialer.go b/internal/engine/netx/dialer/dialer.go index 1bcd272..e436172 100644 --- a/internal/engine/netx/dialer/dialer.go +++ b/internal/engine/netx/dialer/dialer.go @@ -6,6 +6,7 @@ import ( "net/url" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) // Dialer establishes network connections. @@ -64,10 +65,10 @@ type Config struct { // New creates a new Dialer from the specified config and resolver. func New(config *Config, resolver Resolver) Dialer { - var d Dialer = systemDialer + var d Dialer = netxlite.DefaultDialer d = &errorWrapperDialer{Dialer: d} if config.Logger != nil { - d = &loggingDialer{Dialer: d, Logger: config.Logger} + d = &netxlite.DialerLogger{Dialer: d, Logger: config.Logger} } if config.DialSaver != nil { d = &saverDialer{Dialer: d, Saver: config.DialSaver} @@ -75,7 +76,7 @@ func New(config *Config, resolver Resolver) Dialer { if config.ReadWriteSaver != nil { d = &saverConnDialer{Dialer: d, Saver: config.ReadWriteSaver} } - d = &dnsDialer{Resolver: resolver, Dialer: d} + d = &netxlite.DialerResolver{Resolver: resolver, Dialer: d} d = &proxyDialer{ProxyURL: config.ProxyURL, Dialer: d} if config.ContextByteCounting { d = &byteCounterDialer{Dialer: d} diff --git a/internal/engine/netx/dialer/dialer_test.go b/internal/engine/netx/dialer/dialer_test.go index b861be0..57202d2 100644 --- a/internal/engine/netx/dialer/dialer_test.go +++ b/internal/engine/netx/dialer/dialer_test.go @@ -7,6 +7,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestNewCreatesTheExpectedChain(t *testing.T) { @@ -30,7 +31,7 @@ func TestNewCreatesTheExpectedChain(t *testing.T) { if !ok { t.Fatal("not a proxyDialer") } - dnsd, ok := pd.Dialer.(*dnsDialer) + dnsd, ok := pd.Dialer.(*netxlite.DialerResolver) if !ok { t.Fatal("not a dnsDialer") } @@ -42,7 +43,7 @@ func TestNewCreatesTheExpectedChain(t *testing.T) { if !ok { t.Fatal("not a saverDialer") } - ld, ok := sd.Dialer.(*loggingDialer) + ld, ok := sd.Dialer.(*netxlite.DialerLogger) if !ok { t.Fatal("not a loggingDialer") } diff --git a/internal/engine/netx/dialer/dns.go b/internal/engine/netx/dialer/dns.go deleted file mode 100644 index 0062954..0000000 --- a/internal/engine/netx/dialer/dns.go +++ /dev/null @@ -1,71 +0,0 @@ -package dialer - -import ( - "context" - "errors" - "net" - "strings" - - "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" -) - -// dnsDialer is a dialer that uses the configured Resolver to resolver a -// domain name to IP addresses, and the configured Dialer to connect. -type dnsDialer struct { - Dialer - Resolver Resolver -} - -// DialContext implements Dialer.DialContext. -func (d *dnsDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - onlyhost, onlyport, err := net.SplitHostPort(address) - if err != nil { - return nil, err - } - var addrs []string - addrs, err = d.lookupHost(ctx, onlyhost) - if err != nil { - return nil, err - } - var errorslist []error - for _, addr := range addrs { - target := net.JoinHostPort(addr, onlyport) - conn, err := d.Dialer.DialContext(ctx, network, target) - if err == nil { - return conn, nil - } - errorslist = append(errorslist, err) - } - return nil, ReduceErrors(errorslist) -} - -// ReduceErrors finds a known error in a list of errors since it's probably most relevant. -func ReduceErrors(errorslist []error) error { - if len(errorslist) == 0 { - return nil - } - // If we have a known error, let's consider this the real error - // since it's probably most relevant. Otherwise let's return the - // first considering that (1) local resolvers likely will give - // us IPv4 first and (2) also our resolver does that. So, in case - // the user has no IPv6 connectivity, an IPv6 error is going to - // appear later in the list of errors. - for _, err := range errorslist { - var wrapper *errorx.ErrWrapper - if errors.As(err, &wrapper) && !strings.HasPrefix( - err.Error(), "unknown_failure", - ) { - return err - } - } - // TODO(bassosimone): handle this case in a better way - return errorslist[0] -} - -// lookupHost performs a domain name resolution. -func (d *dnsDialer) lookupHost(ctx context.Context, hostname string) ([]string, error) { - if net.ParseIP(hostname) != nil { - return []string{hostname}, nil - } - return d.Resolver.LookupHost(ctx, hostname) -} diff --git a/internal/engine/netx/dialer/logging.go b/internal/engine/netx/dialer/logging.go deleted file mode 100644 index cb53b93..0000000 --- a/internal/engine/netx/dialer/logging.go +++ /dev/null @@ -1,23 +0,0 @@ -package dialer - -import ( - "context" - "net" - "time" -) - -// loggingDialer is a Dialer with logging -type loggingDialer struct { - Dialer - Logger Logger -} - -// DialContext implements Dialer.DialContext -func (d *loggingDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - d.Logger.Debugf("dial %s/%s...", address, network) - start := time.Now() - conn, err := d.Dialer.DialContext(ctx, network, address) - stop := time.Now() - d.Logger.Debugf("dial %s/%s... %+v in %s", address, network, err, stop.Sub(start)) - return conn, err -} diff --git a/internal/engine/netx/dialer/logging_test.go b/internal/engine/netx/dialer/logging_test.go deleted file mode 100644 index f1b88b3..0000000 --- a/internal/engine/netx/dialer/logging_test.go +++ /dev/null @@ -1,30 +0,0 @@ -package dialer - -import ( - "context" - "errors" - "io" - "net" - "testing" - - "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/netx/mockablex" -) - -func TestLoggingDialerFailure(t *testing.T) { - d := &loggingDialer{ - Dialer: mockablex.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return nil, io.EOF - }, - }, - Logger: log.Log, - } - conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") - if !errors.Is(err, io.EOF) { - t.Fatal("not the error we expected") - } - if conn != nil { - t.Fatal("expected nil conn here") - } -} diff --git a/internal/engine/netx/dialer/system.go b/internal/engine/netx/dialer/system.go deleted file mode 100644 index 37f2258..0000000 --- a/internal/engine/netx/dialer/system.go +++ /dev/null @@ -1,12 +0,0 @@ -package dialer - -import ( - "net" - "time" -) - -// systemDialer is the underlying net.Dialer. -var systemDialer = &net.Dialer{ - Timeout: 15 * time.Second, - KeepAlive: 15 * time.Second, -} diff --git a/internal/engine/netx/dialer/system_test.go b/internal/engine/netx/dialer/system_test.go deleted file mode 100644 index a21073a..0000000 --- a/internal/engine/netx/dialer/system_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package dialer - -import ( - "strings" - "testing" - "time" - - "github.com/ooni/psiphon/oopsi/golang.org/x/net/context" -) - -func TestSystemDialerWorks(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() // fail immediately - conn, err := systemDialer.DialContext(ctx, "tcp", "8.8.8.8:853") - if err == nil || !strings.HasSuffix(err.Error(), "operation was canceled") { - t.Fatal("not the error we expected", err) - } - if conn != nil { - t.Fatal("expected nil conn here") - } -} - -func TestSystemDialerHasTimeout(t *testing.T) { - expected := 15 * time.Second - if systemDialer.Timeout != expected { - t.Fatal("unexpected timeout value") - } -} diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 0a3bcb8..13a766e 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -39,6 +39,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) // Logger is the logger assumed by this package @@ -114,7 +115,7 @@ var defaultCertPool *x509.CertPool = tlsx.NewDefaultCertPool() // NewResolver creates a new resolver from the specified config func NewResolver(config Config) Resolver { if config.BaseResolver == nil { - config.BaseResolver = resolver.SystemResolver{} + config.BaseResolver = netxlite.ResolverSystem{} } var r Resolver = config.BaseResolver r = resolver.AddressResolver{Resolver: r} @@ -133,7 +134,7 @@ func NewResolver(config Config) Resolver { } r = resolver.ErrorWrapperResolver{Resolver: r} if config.Logger != nil { - r = resolver.LoggingResolver{Logger: config.Logger, Resolver: r} + r = netxlite.ResolverLogger{Logger: config.Logger, Resolver: r} } if config.ResolveSaver != nil { r = resolver.SaverResolver{Resolver: r, Saver: config.ResolveSaver} @@ -317,7 +318,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride, } switch resolverURL.Scheme { case "system": - c.Resolver = resolver.SystemResolver{} + c.Resolver = netxlite.ResolverSystem{} return c, nil case "https": config.TLSConfig.NextProtos = []string{"h2", "http/1.1"} diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index bf41ea9..dbca660 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -15,6 +15,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestNewResolverVanilla(t *testing.T) { @@ -31,7 +32,7 @@ func TestNewResolverVanilla(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ar.Resolver.(resolver.SystemResolver) + _, ok = ar.Resolver.(netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -81,7 +82,7 @@ func TestNewResolverWithBogonFilter(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ar.Resolver.(resolver.SystemResolver) + _, ok = ar.Resolver.(netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -95,7 +96,7 @@ func TestNewResolverWithLogging(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - lr, ok := ir.Resolver.(resolver.LoggingResolver) + lr, ok := ir.Resolver.(netxlite.ResolverLogger) if !ok { t.Fatal("not the resolver we expected") } @@ -110,7 +111,7 @@ func TestNewResolverWithLogging(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ar.Resolver.(resolver.SystemResolver) + _, ok = ar.Resolver.(netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -140,7 +141,7 @@ func TestNewResolverWithSaver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ar.Resolver.(resolver.SystemResolver) + _, ok = ar.Resolver.(netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -169,7 +170,7 @@ func TestNewResolverWithReadWriteCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ar.Resolver.(resolver.SystemResolver) + _, ok = ar.Resolver.(netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -203,7 +204,7 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - _, ok = ar.Resolver.(resolver.SystemResolver) + _, ok = ar.Resolver.(netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -597,7 +598,7 @@ func TestNewDNSClientSystemResolver(t *testing.T) { if err != nil { t.Fatal(err) } - if _, ok := dnsclient.Resolver.(resolver.SystemResolver); !ok { + if _, ok := dnsclient.Resolver.(netxlite.ResolverSystem); !ok { t.Fatal("not the resolver we expected") } dnsclient.CloseIdleConnections() @@ -609,7 +610,7 @@ func TestNewDNSClientEmpty(t *testing.T) { if err != nil { t.Fatal(err) } - if _, ok := dnsclient.Resolver.(resolver.SystemResolver); !ok { + if _, ok := dnsclient.Resolver.(netxlite.ResolverSystem); !ok { t.Fatal("not the resolver we expected") } dnsclient.CloseIdleConnections() diff --git a/internal/engine/netx/quicdialer/dns.go b/internal/engine/netx/quicdialer/dns.go index 5f390e2..9d83be6 100644 --- a/internal/engine/netx/quicdialer/dns.go +++ b/internal/engine/netx/quicdialer/dns.go @@ -6,7 +6,7 @@ import ( "net" "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) // DNSDialer is a dialer that uses the configured Resolver to resolve a @@ -45,7 +45,7 @@ func (d DNSDialer) DialContext( errorslist = append(errorslist, err) } // TODO(bassosimone): maybe ReduceErrors could be in netx/internal. - return nil, dialer.ReduceErrors(errorslist) + return nil, netxlite.ReduceErrors(errorslist) } // LookupHost implements Resolver.LookupHost diff --git a/internal/engine/netx/resolver/chain_test.go b/internal/engine/netx/resolver/chain_test.go index e6aa1ae..3a15045 100644 --- a/internal/engine/netx/resolver/chain_test.go +++ b/internal/engine/netx/resolver/chain_test.go @@ -5,12 +5,13 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestChainLookupHost(t *testing.T) { r := resolver.ChainResolver{ Primary: resolver.NewFakeResolverThatFails(), - Secondary: resolver.SystemResolver{}, + Secondary: netxlite.ResolverSystem{}, } if r.Address() != "" { t.Fatal("invalid address") diff --git a/internal/engine/netx/resolver/integration_test.go b/internal/engine/netx/resolver/integration_test.go index 28526dd..39cebb9 100644 --- a/internal/engine/netx/resolver/integration_test.go +++ b/internal/engine/netx/resolver/integration_test.go @@ -8,6 +8,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func init() { @@ -18,7 +19,7 @@ func testresolverquick(t *testing.T, reso resolver.Resolver) { if testing.Short() { t.Skip("skip test in short mode") } - reso = resolver.LoggingResolver{Logger: log.Log, Resolver: reso} + reso = netxlite.ResolverLogger{Logger: log.Log, Resolver: reso} addrs, err := reso.LookupHost(context.Background(), "dns.google.com") if err != nil { t.Fatal(err) @@ -44,7 +45,7 @@ func testresolverquickidna(t *testing.T, reso resolver.Resolver) { t.Skip("skip test in short mode") } reso = resolver.IDNAResolver{ - resolver.LoggingResolver{Logger: log.Log, Resolver: reso}, + netxlite.ResolverLogger{Logger: log.Log, Resolver: reso}, } addrs, err := reso.LookupHost(context.Background(), "яндекс.рф") if err != nil { @@ -56,7 +57,7 @@ func testresolverquickidna(t *testing.T, reso resolver.Resolver) { } func TestNewResolverSystem(t *testing.T) { - reso := resolver.SystemResolver{} + reso := netxlite.ResolverSystem{} testresolverquick(t, reso) testresolverquickidna(t, reso) } diff --git a/internal/engine/netx/resolver/logging.go b/internal/engine/netx/resolver/logging.go deleted file mode 100644 index 40e17f2..0000000 --- a/internal/engine/netx/resolver/logging.go +++ /dev/null @@ -1,30 +0,0 @@ -package resolver - -import ( - "context" - "time" -) - -// Logger is the logger assumed by this package -type Logger interface { - Debugf(format string, v ...interface{}) - Debug(message string) -} - -// LoggingResolver is a resolver that emits events -type LoggingResolver struct { - Resolver - Logger Logger -} - -// LookupHost returns the IP addresses of a host -func (r LoggingResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { - r.Logger.Debugf("resolve %s...", hostname) - start := time.Now() - addrs, err := r.Resolver.LookupHost(ctx, hostname) - stop := time.Now() - r.Logger.Debugf("resolve %s... (%+v, %+v) in %s", hostname, addrs, err, stop.Sub(start)) - return addrs, err -} - -var _ Resolver = LoggingResolver{} diff --git a/internal/engine/netx/resolver/logging_test.go b/internal/engine/netx/resolver/logging_test.go deleted file mode 100644 index 4cc18da..0000000 --- a/internal/engine/netx/resolver/logging_test.go +++ /dev/null @@ -1,23 +0,0 @@ -package resolver_test - -import ( - "context" - "testing" - - "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" -) - -func TestLoggingResolver(t *testing.T) { - r := resolver.LoggingResolver{ - Logger: log.Log, - Resolver: resolver.NewFakeResolverThatFails(), - } - addrs, err := r.LookupHost(context.Background(), "www.google.com") - if err == nil { - t.Fatal("expected an error here") - } - if addrs != nil { - t.Fatal("expected nil addr here") - } -} diff --git a/internal/engine/netx/resolver/system.go b/internal/engine/netx/resolver/system.go deleted file mode 100644 index 6c0c049..0000000 --- a/internal/engine/netx/resolver/system.go +++ /dev/null @@ -1,29 +0,0 @@ -package resolver - -import ( - "context" - "net" -) - -// SystemResolver is the system resolver. -type SystemResolver struct{} - -// LookupHost implements Resolver.LookupHost. -func (r SystemResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { - return net.DefaultResolver.LookupHost(ctx, hostname) -} - -// Network implements Resolver.Network. -func (r SystemResolver) Network() string { - return "system" -} - -// Address implements Resolver.Address. -func (r SystemResolver) Address() string { - return "" -} - -// Default is the resolver we use by default. -var Default = SystemResolver{} - -var _ Resolver = SystemResolver{} diff --git a/internal/engine/netx/resolver/system_test.go b/internal/engine/netx/resolver/system_test.go deleted file mode 100644 index a30fb34..0000000 --- a/internal/engine/netx/resolver/system_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package resolver_test - -import ( - "context" - "testing" - - "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" -) - -func TestSystemResolverLookupHost(t *testing.T) { - r := resolver.SystemResolver{} - if r.Network() != "system" { - t.Fatal("invalid Network") - } - if r.Address() != "" { - t.Fatal("invalid Address") - } - addrs, err := r.LookupHost(context.Background(), "dns.google.com") - if err != nil { - t.Fatal(err) - } - if addrs == nil { - t.Fatal("expected non-nil result here") - } -} diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go new file mode 100644 index 0000000..200d50d --- /dev/null +++ b/internal/netxlite/dialer.go @@ -0,0 +1,86 @@ +package netxlite + +import ( + "context" + "net" + "time" +) + +// Dialer establishes network connections. +type Dialer interface { + // DialContext behaves like net.Dialer.DialContext. + DialContext(ctx context.Context, network, address string) (net.Conn, error) +} + +// DefaultDialer is the Dialer we use by default. +var DefaultDialer = &net.Dialer{ + Timeout: 15 * time.Second, + KeepAlive: 15 * time.Second, +} + +var _ Dialer = DefaultDialer + +// DialerResolver is a dialer that uses the configured Resolver to resolver a +// domain name to IP addresses, and the configured Dialer to connect. +type DialerResolver struct { + // Dialer is the underlying Dialer. + Dialer Dialer + + // Resolver is the underlying Resolver. + Resolver Resolver +} + +var _ Dialer = &DialerResolver{} + +// DialContext implements Dialer.DialContext. +func (d *DialerResolver) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + onlyhost, onlyport, err := net.SplitHostPort(address) + if err != nil { + return nil, err + } + var addrs []string + addrs, err = d.lookupHost(ctx, onlyhost) + if err != nil { + return nil, err + } + // TODO(bassosimone): here we should be using multierror rather + // than just calling ReduceErrors. We are not ready to do that + // yet, though. To do that, we need first to modify nettests so + // that we actually avoid dialing when measuring. + var errorslist []error + for _, addr := range addrs { + target := net.JoinHostPort(addr, onlyport) + conn, err := d.Dialer.DialContext(ctx, network, target) + if err == nil { + return conn, nil + } + errorslist = append(errorslist, err) + } + return nil, ReduceErrors(errorslist) +} + +// lookupHost performs a domain name resolution. +func (d *DialerResolver) lookupHost(ctx context.Context, hostname string) ([]string, error) { + if net.ParseIP(hostname) != nil { + return []string{hostname}, nil + } + return d.Resolver.LookupHost(ctx, hostname) +} + +// DialerLogger is a Dialer with logging +type DialerLogger struct { + Dialer + Logger Logger +} + +var _ Dialer = &DialerLogger{} + +// DialContext implements Dialer.DialContext +func (d *DialerLogger) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + d.Logger.Debugf("dial %s/%s...", address, network) + start := time.Now() + conn, err := d.Dialer.DialContext(ctx, network, address) + stop := time.Now() + d.Logger.Debugf("dial %s/%s... %+v in %s", address, network, err, stop.Sub(start)) + return conn, err +} diff --git a/internal/engine/netx/dialer/dns_test.go b/internal/netxlite/dialer_test.go similarity index 54% rename from internal/engine/netx/dialer/dns_test.go rename to internal/netxlite/dialer_test.go index bdbdf68..e07a81d 100644 --- a/internal/engine/netx/dialer/dns_test.go +++ b/internal/netxlite/dialer_test.go @@ -1,18 +1,20 @@ -package dialer +package netxlite import ( "context" "errors" "io" "net" + "strings" "testing" + "time" - "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" + "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/netx/mockablex" ) -func TestDNSDialerNoPort(t *testing.T) { - dialer := &dnsDialer{Dialer: new(net.Dialer), Resolver: new(net.Resolver)} +func TestDialerResolverNoPort(t *testing.T) { + dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: DefaultResolver} conn, err := dialer.DialContext(context.Background(), "tcp", "antani.ooni.nu") if err == nil { t.Fatal("expected an error here") @@ -22,8 +24,8 @@ func TestDNSDialerNoPort(t *testing.T) { } } -func TestDNSDialerLookupHostAddress(t *testing.T) { - dialer := &dnsDialer{Dialer: new(net.Dialer), Resolver: MockableResolver{ +func TestDialerResolverLookupHostAddress(t *testing.T) { + dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: MockableResolver{ Err: errors.New("mocked error"), }} addrs, err := dialer.lookupHost(context.Background(), "1.1.1.1") @@ -35,9 +37,9 @@ func TestDNSDialerLookupHostAddress(t *testing.T) { } } -func TestDNSDialerLookupHostFailure(t *testing.T) { +func TestDialerResolverLookupHostFailure(t *testing.T) { expected := errors.New("mocked error") - dialer := &dnsDialer{Dialer: new(net.Dialer), Resolver: MockableResolver{ + dialer := &DialerResolver{Dialer: new(net.Dialer), Resolver: MockableResolver{ Err: expected, }} conn, err := dialer.DialContext(context.Background(), "tcp", "dns.google.com:853") @@ -58,12 +60,20 @@ func (r MockableResolver) LookupHost(ctx context.Context, host string) ([]string return r.Addresses, r.Err } -func TestDNSDialerDialForSingleIPFails(t *testing.T) { - dialer := &dnsDialer{Dialer: mockablex.Dialer{ +func (r MockableResolver) Network() string { + return "mockable" +} + +func (r MockableResolver) Address() string { + return "" +} + +func TestDialerResolverDialForSingleIPFails(t *testing.T) { + dialer := &DialerResolver{Dialer: mockablex.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { return nil, io.EOF }, - }, Resolver: new(net.Resolver)} + }, Resolver: DefaultResolver} conn, err := dialer.DialContext(context.Background(), "tcp", "1.1.1.1:853") if !errors.Is(err, io.EOF) { t.Fatal("not the error we expected") @@ -73,8 +83,8 @@ func TestDNSDialerDialForSingleIPFails(t *testing.T) { } } -func TestDNSDialerDialForManyIPFails(t *testing.T) { - dialer := &dnsDialer{ +func TestDialerResolverDialForManyIPFails(t *testing.T) { + dialer := &DialerResolver{ Dialer: mockablex.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { return nil, io.EOF @@ -91,8 +101,8 @@ func TestDNSDialerDialForManyIPFails(t *testing.T) { } } -func TestDNSDialerDialForManyIPSuccess(t *testing.T) { - dialer := &dnsDialer{Dialer: mockablex.Dialer{ +func TestDialerResolverDialForManyIPSuccess(t *testing.T) { + dialer := &DialerResolver{Dialer: mockablex.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { return &mockablex.Conn{ MockClose: func() error { @@ -113,43 +123,39 @@ func TestDNSDialerDialForManyIPSuccess(t *testing.T) { conn.Close() } -func TestReduceErrors(t *testing.T) { - t.Run("no errors", func(t *testing.T) { - result := ReduceErrors(nil) - if result != nil { - t.Fatal("wrong result") - } - }) - - t.Run("single error", func(t *testing.T) { - err := errors.New("mocked error") - result := ReduceErrors([]error{err}) - if result != err { - t.Fatal("wrong result") - } - }) - - t.Run("multiple errors", func(t *testing.T) { - err1 := errors.New("mocked error #1") - err2 := errors.New("mocked error #2") - result := ReduceErrors([]error{err1, err2}) - if result.Error() != "mocked error #1" { - t.Fatal("wrong result") - } - }) - - t.Run("multiple errors with meaningful ones", func(t *testing.T) { - err1 := errors.New("mocked error #1") - err2 := &errorx.ErrWrapper{ - Failure: "unknown_failure: antani", - } - err3 := &errorx.ErrWrapper{ - Failure: errorx.FailureConnectionRefused, - } - err4 := errors.New("mocked error #3") - result := ReduceErrors([]error{err1, err2, err3, err4}) - if result.Error() != errorx.FailureConnectionRefused { - t.Fatal("wrong result") - } - }) +func TestDialerLoggerFailure(t *testing.T) { + d := &DialerLogger{ + Dialer: mockablex.Dialer{ + MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { + return nil, io.EOF + }, + }, + Logger: log.Log, + } + conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") + if !errors.Is(err, io.EOF) { + t.Fatal("not the error we expected") + } + if conn != nil { + t.Fatal("expected nil conn here") + } +} + +func TestDefaultDialerWorks(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // fail immediately + conn, err := DefaultDialer.DialContext(ctx, "tcp", "8.8.8.8:853") + if err == nil || !strings.HasSuffix(err.Error(), "operation was canceled") { + t.Fatal("not the error we expected", err) + } + if conn != nil { + t.Fatal("expected nil conn here") + } +} + +func TestDefaultDialerHasTimeout(t *testing.T) { + expected := 15 * time.Second + if DefaultDialer.Timeout != expected { + t.Fatal("unexpected timeout value") + } } diff --git a/internal/netxlite/doc.go b/internal/netxlite/doc.go new file mode 100644 index 0000000..59dafb3 --- /dev/null +++ b/internal/netxlite/doc.go @@ -0,0 +1,46 @@ +// Package netxlite contains network extensions. +// +// This package is the basic networking building block that you +// should be using every time you need networking. +// +// Naming and history +// +// Previous versions of this package were called netx. Compared to such +// versions this package is lightweight because it does not contain code +// to perform the measurements, hence its name. +// +// Design +// +// We want to potentially be able to observe each low-level operation +// separately, even though this is not done by this package. This is +// the use case where we are performing measurements. +// +// We also want to be able to use this package in a more casual way +// without having to compose each operation separately. This, instead, is +// the use case where we're communicating with the OONI backend. +// +// We want to optionally provide detailed logging of every operation, +// thus users can use `-v` to obtain OONI logs. +// +// We also want to mock any underlying dependency for testing. +// +// Operations +// +// This package implements the following operations: +// +// 1. establishing a TCP connection; +// +// 2. performing a domain name resolution; +// +// 3. performing the TLS handshake; +// +// 4. performing the QUIC handshake; +// +// 5. dialing with TCP, TLS, and QUIC (where in this context dialing +// means combining domain name resolution and "connecting"); +// +// 6. performing HTTP, HTTP2, and HTTP3 round trips. +// +// Operations 1, 2, 3, and 4 are used when we perform measurements, +// while 5 and 6 are mostly used when speaking with our backend. +package netxlite diff --git a/internal/netxlite/legacy.go b/internal/netxlite/legacy.go new file mode 100644 index 0000000..670320c --- /dev/null +++ b/internal/netxlite/legacy.go @@ -0,0 +1,39 @@ +package netxlite + +import ( + "errors" + "strings" + + "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" +) + +// ReduceErrors finds a known error in a list of errors since +// it's probably most relevant. +// +// Deprecation warning +// +// Albeit still used, this function is now DEPRECATED. +// +// In perspective, we would like to transition to a scenario where +// full dialing is NOT used for measurements and we return a multierror here. +func ReduceErrors(errorslist []error) error { + if len(errorslist) == 0 { + return nil + } + // If we have a known error, let's consider this the real error + // since it's probably most relevant. Otherwise let's return the + // first considering that (1) local resolvers likely will give + // us IPv4 first and (2) also our resolver does that. So, in case + // the user has no IPv6 connectivity, an IPv6 error is going to + // appear later in the list of errors. + for _, err := range errorslist { + var wrapper *errorx.ErrWrapper + if errors.As(err, &wrapper) && !strings.HasPrefix( + err.Error(), "unknown_failure", + ) { + return err + } + } + // TODO(bassosimone): handle this case in a better way + return errorslist[0] +} diff --git a/internal/netxlite/legacy_test.go b/internal/netxlite/legacy_test.go new file mode 100644 index 0000000..a6d7254 --- /dev/null +++ b/internal/netxlite/legacy_test.go @@ -0,0 +1,49 @@ +package netxlite + +import ( + "errors" + "testing" + + "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" +) + +func TestReduceErrors(t *testing.T) { + t.Run("no errors", func(t *testing.T) { + result := ReduceErrors(nil) + if result != nil { + t.Fatal("wrong result") + } + }) + + t.Run("single error", func(t *testing.T) { + err := errors.New("mocked error") + result := ReduceErrors([]error{err}) + if result != err { + t.Fatal("wrong result") + } + }) + + t.Run("multiple errors", func(t *testing.T) { + err1 := errors.New("mocked error #1") + err2 := errors.New("mocked error #2") + result := ReduceErrors([]error{err1, err2}) + if result.Error() != "mocked error #1" { + t.Fatal("wrong result") + } + }) + + t.Run("multiple errors with meaningful ones", func(t *testing.T) { + err1 := errors.New("mocked error #1") + err2 := &errorx.ErrWrapper{ + Failure: "unknown_failure: antani", + } + err3 := &errorx.ErrWrapper{ + Failure: errorx.FailureConnectionRefused, + } + err4 := errors.New("mocked error #3") + result := ReduceErrors([]error{err1, err2, err3, err4}) + if result.Error() != errorx.FailureConnectionRefused { + t.Fatal("wrong result") + } + }) +} diff --git a/internal/netxlite/logger.go b/internal/netxlite/logger.go new file mode 100644 index 0000000..089a36d --- /dev/null +++ b/internal/netxlite/logger.go @@ -0,0 +1,7 @@ +package netxlite + +// Logger is the interface we expect from a logger. +type Logger interface { + // Debugf formats and emits a debug message. + Debugf(format string, v ...interface{}) +} diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go new file mode 100644 index 0000000..99d07e8 --- /dev/null +++ b/internal/netxlite/resolver.go @@ -0,0 +1,78 @@ +package netxlite + +import ( + "context" + "net" + "time" +) + +// Resolver performs domain name resolutions. +type Resolver interface { + // LookupHost behaves like net.Resolver.LookupHost. + LookupHost(ctx context.Context, hostname string) (addrs []string, err error) +} + +// ResolverSystem is the system resolver. +type ResolverSystem struct{} + +var _ Resolver = ResolverSystem{} + +// LookupHost implements Resolver.LookupHost. +func (r ResolverSystem) LookupHost(ctx context.Context, hostname string) ([]string, error) { + return net.DefaultResolver.LookupHost(ctx, hostname) +} + +// Network implements Resolver.Network. +func (r ResolverSystem) Network() string { + return "system" +} + +// Address implements Resolver.Address. +func (r ResolverSystem) Address() string { + return "" +} + +// DefaultResolver is the resolver we use by default. +var DefaultResolver = ResolverSystem{} + +// ResolverLogger is a resolver that emits events +type ResolverLogger struct { + Resolver + Logger Logger +} + +var _ Resolver = ResolverLogger{} + +// LookupHost returns the IP addresses of a host +func (r ResolverLogger) LookupHost(ctx context.Context, hostname string) ([]string, error) { + r.Logger.Debugf("resolve %s...", hostname) + start := time.Now() + addrs, err := r.Resolver.LookupHost(ctx, hostname) + stop := time.Now() + r.Logger.Debugf("resolve %s... (%+v, %+v) in %s", hostname, addrs, err, stop.Sub(start)) + return addrs, err +} + +type resolverNetworker interface { + Network() string +} + +// Network implements Resolver.Network. +func (r ResolverLogger) Network() string { + if rn, ok := r.Resolver.(resolverNetworker); ok { + return rn.Network() + } + return "logger" +} + +type resolverAddresser interface { + Address() string +} + +// Address implements Resolver.Address. +func (r ResolverLogger) Address() string { + if ra, ok := r.Resolver.(resolverAddresser); ok { + return ra.Address() + } + return "" +} diff --git a/internal/netxlite/resolver_test.go b/internal/netxlite/resolver_test.go new file mode 100644 index 0000000..7490589 --- /dev/null +++ b/internal/netxlite/resolver_test.go @@ -0,0 +1,56 @@ +package netxlite + +import ( + "context" + "net" + "testing" + + "github.com/apex/log" +) + +func TestResolverSystemLookupHost(t *testing.T) { + r := ResolverSystem{} + if r.Network() != "system" { + t.Fatal("invalid Network") + } + if r.Address() != "" { + t.Fatal("invalid Address") + } + addrs, err := r.LookupHost(context.Background(), "dns.google.com") + if err != nil { + t.Fatal(err) + } + if addrs == nil { + t.Fatal("expected non-nil result here") + } +} + +func TestResolverLoggerWithFailure(t *testing.T) { + r := ResolverLogger{ + Logger: log.Log, + Resolver: DefaultResolver, + } + if r.Network() != "system" { + t.Fatal("invalid Network") + } + if r.Address() != "" { + t.Fatal("invalid Address") + } + addrs, err := r.LookupHost(context.Background(), "nonexistent.antani") + if err == nil { + t.Fatal("expected an error here") + } + if addrs != nil { + t.Fatal("expected nil addr here") + } +} + +func TestResolverLoggerDefaultNetworkAddress(t *testing.T) { + r := &ResolverLogger{Logger: log.Log, Resolver: &net.Resolver{}} + if r.Network() != "logger" { + t.Fatal("invalid Network") + } + if r.Address() != "" { + t.Fatal("invalid Address") + } +}