diff --git a/internal/netxlite/certifi_test.go b/internal/netxlite/certifi_test.go new file mode 100644 index 0000000000..6851ca4e37 --- /dev/null +++ b/internal/netxlite/certifi_test.go @@ -0,0 +1,13 @@ +package netxlite + +import ( + "crypto/x509" + "testing" +) + +func TestPEMCerts(t *testing.T) { + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM([]byte(pemcerts)) { + t.Fatal("cannot load pemcerts") + } +} diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go index cba86d7178..7e78f61523 100644 --- a/internal/netxlite/dialer.go +++ b/internal/netxlite/dialer.go @@ -19,7 +19,16 @@ type Dialer interface { CloseIdleConnections() } -// NewDialerWithResolver creates a dialer using the given resolver and logger. +// NewDialerWithResolver creates a new Dialer. The returned Dialer +// has the following properties: +// +// 1. logs events using the given logger +// +// 2. resolves domain names using the givern resolver +// +// 3. wraps errors +// +// 4. has a configured connect timeout func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer { return &dialerLogger{ Dialer: &dialerResolver{ @@ -36,24 +45,30 @@ func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer { } } -// NewDialerWithoutResolver creates a dialer that uses the given -// logger and fails with ErrNoResolver when it is passed a domain name. +// NewDialerWithoutResolver is like NewDialerWithResolver except that +// it will fail with ErrNoResolver if passed a domain name. func NewDialerWithoutResolver(logger Logger) Dialer { return NewDialerWithResolver(logger, &nullResolver{}) } -// underlyingDialer is the Dialer we use by default. -var underlyingDialer = &net.Dialer{ - Timeout: 15 * time.Second, - KeepAlive: 15 * time.Second, +// dialerSystem dials using Go stdlib. +type dialerSystem struct { + // timeout is the OPTIONAL timeout used for testing. + timeout time.Duration } -// dialerSystem dials using Go stdlib. -type dialerSystem struct{} +// newUnderlyingDialer creates a new underlying dialer. +func (d *dialerSystem) newUnderlyingDialer() *net.Dialer { + t := d.timeout + if t <= 0 { + t = 15 * time.Second + } + return &net.Dialer{Timeout: t} +} // DialContext implements Dialer.DialContext. func (d *dialerSystem) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - return underlyingDialer.DialContext(ctx, network, address) + return d.newUnderlyingDialer().DialContext(ctx, network, address) } // CloseIdleConnections implements Dialer.CloseIdleConnections. @@ -61,8 +76,6 @@ func (d *dialerSystem) CloseIdleConnections() { // nothing } -var defaultDialer Dialer = &dialerSystem{} - // 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 { diff --git a/internal/netxlite/dialer_test.go b/internal/netxlite/dialer_test.go index bc624a3191..20b5f9601f 100644 --- a/internal/netxlite/dialer_test.go +++ b/internal/netxlite/dialer_test.go @@ -14,252 +14,284 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" ) -func TestDialerSystemCloseIdleConnections(t *testing.T) { - d := &dialerSystem{} - d.CloseIdleConnections() // should not crash +func TestNewDialer(t *testing.T) { + t.Run("produces a chain with the expected types", func(t *testing.T) { + dlr := NewDialerWithoutResolver(log.Log) + logger := dlr.(*dialerLogger) + if logger.Logger != log.Log { + t.Fatal("invalid logger") + } + reso := logger.Dialer.(*dialerResolver) + if _, okay := reso.Resolver.(*nullResolver); !okay { + t.Fatal("invalid Resolver type") + } + logger = reso.Dialer.(*dialerLogger) + if logger.Logger != log.Log { + t.Fatal("invalid logger") + } + errWrapper := logger.Dialer.(*dialerErrWrapper) + _ = errWrapper.Dialer.(*dialerSystem) + }) } -func TestDialerResolverNoPort(t *testing.T) { - dialer := &dialerResolver{Dialer: defaultDialer, Resolver: DefaultResolver} - conn, err := dialer.DialContext(context.Background(), "tcp", "ooni.nu") - if err == nil || !strings.HasSuffix(err.Error(), "missing port in address") { - t.Fatal("not the error we expected", err) - } - if conn != nil { - t.Fatal("expected a nil conn here") - } -} +func TestDialerSystem(t *testing.T) { + t.Run("has a default timeout of 15 seconds", func(t *testing.T) { + d := &dialerSystem{} + ud := d.newUnderlyingDialer() + if ud.Timeout != 15*time.Second { + t.Fatal("invalid default timeout") + } + }) -func TestDialerResolverLookupHostAddress(t *testing.T) { - dialer := &dialerResolver{Dialer: defaultDialer, Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return nil, errors.New("we should not call this function") - }, - }} - addrs, err := dialer.lookupHost(context.Background(), "1.1.1.1") - if err != nil { - t.Fatal(err) - } - if len(addrs) != 1 || addrs[0] != "1.1.1.1" { - t.Fatal("not the result we expected") - } -} + t.Run("we can change the default timeout for testing", func(t *testing.T) { + d := &dialerSystem{timeout: 1 * time.Second} + ud := d.newUnderlyingDialer() + if ud.Timeout != 1*time.Second { + t.Fatal("invalid default timeout") + } + }) -func TestDialerResolverLookupHostFailure(t *testing.T) { - expected := errors.New("mocked error") - dialer := &dialerResolver{Dialer: defaultDialer, Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return nil, expected - }, - }} - ctx := context.Background() - conn, err := dialer.DialContext(ctx, "tcp", "dns.google.com:853") - if !errors.Is(err, expected) { - t.Fatal("not the error we expected", err) - } - if conn != nil { - t.Fatal("expected nil conn") - } -} + t.Run("CloseIdleConnections", func(t *testing.T) { + d := &dialerSystem{} + d.CloseIdleConnections() // should not crash + }) -func TestDialerResolverDialForSingleIPFails(t *testing.T) { - dialer := &dialerResolver{Dialer: &mocks.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return nil, io.EOF - }, - }, 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") - } - if conn != nil { - t.Fatal("expected nil conn") - } + t.Run("DialContext with canceled context", func(t *testing.T) { + d := &dialerSystem{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() // immediately! + conn, err := d.DialContext(ctx, "tcp", "dns.google:443") + if err == nil || err.Error() != "dial tcp: operation was canceled" { + t.Fatal("unexpected err", err) + } + if conn != nil { + t.Fatal("unexpected conn") + } + }) } -func TestDialerResolverDialForManyIPFails(t *testing.T) { - dialer := &dialerResolver{ - Dialer: &mocks.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return nil, io.EOF - }, - }, Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return []string{"1.1.1.1", "8.8.8.8"}, nil - }, - }} - conn, err := dialer.DialContext(context.Background(), "tcp", "dot.dns:853") - if !errors.Is(err, io.EOF) { - t.Fatal("not the error we expected") - } - if conn != nil { - t.Fatal("expected nil conn") - } -} +func TestDialerResolver(t *testing.T) { + t.Run("DialContext", func(t *testing.T) { + t.Run("without a port", func(t *testing.T) { + d := &dialerResolver{ + Dialer: &dialerSystem{}, + Resolver: &resolverSystem{}, + } + conn, err := d.DialContext(context.Background(), "tcp", "ooni.nu") + if err == nil || !strings.HasSuffix(err.Error(), "missing port in address") { + t.Fatal("not the error we expected", err) + } + if conn != nil { + t.Fatal("expected a nil conn here") + } + }) -func TestDialerResolverDialForManyIPSuccess(t *testing.T) { - dialer := &dialerResolver{Dialer: &mocks.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return &mocks.Conn{ - MockClose: func() error { - return nil + t.Run("handles dialing error correctly for single IP address", func(t *testing.T) { + d := &dialerResolver{ + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { + return nil, io.EOF + }, }, - }, nil - }, - }, Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return []string{"1.1.1.1", "8.8.8.8"}, nil - }, - }} - conn, err := dialer.DialContext(context.Background(), "tcp", "dot.dns:853") - if err != nil { - t.Fatal("expected nil error here") - } - if conn == nil { - t.Fatal("expected non-nil conn") - } - conn.Close() -} + Resolver: &nullResolver{}, + } + conn, err := d.DialContext(context.Background(), "tcp", "1.1.1.1:853") + if !errors.Is(err, io.EOF) { + t.Fatal("not the error we expected") + } + if conn != nil { + t.Fatal("expected nil conn") + } + }) -func TestDialerResolverCloseIdleConnections(t *testing.T) { - var ( - calledDialer bool - calledResolver bool - ) - d := &dialerResolver{ - Dialer: &mocks.Dialer{ - MockCloseIdleConnections: func() { - calledDialer = true - }, - }, - Resolver: &mocks.Resolver{ - MockCloseIdleConnections: func() { - calledResolver = true - }, - }, - } - d.CloseIdleConnections() - if !calledDialer || !calledResolver { - t.Fatal("not called") - } -} + t.Run("handles dialing error correctly for many IP addresses", func(t *testing.T) { + d := &dialerResolver{ + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { + return nil, io.EOF + }, + }, + Resolver: &nullResolver{}, + } + conn, err := d.DialContext(context.Background(), "tcp", "1.1.1.1:853") + if !errors.Is(err, io.EOF) { + t.Fatal("not the error we expected") + } + if conn != nil { + t.Fatal("expected nil conn") + } + }) -func TestDialerLoggerSuccess(t *testing.T) { - d := &dialerLogger{ - Dialer: &mocks.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return &mocks.Conn{ - MockClose: func() error { - return nil + t.Run("handles dialing success correctly for many IP addresses", func(t *testing.T) { + d := &dialerResolver{ + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { + return &mocks.Conn{ + MockClose: func() error { + return nil + }, + }, nil }, - }, nil - }, - }, - Logger: log.Log, - } - conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") - if err != nil { - t.Fatal(err) - } - if conn == nil { - t.Fatal("expected non-nil conn here") - } - conn.Close() -} + }, Resolver: &mocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return []string{"1.1.1.1", "8.8.8.8"}, nil + }, + }, + } + conn, err := d.DialContext(context.Background(), "tcp", "dot.dns:853") + if err != nil { + t.Fatal(err) + } + if conn == nil { + t.Fatal("expected non-nil conn") + } + conn.Close() + }) + }) -func TestDialerLoggerFailure(t *testing.T) { - d := &dialerLogger{ - Dialer: &mocks.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") - } -} + t.Run("lookupHost", func(t *testing.T) { + t.Run("handles addresses correctly", func(t *testing.T) { + dialer := &dialerResolver{ + Dialer: &dialerSystem{}, + Resolver: &nullResolver{}, + } + addrs, err := dialer.lookupHost(context.Background(), "1.1.1.1") + if err != nil { + t.Fatal(err) + } + if len(addrs) != 1 || addrs[0] != "1.1.1.1" { + t.Fatal("not the result we expected") + } + }) + + t.Run("fails correctly on lookup error", func(t *testing.T) { + dialer := &dialerResolver{ + Dialer: &dialerSystem{}, + Resolver: &nullResolver{}, + } + ctx := context.Background() + conn, err := dialer.DialContext(ctx, "tcp", "dns.google.com:853") + if !errors.Is(err, ErrNoResolver) { + t.Fatal("not the error we expected", err) + } + if conn != nil { + t.Fatal("expected nil conn") + } + }) + }) -func TestDialerLoggerCloseIdleConnections(t *testing.T) { - var ( - calledDialer bool - ) - d := &dialerLogger{ - Dialer: &mocks.Dialer{ - MockCloseIdleConnections: func() { - calledDialer = true + t.Run("CloseIdleConnections", func(t *testing.T) { + var ( + calledDialer bool + calledResolver bool + ) + d := &dialerResolver{ + Dialer: &mocks.Dialer{ + MockCloseIdleConnections: func() { + calledDialer = true + }, + }, + Resolver: &mocks.Resolver{ + MockCloseIdleConnections: func() { + calledResolver = true + }, }, - }, - } - d.CloseIdleConnections() - if !calledDialer { - t.Fatal("not called") - } + } + d.CloseIdleConnections() + if !calledDialer || !calledResolver { + t.Fatal("not called") + } + }) } -func TestUnderlyingDialerHasTimeout(t *testing.T) { - expected := 15 * time.Second - if underlyingDialer.Timeout != expected { - t.Fatal("unexpected timeout value") - } -} +func TestDialerLogger(t *testing.T) { + t.Run("DialContext", func(t *testing.T) { + t.Run("handles success correctly", func(t *testing.T) { + d := &dialerLogger{ + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { + return &mocks.Conn{ + MockClose: func() error { + return nil + }, + }, nil + }, + }, + Logger: log.Log, + } + conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") + if err != nil { + t.Fatal(err) + } + if conn == nil { + t.Fatal("expected non-nil conn here") + } + conn.Close() + }) + + t.Run("handles failure correctly", func(t *testing.T) { + d := &dialerLogger{ + Dialer: &mocks.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 TestNewDialerWithoutResolverChain(t *testing.T) { - dlr := NewDialerWithoutResolver(log.Log) - dlog, okay := dlr.(*dialerLogger) - if !okay { - t.Fatal("invalid type") - } - if dlog.Logger != log.Log { - t.Fatal("invalid logger") - } - dreso, okay := dlog.Dialer.(*dialerResolver) - if !okay { - t.Fatal("invalid type") - } - if _, okay := dreso.Resolver.(*nullResolver); !okay { - t.Fatal("invalid Resolver type") - } - dlog, okay = dreso.Dialer.(*dialerLogger) - if !okay { - t.Fatal("invalid type") - } - if dlog.Logger != log.Log { - t.Fatal("invalid logger") - } - dew, okay := dlog.Dialer.(*dialerErrWrapper) - if !okay { - t.Fatal("invalid type") - } - if _, okay := dew.Dialer.(*dialerSystem); !okay { - t.Fatal("invalid type") - } + t.Run("CloseIdleConnections", func(t *testing.T) { + var ( + calledDialer bool + ) + d := &dialerLogger{ + Dialer: &mocks.Dialer{ + MockCloseIdleConnections: func() { + calledDialer = true + }, + }, + } + d.CloseIdleConnections() + if !calledDialer { + t.Fatal("not called") + } + }) } -func TestNewSingleUseDialerWorksAsIntended(t *testing.T) { - conn := &mocks.Conn{} - d := NewSingleUseDialer(conn) - outconn, err := d.DialContext(context.Background(), "", "") - if err != nil { - t.Fatal(err) - } - if conn != outconn { - t.Fatal("invalid outconn") - } - for i := 0; i < 4; i++ { - outconn, err = d.DialContext(context.Background(), "", "") - if !errors.Is(err, ErrNoConnReuse) { - t.Fatal("not the error we expected", err) +func TestDialerSingleUse(t *testing.T) { + t.Run("works as intended", func(t *testing.T) { + conn := &mocks.Conn{} + d := NewSingleUseDialer(conn) + outconn, err := d.DialContext(context.Background(), "", "") + if err != nil { + t.Fatal(err) } - if outconn != nil { - t.Fatal("expected nil outconn here") + if conn != outconn { + t.Fatal("invalid outconn") } - } + for i := 0; i < 4; i++ { + outconn, err = d.DialContext(context.Background(), "", "") + if !errors.Is(err, ErrNoConnReuse) { + t.Fatal("not the error we expected", err) + } + if outconn != nil { + t.Fatal("expected nil outconn here") + } + } + }) + + t.Run("CloseIdleConnections", func(t *testing.T) { + d := &dialerSingleUse{} + d.CloseIdleConnections() // does not crash + }) } func TestDialerErrWrapper(t *testing.T) { diff --git a/internal/netxlite/legacy.go b/internal/netxlite/legacy.go index 9a28cb0f0b..2f304b623e 100644 --- a/internal/netxlite/legacy.go +++ b/internal/netxlite/legacy.go @@ -10,7 +10,7 @@ import ( // These vars export internal names to legacy ooni/probe-cli code. var ( - DefaultDialer = defaultDialer + DefaultDialer = &dialerSystem{} DefaultTLSHandshaker = defaultTLSHandshaker NewConnUTLS = newConnUTLS ) diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go index 36fb64bfdd..253e7f3359 100644 --- a/internal/netxlite/tls.go +++ b/internal/netxlite/tls.go @@ -73,7 +73,7 @@ func TLSCipherSuiteString(value uint16) string { func NewDefaultCertPool() *x509.CertPool { pool := x509.NewCertPool() // Assumption: AppendCertsFromPEM cannot fail because we - // run this function already in the generate.go file + // have a test in certify_test.go that guarantees that pool.AppendCertsFromPEM([]byte(pemcerts)) return pool } diff --git a/internal/netxlite/tls_test.go b/internal/netxlite/tls_test.go index 7d40520cdf..8d18d3d41c 100644 --- a/internal/netxlite/tls_test.go +++ b/internal/netxlite/tls_test.go @@ -310,7 +310,7 @@ func TestTLSDialerDialTLSContextFailureSplitHostPort(t *testing.T) { func TestTLSDialerDialTLSContextFailureDialing(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // immediately fail - dialer := tlsDialer{Dialer: defaultDialer} + dialer := tlsDialer{Dialer: &dialerSystem{}} conn, err := dialer.DialTLSContext(ctx, "tcp", "www.google.com:443") if err == nil || !strings.HasSuffix(err.Error(), "operation was canceled") { t.Fatal("not the error we expected", err)