c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(netxlite): add more functions to resolver (#455)

Browse Source 
We would like to refactor the code so that a DoH resolver owns the
connections of its underlying HTTP client.

To do that, we need first to incorporate CloseIdleConnections
into the Resolver model. Then, we need to add the same function
to all netxlite types that wrap a Resolver type.

At the same time, we want the rest of the code for now to continue
with the simpler definition of a Resolver, now called ResolverLegacy.

We will eventually propagate this change to the rest of the tree
and simplify the way in which we manage Resolvers.

To make this possible, we introduce a new factory function that
adapts a ResolverLegacy to become a Resolver.

See https://github.com/ooni/probe/issues/1591.
This commit is contained in:
Simone Basso
2021-09-05 18:03:50 +02:00
committed by GitHub
parent 2e0118d1a6
commit a3654f60b7
22 changed files with 279 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/netxlite/http3_test.go
+3 -2
View File
@@ -2,9 +2,10 @@ package netxlite
import (
"crypto/tls"
"net"
"net/http"
"testing"
"github.com/apex/log"
)
func TestHTTP3TransportWorks(t *testing.T) {
@@ -12,7 +13,7 @@ func TestHTTP3TransportWorks(t *testing.T) {
Dialer: &quicDialerQUICGo{
QUICListener: &quicListenerStdlib{},
},
Resolver: &net.Resolver{},
Resolver: NewResolver(&ResolverConfig{Logger: log.Log}),
}
txp := NewHTTP3Transport(d, &tls.Config{})
client := &http.Client{Transport: txp}
internal/netxlite/http_test.go
+2 -2
View File
@@ -112,7 +112,7 @@ func TestHTTPTransportLoggerCloseIdleConnections(t *testing.T) {
func TestHTTPTransportWorks(t *testing.T) {
d := &dialerResolver{
Dialer: defaultDialer,
Resolver: &net.Resolver{},
Resolver: NewResolver(&ResolverConfig{Logger: log.Log}),
}
th := &tlsHandshakerConfigurable{}
txp := NewHTTPTransport(d, &tls.Config{}, th)
@@ -134,7 +134,7 @@ func TestHTTPTransportWithFailingDialer(t *testing.T) {
return nil, expected
},
},
Resolver: &net.Resolver{},
Resolver: NewResolver(&ResolverConfig{Logger: log.Log}),
}
th := &tlsHandshakerConfigurable{}
txp := NewHTTPTransport(d, &tls.Config{}, th)
internal/netxlite/legacy.go
+63
View File
@@ -1,6 +1,7 @@
package netxlite
import (
"context"
"errors"
"strings"
@@ -59,3 +60,65 @@ type (
TLSHandshakerConfigurable = tlsHandshakerConfigurable
TLSHandshakerLogger = tlsHandshakerLogger
)
// ResolverLegacy performs domain name resolutions.
//
// This definition of Resolver is DEPRECATED. New code should use
// the more complete definition in the new Resolver interface.
//
// Existing code in ooni/probe-cli is still using this definition.
type ResolverLegacy interface {
// LookupHost behaves like net.Resolver.LookupHost.
LookupHost(ctx context.Context, hostname string) (addrs []string, err error)
}
// NewResolverLegacyAdapter adapts a ResolverLegacy to
// become compatible with the Resolver definition.
func NewResolverLegacyAdapter(reso ResolverLegacy) Resolver {
return &ResolverLegacyAdapter{reso}
}
// ResolverLegacyAdapter makes a ResolverLegacy behave like
// it was a Resolver type. If ResolverLegacy is actually also
// a Resolver, this adapter will just forward missing calls,
// otherwise it will implement a sensible default action.
type ResolverLegacyAdapter struct {
ResolverLegacy
}
var _ Resolver = &ResolverLegacyAdapter{}
type resolverLegacyNetworker interface {
Network() string
}
// Network implements Resolver.Network.
func (r *ResolverLegacyAdapter) Network() string {
if rn, ok := r.ResolverLegacy.(resolverLegacyNetworker); ok {
return rn.Network()
}
return "adapter"
}
type resolverLegacyAddresser interface {
Address() string
}
// Address implements Resolver.Address.
func (r *ResolverLegacyAdapter) Address() string {
if ra, ok := r.ResolverLegacy.(resolverLegacyAddresser); ok {
return ra.Address()
}
return ""
}
type resolverLegacyIdleConnectionsCloser interface {
CloseIdleConnections()
}
// CloseIdleConnections implements Resolver.CloseIdleConnections.
func (r *ResolverLegacyAdapter) CloseIdleConnections() {
if ra, ok := r.ResolverLegacy.(resolverLegacyIdleConnectionsCloser); ok {
ra.CloseIdleConnections()
}
}
internal/netxlite/legacy_test.go
+38
View File
@@ -2,9 +2,11 @@ package netxlite
import (
"errors"
"net"
"testing"
"github.com/ooni/probe-cli/v3/internal/errorsx"
"github.com/ooni/probe-cli/v3/internal/netxlite/mocks"
)
func TestReduceErrors(t *testing.T) {
@@ -44,3 +46,39 @@ func TestReduceErrors(t *testing.T) {
}
})
}
func TestResolverLegacyAdapterWithCompatibleType(t *testing.T) {
var called bool
r := NewResolverLegacyAdapter(&mocks.Resolver{
MockNetwork: func() string {
return "network"
},
MockAddress: func() string {
return "address"
},
MockCloseIdleConnections: func() {
called = true
},
})
if r.Network() != "network" {
t.Fatal("invalid Network")
}
if r.Address() != "address" {
t.Fatal("invalid Address")
}
r.CloseIdleConnections()
if !called {
t.Fatal("not called")
}
}
func TestResolverLegacyAdapterDefaults(t *testing.T) {
r := NewResolverLegacyAdapter(&net.Resolver{})
if r.Network() != "adapter" {
t.Fatal("invalid Network")
}
if r.Address() != "" {
t.Fatal("invalid Address")
}
r.CloseIdleConnections() // does not crash
}
internal/netxlite/mocks/resolver.go
+9 -3
View File
@@ -4,9 +4,10 @@ import "context"
// Resolver is a mockable Resolver.
type Resolver struct {
MockLookupHost func(ctx context.Context, domain string) ([]string, error)
MockNetwork func() string
MockAddress func() string
MockLookupHost func(ctx context.Context, domain string) ([]string, error)
MockNetwork func() string
MockAddress func() string
MockCloseIdleConnections func()
}
// LookupHost calls MockLookupHost.
@@ -23,3 +24,8 @@ func (r *Resolver) Address() string {
func (r *Resolver) Network() string {
return r.MockNetwork()
}
// CloseIdleConnections calls MockCloseIdleConnections.
func (r *Resolver) CloseIdleConnections() {
r.MockCloseIdleConnections()
}
internal/netxlite/mocks/resolver_test.go
+13
View File
@@ -44,3 +44,16 @@ func TestResolverAddress(t *testing.T) {
t.Fatal("unexpected address", v)
}
}
func TestResolverCloseIdleConnections(t *testing.T) {
var called bool
r := &Resolver{
MockCloseIdleConnections: func() {
called = true
},
}
r.CloseIdleConnections()
if !called {
t.Fatal("not called")
}
}
internal/netxlite/quic_test.go
+8 -4
View File
@@ -215,7 +215,8 @@ func TestQUICDialerQUICGoTLSDefaultsForDoQ(t *testing.T) {
func TestQUICDialerResolverSuccess(t *testing.T) {
tlsConfig := &tls.Config{}
dialer := &quicDialerResolver{
Resolver: &net.Resolver{}, Dialer: &quicDialerQUICGo{
Resolver: NewResolver(&ResolverConfig{Logger: log.Log}),
Dialer: &quicDialerQUICGo{
QUICListener: &quicListenerStdlib{},
}}
sess, err := dialer.DialContext(
@@ -233,7 +234,8 @@ func TestQUICDialerResolverSuccess(t *testing.T) {
func TestQUICDialerResolverNoPort(t *testing.T) {
tlsConfig := &tls.Config{}
dialer := &quicDialerResolver{
Resolver: new(net.Resolver), Dialer: &quicDialerQUICGo{}}
Resolver: NewResolver(&ResolverConfig{Logger: log.Log}),
Dialer: &quicDialerQUICGo{}}
sess, err := dialer.DialContext(
context.Background(), "udp", "www.google.com",
tlsConfig, &quic.Config{})
@@ -286,7 +288,8 @@ func TestQUICDialerResolverInvalidPort(t *testing.T) {
// to establish a connection leads to a failure
tlsConf := &tls.Config{}
dialer := &quicDialerResolver{
Resolver: new(net.Resolver), Dialer: &quicDialerQUICGo{
Resolver: NewResolver(&ResolverConfig{Logger: log.Log}),
Dialer: &quicDialerQUICGo{
QUICListener: &quicListenerStdlib{},
}}
sess, err := dialer.DialContext(
@@ -309,7 +312,8 @@ func TestQUICDialerResolverApplyTLSDefaults(t *testing.T) {
var gotTLSConfig *tls.Config
tlsConfig := &tls.Config{}
dialer := &quicDialerResolver{
Resolver: new(net.Resolver), Dialer: &mocks.QUICContextDialer{
Resolver: NewResolver(&ResolverConfig{Logger: log.Log}),
Dialer: &mocks.QUICContextDialer{
MockDialContext: func(ctx context.Context, network, address string,
tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlySession, error) {
gotTLSConfig = tlsConfig
internal/netxlite/resolver.go
+30 -40
View File
@@ -12,6 +12,31 @@ import (
type Resolver interface {
// LookupHost behaves like net.Resolver.LookupHost.
LookupHost(ctx context.Context, hostname string) (addrs []string, err error)
// Network returns the resolver type (e.g., system, dot, doh).
Network() string
// Address returns the resolver address (e.g., 8.8.8.8:53).
Address() string
// CloseIdleConnections closes idle connections, if any.
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 {
return &resolverIDNA{
Resolver: &resolverLogger{
Resolver: &resolverSystem{},
Logger: config.Logger,
},
}
}
// resolverSystem is the system resolver.
@@ -34,6 +59,11 @@ func (r *resolverSystem) Address() string {
return ""
}
// CloseIdleConnections implements Resolver.CloseIdleConnections.
func (r *resolverSystem) CloseIdleConnections() {
// nothing
}
// DefaultResolver is the resolver we use by default.
var DefaultResolver = &resolverSystem{}
@@ -59,30 +89,6 @@ func (r *resolverLogger) LookupHost(ctx context.Context, hostname string) ([]str
return addrs, nil
}
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 ""
}
// resolverIDNA supports resolving Internationalized Domain Names.
//
// See RFC3492 for more information.
@@ -98,19 +104,3 @@ func (r *resolverIDNA) LookupHost(ctx context.Context, hostname string) ([]strin
}
return r.Resolver.LookupHost(ctx, host)
}
// Network implements Resolver.Network.
func (r *resolverIDNA) Network() string {
if rn, ok := r.Resolver.(resolverNetworker); ok {
return rn.Network()
}
return "idna"
}
// Address implements Resolver.Address.
func (r *resolverIDNA) Address() string {
if ra, ok := r.Resolver.(resolverAddresser); ok {
return ra.Address()
}
return ""
}
internal/netxlite/resolver_test.go
+15 -38
View File
@@ -3,7 +3,6 @@ package netxlite
import (
"context"
"errors"
"net"
"strings"
"testing"
@@ -71,26 +70,6 @@ func TestResolverLoggerWithFailure(t *testing.T) {
}
}
func TestResolverLoggerChildNetworkAddress(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")
}
}
func TestResolverLoggerNoChildNetworkAddress(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")
}
}
func TestResolverIDNAWorksAsIntended(t *testing.T) {
expectedIPs := []string{"77.88.55.66"}
r := &resolverIDNA{
@@ -130,24 +109,22 @@ func TestResolverIDNAWithInvalidPunycode(t *testing.T) {
}
}
func TestResolverIDNAChildNetworkAddress(t *testing.T) {
r := &resolverIDNA{
Resolver: DefaultResolver,
func TestNewResolverTypeChain(t *testing.T) {
r := NewResolver(&ResolverConfig{
Logger: log.Log,
})
ridna, ok := r.(*resolverIDNA)
if !ok {
t.Fatal("invalid resolver")
}
if v := r.Network(); v != "system" {
t.Fatal("invalid network", v)
rl, ok := ridna.Resolver.(*resolverLogger)
if !ok {
t.Fatal("invalid resolver")
}
if v := r.Address(); v != "" {
t.Fatal("invalid address", v)
}
}
func TestResolverIDNANoChildNetworkAddress(t *testing.T) {
r := &resolverIDNA{}
if v := r.Network(); v != "idna" {
t.Fatal("invalid network", v)
}
if v := r.Address(); v != "" {
t.Fatal("invalid address", v)
if rl.Logger != log.Log {
t.Fatal("invalid logger")
}
if _, ok := rl.Resolver.(*resolverSystem); !ok {
t.Fatal("invalid resolver")
}
}