From 6a1e92cace46c14ae171669fc06ad70dfd65c162 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Sun, 5 Sep 2021 20:41:46 +0200 Subject: [PATCH] feat(netxlite): add dialer factory, simplify resolver factory (#459) See https://github.com/ooni/probe/issues/1591 --- internal/netxlite/dialer.go | 20 +++++++++++++++ internal/netxlite/dialer_test.go | 28 ++++++++++++++++++++ internal/netxlite/http3_test.go | 2 +- internal/netxlite/http_test.go | 4 +-- internal/netxlite/quic_test.go | 8 +++--- internal/netxlite/resolver.go | 41 +++++++++++++++++++++++------- internal/netxlite/resolver_test.go | 23 ++++++++++++++--- 7 files changed, 107 insertions(+), 19 deletions(-) diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go index 3548cb1..e67f284 100644 --- a/internal/netxlite/dialer.go +++ b/internal/netxlite/dialer.go @@ -15,6 +15,26 @@ type Dialer interface { CloseIdleConnections() } +// NewDialerWithResolver creates a dialer using the given resolver and logger. +func NewDialerWithResolver(logger Logger, resolver Resolver) Dialer { + return &dialerLogger{ + Dialer: &dialerResolver{ + Dialer: &dialerLogger{ + Dialer: &dialerSystem{}, + Logger: logger, + }, + Resolver: resolver, + }, + Logger: logger, + } +} + +// NewDialerWithoutResolver creates a dialer that uses the given +// logger and fails with ErrNoResolver when it is 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, diff --git a/internal/netxlite/dialer_test.go b/internal/netxlite/dialer_test.go index c91d69c..91b5478 100644 --- a/internal/netxlite/dialer_test.go +++ b/internal/netxlite/dialer_test.go @@ -207,3 +207,31 @@ func TestUnderlyingDialerHasTimeout(t *testing.T) { t.Fatal("unexpected timeout value") } } + +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") + } + if _, okay := dlog.Dialer.(*dialerSystem); !okay { + t.Fatal("invalid type") + } +} diff --git a/internal/netxlite/http3_test.go b/internal/netxlite/http3_test.go index e3e9434..9a2e464 100644 --- a/internal/netxlite/http3_test.go +++ b/internal/netxlite/http3_test.go @@ -13,7 +13,7 @@ func TestHTTP3TransportWorks(t *testing.T) { Dialer: &quicDialerQUICGo{ QUICListener: &quicListenerStdlib{}, }, - Resolver: NewResolver(&ResolverConfig{Logger: log.Log}), + Resolver: NewResolverSystem(log.Log), } txp := NewHTTP3Transport(d, &tls.Config{}) client := &http.Client{Transport: txp} diff --git a/internal/netxlite/http_test.go b/internal/netxlite/http_test.go index dae6373..eca16c4 100644 --- a/internal/netxlite/http_test.go +++ b/internal/netxlite/http_test.go @@ -112,7 +112,7 @@ func TestHTTPTransportLoggerCloseIdleConnections(t *testing.T) { func TestHTTPTransportWorks(t *testing.T) { d := &dialerResolver{ Dialer: defaultDialer, - Resolver: NewResolver(&ResolverConfig{Logger: log.Log}), + Resolver: NewResolverSystem(log.Log), } th := &tlsHandshakerConfigurable{} txp := NewHTTPTransport(d, &tls.Config{}, th) @@ -134,7 +134,7 @@ func TestHTTPTransportWithFailingDialer(t *testing.T) { return nil, expected }, }, - Resolver: NewResolver(&ResolverConfig{Logger: log.Log}), + Resolver: NewResolverSystem(log.Log), } th := &tlsHandshakerConfigurable{} txp := NewHTTPTransport(d, &tls.Config{}, th) diff --git a/internal/netxlite/quic_test.go b/internal/netxlite/quic_test.go index 1e2278c..5db4a02 100644 --- a/internal/netxlite/quic_test.go +++ b/internal/netxlite/quic_test.go @@ -215,7 +215,7 @@ func TestQUICDialerQUICGoTLSDefaultsForDoQ(t *testing.T) { func TestQUICDialerResolverSuccess(t *testing.T) { tlsConfig := &tls.Config{} dialer := &quicDialerResolver{ - Resolver: NewResolver(&ResolverConfig{Logger: log.Log}), + Resolver: NewResolverSystem(log.Log), Dialer: &quicDialerQUICGo{ QUICListener: &quicListenerStdlib{}, }} @@ -234,7 +234,7 @@ func TestQUICDialerResolverSuccess(t *testing.T) { func TestQUICDialerResolverNoPort(t *testing.T) { tlsConfig := &tls.Config{} dialer := &quicDialerResolver{ - Resolver: NewResolver(&ResolverConfig{Logger: log.Log}), + Resolver: NewResolverSystem(log.Log), Dialer: &quicDialerQUICGo{}} sess, err := dialer.DialContext( context.Background(), "udp", "www.google.com", @@ -288,7 +288,7 @@ func TestQUICDialerResolverInvalidPort(t *testing.T) { // to establish a connection leads to a failure tlsConf := &tls.Config{} dialer := &quicDialerResolver{ - Resolver: NewResolver(&ResolverConfig{Logger: log.Log}), + Resolver: NewResolverSystem(log.Log), Dialer: &quicDialerQUICGo{ QUICListener: &quicListenerStdlib{}, }} @@ -312,7 +312,7 @@ func TestQUICDialerResolverApplyTLSDefaults(t *testing.T) { var gotTLSConfig *tls.Config tlsConfig := &tls.Config{} dialer := &quicDialerResolver{ - Resolver: NewResolver(&ResolverConfig{Logger: log.Log}), + Resolver: NewResolverSystem(log.Log), Dialer: &mocks.QUICContextDialer{ MockDialContext: func(ctx context.Context, network, address string, tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlySession, error) { diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go index a3c8960..fbbabb6 100644 --- a/internal/netxlite/resolver.go +++ b/internal/netxlite/resolver.go @@ -2,6 +2,7 @@ package netxlite import ( "context" + "errors" "net" "time" @@ -23,20 +24,15 @@ type Resolver interface { CloseIdleConnections() } -// ResolverConfig contains config for creating a resolver. -type ResolverConfig struct { - // Logger is the MANDATORY logger to use. - Logger Logger -} - -// NewResolver creates a new resolver. -func NewResolver(config *ResolverConfig) Resolver { +// NewResolverSystem creates a new resolver using system +// facilities for resolving domain names (e.g., getaddrinfo). +func NewResolverSystem(logger Logger) Resolver { return &resolverIDNA{ Resolver: &resolverLogger{ Resolver: &resolverShortCircuitIPAddr{ Resolver: &resolverSystem{}, }, - Logger: config.Logger, + Logger: logger, }, } } @@ -159,3 +155,30 @@ func (r *resolverShortCircuitIPAddr) LookupHost(ctx context.Context, hostname st } return r.Resolver.LookupHost(ctx, hostname) } + +// ErrNoResolver indicates you are using a dialer without a resolver. +var ErrNoResolver = errors.New("no configured resolver") + +// nullResolver is a resolver that is not capable of resolving +// domain names to IP addresses and always returns ErrNoResolver. +type nullResolver struct{} + +// LookupHost implements Resolver.LookupHost. +func (r *nullResolver) LookupHost(ctx context.Context, hostname string) (addrs []string, err error) { + return nil, ErrNoResolver +} + +// Network implements Resolver.Network. +func (r *nullResolver) Network() string { + return "null" +} + +// Address implements Resolver.Address. +func (r *nullResolver) Address() string { + return "" +} + +// CloseIdleConnections implements Resolver.CloseIdleConnections. +func (r *nullResolver) CloseIdleConnections() { + // nothing +} diff --git a/internal/netxlite/resolver_test.go b/internal/netxlite/resolver_test.go index ea9f098..da58322 100644 --- a/internal/netxlite/resolver_test.go +++ b/internal/netxlite/resolver_test.go @@ -180,9 +180,7 @@ func TestResolverIDNAWithInvalidPunycode(t *testing.T) { } func TestNewResolverTypeChain(t *testing.T) { - r := NewResolver(&ResolverConfig{ - Logger: log.Log, - }) + r := NewResolverSystem(log.Log) ridna, ok := r.(*resolverIDNA) if !ok { t.Fatal("invalid resolver") @@ -238,3 +236,22 @@ func TestResolverShortCircuitIPAddrWithDomain(t *testing.T) { t.Fatal("invalid result") } } + +func TestNullResolverWorksAsIntended(t *testing.T) { + r := &nullResolver{} + ctx := context.Background() + addrs, err := r.LookupHost(ctx, "dns.google") + if !errors.Is(err, ErrNoResolver) { + t.Fatal("not the error we expected", err) + } + if addrs != nil { + t.Fatal("expected nil addr") + } + if r.Network() != "null" { + t.Fatal("invalid network") + } + if r.Address() != "" { + t.Fatal("invalid address") + } + r.CloseIdleConnections() // should not crash +}