refactor: continue to simplify engine/netx (#769)

The objective of this diff is to simplify the code inside engine/netx
while moving more bits of code inside netxlite.

See https://github.com/ooni/probe/issues/2121

internal/engine/experiment.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/ooni/probe-cli/v3/internal/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/engine/geolocate"
-	"github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport"
 	"github.com/ooni/probe-cli/v3/internal/engine/probeservices"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/version"
@@ -286,7 +285,7 @@ func (e *Experiment) OpenReportContext(ctx context.Context) error {
 	}
 	// use custom client to have proper byte accounting
 	httpClient := &http.Client{
-		Transport: &httptransport.ByteCountingTransport{
+		Transport: &bytecounter.HTTPTransport{
 			HTTPTransport: e.session.httpDefaultTransport, // proxy is OK
 			Counter:       e.byteCounter,
 		},

internal/engine/netx/dialer/bytecounter.go

@@ -1,5 +0,0 @@
-package dialer
-
-import "github.com/ooni/probe-cli/v3/internal/bytecounter"
-
-type byteCounterDialer = bytecounter.ContextAwareDialer

internal/engine/netx/dialer/dialer.go

@@ -1,8 +1,12 @@
+// Package dialer allows you to create a net.Dialer-compatible
+// DialContext-enabled dialer with error wrapping, optional logging,
+// optional network-events saving, and optional proxying.
 package dialer
 
 import (
 	"net/url"
 
+	"github.com/ooni/probe-cli/v3/internal/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/trace"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
@@ -63,9 +67,9 @@ func New(config *Config, resolver model.Resolver) model.Dialer {
 		Resolver: resolver,
 		Dialer:   d,
 	}
-	d = &proxyDialer{ProxyURL: config.ProxyURL, Dialer: d}
+	d = &netxlite.MaybeProxyDialer{ProxyURL: config.ProxyURL, Dialer: d}
 	if config.ContextByteCounting {
-		d = &byteCounterDialer{Dialer: d}
+		d = &bytecounter.ContextAwareDialer{Dialer: d}
 	}
 	return d
 }

internal/engine/netx/dialer/dialer_test.go

@@ -1,10 +1,12 @@
 package dialer
 
 import (
+	"net/http"
 	"net/url"
 	"testing"
 
 	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/trace"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
@@ -18,11 +20,11 @@ func TestNewCreatesTheExpectedChain(t *testing.T) {
 		ProxyURL:            &url.URL{},
 		ReadWriteSaver:      saver,
 	}, netxlite.DefaultResolver)
-	bcd, ok := dlr.(*byteCounterDialer)
+	bcd, ok := dlr.(*bytecounter.ContextAwareDialer)
 	if !ok {
 		t.Fatal("not a byteCounterDialer")
 	}
-	pd, ok := bcd.Dialer.(*proxyDialer)
+	pd, ok := bcd.Dialer.(*netxlite.MaybeProxyDialer)
 	if !ok {
 		t.Fatal("not a proxyDialer")
 	}
@@ -51,3 +53,18 @@ func TestNewCreatesTheExpectedChain(t *testing.T) {
 		t.Fatal("not a DialerSystem")
 	}
 }
+
+func TestDialerNewSuccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+	log.SetLevel(log.DebugLevel)
+	d := New(&Config{Logger: log.Log}, netxlite.DefaultResolver)
+	txp := &http.Transport{DialContext: d.DialContext}
+	client := &http.Client{Transport: txp}
+	resp, err := client.Get("http://www.google.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+}

internal/engine/netx/dialer/doc.go

@@ -1,4 +0,0 @@
-// Package dialer allows you to create a net.Dialer-compatible
-// DialContext-enabled dialer with error wrapping, optional logging,
-// optional network-events saving, and optional proxying.
-package dialer

internal/engine/netx/dialer/example_test.go

@@ -1,30 +0,0 @@
-package dialer_test
-
-import (
-	"context"
-
-	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/engine/netx/dialer"
-	"github.com/ooni/probe-cli/v3/internal/engine/netx/trace"
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-func Example() {
-	saver := &trace.Saver{}
-
-	dlr := dialer.New(&dialer.Config{
-		DialSaver:      saver,
-		Logger:         log.Log,
-		ReadWriteSaver: saver,
-	}, netxlite.DefaultResolver)
-
-	ctx := context.Background()
-	conn, err := dlr.DialContext(ctx, "tcp", "8.8.8.8:53")
-	if err != nil {
-		log.WithError(err).Fatal("DialContext failed")
-	}
-
-	// ... use the connection ...
-
-	conn.Close()
-}

internal/engine/netx/dialer/integration_test.go

@@ -1,25 +0,0 @@
-package dialer_test
-
-import (
-	"net/http"
-	"testing"
-
-	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/engine/netx/dialer"
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-func TestDialerNewSuccess(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skip test in short mode")
-	}
-	log.SetLevel(log.DebugLevel)
-	d := dialer.New(&dialer.Config{Logger: log.Log}, netxlite.DefaultResolver)
-	txp := &http.Transport{DialContext: d.DialContext}
-	client := &http.Client{Transport: txp}
-	resp, err := client.Get("http://www.google.com")
-	if err != nil {
-		t.Fatal(err)
-	}
-	resp.Body.Close()
-}

internal/engine/netx/dialer/proxy.go

@@ -1,5 +0,0 @@
-package dialer
-
-import "github.com/ooni/probe-cli/v3/internal/netxlite"
-
-type proxyDialer = netxlite.MaybeProxyDialer

internal/engine/netx/httptransport/bytecounter.go

@@ -1,5 +0,0 @@
-package httptransport
-
-import "github.com/ooni/probe-cli/v3/internal/bytecounter"
-
-type ByteCountingTransport = bytecounter.HTTPTransport

internal/engine/netx/httptransport/fake_test.go

@@ -1,62 +0,0 @@
-package httptransport
-
-import (
-	"context"
-	"net"
-	"net/http"
-	"time"
-
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-type FakeDialer struct {
-	Conn net.Conn
-	Err  error
-}
-
-func (d FakeDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) {
-	time.Sleep(10 * time.Microsecond)
-	return d.Conn, d.Err
-}
-
-type FakeTransport struct {
-	Name string
-	Err  error
-	Func func(*http.Request) (*http.Response, error)
-	Resp *http.Response
-}
-
-func (txp FakeTransport) Network() string {
-	return txp.Name
-}
-
-func (txp FakeTransport) RoundTrip(req *http.Request) (*http.Response, error) {
-	time.Sleep(10 * time.Microsecond)
-	if txp.Func != nil {
-		return txp.Func(req)
-	}
-	if req.Body != nil {
-		netxlite.ReadAllContext(req.Context(), req.Body)
-		req.Body.Close()
-	}
-	if txp.Err != nil {
-		return nil, txp.Err
-	}
-	txp.Resp.Request = req // non thread safe but it doesn't matter
-	return txp.Resp, nil
-}
-
-func (txp FakeTransport) CloseIdleConnections() {}
-
-type FakeBody struct {
-	Err error
-}
-
-func (fb FakeBody) Read(p []byte) (int, error) {
-	time.Sleep(10 * time.Microsecond)
-	return 0, fb.Err
-}
-
-func (fb FakeBody) Close() error {
-	return nil
-}

internal/engine/netx/httptransport/http3transport.go

@@ -1,24 +0,0 @@
-package httptransport
-
-import (
-	"github.com/ooni/probe-cli/v3/internal/model"
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-// NewHTTP3Transport creates a new HTTP3Transport instance.
-//
-// Deprecation warning
-//
-// New code should use netxlite.NewHTTP3Transport instead.
-func NewHTTP3Transport(config Config) model.HTTPTransport {
-	// Rationale for using NoLogger here: previously this code did
-	// not use a logger as well, so it's fine to keep it as is.
-	return netxlite.NewHTTP3Transport(&NoLogger{},
-		config.QUICDialer, config.TLSConfig)
-}
-
-type NoLogger struct{}
-
-func (*NoLogger) Debug(message string) {}
-
-func (*NoLogger) Debugf(format string, v ...interface{}) {}

internal/engine/netx/httptransport/http3transport_test.go

@@ -1,40 +0,0 @@
-package httptransport_test
-
-import (
-	"context"
-	"crypto/tls"
-	"errors"
-	"net/http"
-	"testing"
-
-	"github.com/lucas-clemente/quic-go"
-	"github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport"
-	"github.com/ooni/probe-cli/v3/internal/model/mocks"
-)
-
-func TestNewHTTP3Transport(t *testing.T) {
-	// make sure we can create a working transport using this factory.
-	expected := errors.New("mocked error")
-	txp := httptransport.NewHTTP3Transport(httptransport.Config{
-		QUICDialer: &mocks.QUICDialer{
-			MockDialContext: func(ctx context.Context, network, address string,
-				tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) {
-				return nil, expected
-			},
-			MockCloseIdleConnections: func() {
-				// nothing
-			},
-		},
-	})
-	req, err := http.NewRequest("GET", "https://google.com", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	resp, err := txp.RoundTrip(req)
-	if !errors.Is(err, expected) {
-		t.Fatal("unexpected err", err)
-	}
-	if resp != nil {
-		t.Fatal("expected nil resp")
-	}
-}

internal/engine/netx/httptransport/httptransport.go

@@ -5,6 +5,7 @@ import (
 	"crypto/tls"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
 
 // Config contains the configuration required for constructing an HTTP transport
@@ -14,3 +15,25 @@ type Config struct {
 	TLSDialer  model.TLSDialer
 	TLSConfig  *tls.Config
 }
+
+// NewHTTP3Transport creates a new HTTP3Transport instance.
+//
+// Deprecation warning
+//
+// New code should use netxlite.NewHTTP3Transport instead.
+func NewHTTP3Transport(config Config) model.HTTPTransport {
+	// Rationale for using NoLogger here: previously this code did
+	// not use a logger as well, so it's fine to keep it as is.
+	return netxlite.NewHTTP3Transport(model.DiscardLogger,
+		config.QUICDialer, config.TLSConfig)
+}
+
+// NewSystemTransport creates a new "system" HTTP transport. That is a transport
+// using the Go standard library with custom dialer and TLS dialer.
+//
+// Deprecation warning
+//
+// New code should use netxlite.NewHTTPTransport instead.
+func NewSystemTransport(config Config) model.HTTPTransport {
+	return netxlite.NewOOHTTPBaseTransport(config.Dialer, config.TLSDialer)
+}

internal/engine/netx/httptransport/saver_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"net"
 	"net/http"
 	"net/url"
 	"strings"
@@ -76,7 +77,7 @@ func TestSaverMetadataFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	saver := &trace.Saver{}
 	txp := httptransport.SaverMetadataHTTPTransport{
-		HTTPTransport: httptransport.FakeTransport{
+		HTTPTransport: FakeTransport{
 			Err: expected,
 		},
 		Saver: saver,
@@ -161,7 +162,7 @@ func TestSaverTransactionFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	saver := &trace.Saver{}
 	txp := httptransport.SaverTransactionHTTPTransport{
-		HTTPTransport: httptransport.FakeTransport{
+		HTTPTransport: FakeTransport{
 			Err: expected,
 		},
 		Saver: saver,
@@ -201,7 +202,7 @@ func TestSaverTransactionFailure(t *testing.T) {
 func TestSaverBodySuccess(t *testing.T) {
 	saver := new(trace.Saver)
 	txp := httptransport.SaverBodyHTTPTransport{
-		HTTPTransport: httptransport.FakeTransport{
+		HTTPTransport: FakeTransport{
 			Func: func(req *http.Request) (*http.Response, error) {
 				data, err := netxlite.ReadAllContext(context.Background(), req.Body)
 				if err != nil {
@@ -272,7 +273,7 @@ func TestSaverBodySuccess(t *testing.T) {
 func TestSaverBodyRequestReadError(t *testing.T) {
 	saver := new(trace.Saver)
 	txp := httptransport.SaverBodyHTTPTransport{
-		HTTPTransport: httptransport.FakeTransport{
+		HTTPTransport: FakeTransport{
 			Func: func(req *http.Request) (*http.Response, error) {
 				panic("should not be called")
 			},
@@ -281,7 +282,7 @@ func TestSaverBodyRequestReadError(t *testing.T) {
 		Saver:        saver,
 	}
 	expected := errors.New("mocked error")
-	body := httptransport.FakeBody{Err: expected}
+	body := FakeBody{Err: expected}
 	req, err := http.NewRequest("POST", "http://x.org/y", body)
 	if err != nil {
 		t.Fatal(err)
@@ -303,7 +304,7 @@ func TestSaverBodyRoundTripError(t *testing.T) {
 	saver := new(trace.Saver)
 	expected := errors.New("mocked error")
 	txp := httptransport.SaverBodyHTTPTransport{
-		HTTPTransport: httptransport.FakeTransport{
+		HTTPTransport: FakeTransport{
 			Err: expected,
 		},
 		SnapshotSize: 4,
@@ -343,11 +344,11 @@ func TestSaverBodyResponseReadError(t *testing.T) {
 	saver := new(trace.Saver)
 	expected := errors.New("mocked error")
 	txp := httptransport.SaverBodyHTTPTransport{
-		HTTPTransport: httptransport.FakeTransport{
+		HTTPTransport: FakeTransport{
 			Func: func(req *http.Request) (*http.Response, error) {
 				return &http.Response{
 					StatusCode: 200,
-					Body: httptransport.FakeBody{
+					Body: FakeBody{
 						Err: expected,
 					},
 				}, nil
@@ -417,3 +418,55 @@ func TestCloneHeaders(t *testing.T) {
 		}
 	})
 }
+
+type FakeDialer struct {
+	Conn net.Conn
+	Err  error
+}
+
+func (d FakeDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) {
+	time.Sleep(10 * time.Microsecond)
+	return d.Conn, d.Err
+}
+
+type FakeTransport struct {
+	Name string
+	Err  error
+	Func func(*http.Request) (*http.Response, error)
+	Resp *http.Response
+}
+
+func (txp FakeTransport) Network() string {
+	return txp.Name
+}
+
+func (txp FakeTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	time.Sleep(10 * time.Microsecond)
+	if txp.Func != nil {
+		return txp.Func(req)
+	}
+	if req.Body != nil {
+		netxlite.ReadAllContext(req.Context(), req.Body)
+		req.Body.Close()
+	}
+	if txp.Err != nil {
+		return nil, txp.Err
+	}
+	txp.Resp.Request = req // non thread safe but it doesn't matter
+	return txp.Resp, nil
+}
+
+func (txp FakeTransport) CloseIdleConnections() {}
+
+type FakeBody struct {
+	Err error
+}
+
+func (fb FakeBody) Read(p []byte) (int, error) {
+	time.Sleep(10 * time.Microsecond)
+	return 0, fb.Err
+}
+
+func (fb FakeBody) Close() error {
+	return nil
+}

internal/engine/netx/httptransport/system.go

@@ -1,38 +0,0 @@
-package httptransport
-
-import (
-	oohttp "github.com/ooni/oohttp"
-	"github.com/ooni/probe-cli/v3/internal/model"
-)
-
-// NewSystemTransport creates a new "system" HTTP transport. That is a transport
-// using the Go standard library with custom dialer and TLS dialer.
-//
-// Deprecation warning
-//
-// New code should use netxlite.NewHTTPTransport instead.
-func NewSystemTransport(config Config) model.HTTPTransport {
-	txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone()
-	txp.DialContext = config.Dialer.DialContext
-	txp.DialTLSContext = config.TLSDialer.DialTLSContext
-	// Better for Cloudflare DNS and also better because we have less
-	// noisy events and we can better understand what happened.
-	txp.MaxConnsPerHost = 1
-	// The following (1) reduces the number of headers that Go will
-	// automatically send for us and (2) ensures that we always receive
-	// back the true headers, such as Content-Length. This change is
-	// functional to OONI's goal of observing the network.
-	txp.DisableCompression = true
-	return &SystemTransportWrapper{&oohttp.StdlibTransport{Transport: txp}}
-}
-
-// SystemTransportWrapper adapts *http.Transport to have the .Network method
-type SystemTransportWrapper struct {
-	*oohttp.StdlibTransport
-}
-
-func (txp *SystemTransportWrapper) Network() string {
-	return "tcp"
-}
-
-var _ model.HTTPTransport = &SystemTransportWrapper{}

internal/engine/netx/netx.go

@@ -97,7 +97,7 @@ func NewResolver(config Config) model.Resolver {
 		r = cache
 	}
 	if config.BogonIsError {
-		r = resolver.BogonResolver{Resolver: r}
+		r = &netxlite.BogonResolver{Resolver: r}
 	}
 	r = &netxlite.ErrorWrapperResolver{Resolver: r}
 	if config.Logger != nil {
@@ -202,7 +202,7 @@ func NewHTTPTransport(config Config) model.HTTPTransport {
 		TLSConfig: config.TLSConfig})
 
 	if config.ByteCounter != nil {
-		txp = &httptransport.ByteCountingTransport{
+		txp = &bytecounter.HTTPTransport{
 			Counter: config.ByteCounter, HTTPTransport: txp}
 	}
 	if config.Logger != nil {

internal/engine/netx/netx_test.go

@@ -42,7 +42,7 @@ func TestNewResolverVanilla(t *testing.T) {
 
 func TestNewResolverSpecificResolver(t *testing.T) {
 	r := netx.NewResolver(netx.Config{
-		BaseResolver: resolver.BogonResolver{
+		BaseResolver: &netxlite.BogonResolver{
 			// not initialized because it doesn't matter in this context
 		},
 	})
@@ -58,7 +58,7 @@ func TestNewResolverSpecificResolver(t *testing.T) {
 	if !ok {
 		t.Fatal("not the resolver we expected")
 	}
-	_, ok = ar.Resolver.(resolver.BogonResolver)
+	_, ok = ar.Resolver.(*netxlite.BogonResolver)
 	if !ok {
 		t.Fatal("not the resolver we expected")
 	}
@@ -76,7 +76,7 @@ func TestNewResolverWithBogonFilter(t *testing.T) {
 	if !ok {
 		t.Fatal("not the resolver we expected")
 	}
-	br, ok := ewr.Resolver.(resolver.BogonResolver)
+	br, ok := ewr.Resolver.(*netxlite.BogonResolver)
 	if !ok {
 		t.Fatal("not the resolver we expected")
 	}
@@ -420,7 +420,7 @@ func TestNewTLSDialerWithNoTLSVerifyAndNoConfig(t *testing.T) {
 
 func TestNewVanilla(t *testing.T) {
 	txp := netx.NewHTTPTransport(netx.Config{})
-	if _, ok := txp.(*httptransport.SystemTransportWrapper); !ok {
+	if _, ok := txp.(*netxlite.HTTPTransportWrapper); !ok {
 		t.Fatal("not the transport we expected")
 	}
 }
@@ -473,14 +473,14 @@ func TestNewWithByteCounter(t *testing.T) {
 	txp := netx.NewHTTPTransport(netx.Config{
 		ByteCounter: counter,
 	})
-	bctxp, ok := txp.(*httptransport.ByteCountingTransport)
+	bctxp, ok := txp.(*bytecounter.HTTPTransport)
 	if !ok {
 		t.Fatal("not the transport we expected")
 	}
 	if bctxp.Counter != counter {
 		t.Fatal("not the byte counter we expected")
 	}
-	if _, ok := bctxp.HTTPTransport.(*httptransport.SystemTransportWrapper); !ok {
+	if _, ok := bctxp.HTTPTransport.(*netxlite.HTTPTransportWrapper); !ok {
 		t.Fatal("not the transport we expected")
 	}
 }
@@ -496,7 +496,7 @@ func TestNewWithLogger(t *testing.T) {
 	if ltxp.Logger != log.Log {
 		t.Fatal("not the logger we expected")
 	}
-	if _, ok := ltxp.HTTPTransport.(*httptransport.SystemTransportWrapper); !ok {
+	if _, ok := ltxp.HTTPTransport.(*netxlite.HTTPTransportWrapper); !ok {
 		t.Fatal("not the transport we expected")
 	}
 }
@@ -527,7 +527,7 @@ func TestNewWithSaver(t *testing.T) {
 	if smtxp.Saver != saver {
 		t.Fatal("not the logger we expected")
 	}
-	if _, ok := smtxp.HTTPTransport.(*httptransport.SystemTransportWrapper); !ok {
+	if _, ok := smtxp.HTTPTransport.(*netxlite.HTTPTransportWrapper); !ok {
 		t.Fatal("not the transport we expected")
 	}
 }

internal/engine/netx/quicdialer/connectionstate.go

@@ -1,12 +0,0 @@
-package quicdialer
-
-import (
-	"crypto/tls"
-
-	"github.com/lucas-clemente/quic-go"
-)
-
-// connectionState returns the ConnectionState of a QUIC Session.
-func connectionState(sess quic.EarlyConnection) tls.ConnectionState {
-	return sess.ConnectionState().TLS.ConnectionState
-}

internal/engine/netx/quicdialer/saver.go

@@ -61,3 +61,8 @@ func (h HandshakeSaver) DialContext(ctx context.Context, network string,
 	})
 	return sess, nil
 }
+
+// connectionState returns the ConnectionState of a QUIC Session.
+func connectionState(sess quic.EarlyConnection) tls.ConnectionState {
+	return sess.ConnectionState().TLS.ConnectionState
+}

internal/engine/netx/resolver/bogon.go

@@ -1,32 +0,0 @@
-package resolver
-
-import (
-	"context"
-
-	"github.com/ooni/probe-cli/v3/internal/model"
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-// BogonResolver is a bogon aware resolver. When a bogon is encountered in
-// a reply, this resolver will return an error.
-//
-// Deprecation warning
-//
-// This resolver is deprecated. The right thing to do would be to check
-// for bogons right after a domain name resolution in the nettest.
-type BogonResolver struct {
-	model.Resolver
-}
-
-// LookupHost implements Resolver.LookupHost
-func (r BogonResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) {
-	addrs, err := r.Resolver.LookupHost(ctx, hostname)
-	for _, addr := range addrs {
-		if netxlite.IsBogon(addr) {
-			return nil, netxlite.ErrDNSBogon
-		}
-	}
-	return addrs, err
-}
-
-var _ model.Resolver = BogonResolver{}

internal/engine/netx/resolver/bogon_test.go

@@ -1,37 +0,0 @@
-package resolver_test
-
-import (
-	"context"
-	"errors"
-	"testing"
-
-	"github.com/ooni/probe-cli/v3/internal/engine/netx/resolver"
-	"github.com/ooni/probe-cli/v3/internal/netxlite"
-)
-
-func TestBogonAwareResolverWithBogon(t *testing.T) {
-	r := resolver.BogonResolver{
-		Resolver: resolver.NewFakeResolverWithResult([]string{"127.0.0.1"}),
-	}
-	addrs, err := r.LookupHost(context.Background(), "dns.google.com")
-	if !errors.Is(err, netxlite.ErrDNSBogon) {
-		t.Fatal("not the error we expected")
-	}
-	if len(addrs) > 0 {
-		t.Fatal("expected to see nil here")
-	}
-}
-
-func TestBogonAwareResolverWithoutBogon(t *testing.T) {
-	orig := []string{"8.8.8.8"}
-	r := resolver.BogonResolver{
-		Resolver: resolver.NewFakeResolverWithResult(orig),
-	}
-	addrs, err := r.LookupHost(context.Background(), "dns.google.com")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(addrs) != len(orig) || addrs[0] != orig[0] {
-		t.Fatal("not the error we expected")
-	}
-}

internal/netxlite/bogon.go

@@ -7,11 +7,62 @@ package netxlite
 //
 
 import (
+	"context"
 	"net"
 
+	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
+// BogonResolver is a bogon aware resolver. When a bogon is encountered in
+// a reply, this resolver will return ErrDNSBogon.
+type BogonResolver struct {
+	Resolver model.Resolver
+}
+
+var _ model.Resolver = &BogonResolver{}
+
+// LookupHost implements Resolver.LookupHost
+func (r *BogonResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) {
+	addrs, err := r.Resolver.LookupHost(ctx, hostname)
+	if err != nil {
+		return nil, err
+	}
+	for _, addr := range addrs {
+		if IsBogon(addr) {
+			return nil, ErrDNSBogon
+		}
+	}
+	return addrs, nil
+}
+
+// LookupHTTPS implements Resolver.LookupHTTPS
+func (r *BogonResolver) LookupHTTPS(ctx context.Context, hostname string) (*model.HTTPSSvc, error) {
+	// TODO(bassosimone): decide whether we want to implement this method or not
+	return nil, ErrNoDNSTransport
+}
+
+// LookupNS implements Resolver.LookupNS
+func (r *BogonResolver) LookupNS(ctx context.Context, hostname string) ([]*net.NS, error) {
+	// TODO(bassosimone): decide whether we want to implement this method or not
+	return nil, ErrNoDNSTransport
+}
+
+// Network implements Resolver.Network
+func (r *BogonResolver) Network() string {
+	return r.Resolver.Network()
+}
+
+// Address implements Resolver.Address
+func (r *BogonResolver) Address() string {
+	return r.Resolver.Address()
+}
+
+// CloseIdleConnections implements Resolver.CloseIdleConnections
+func (r *BogonResolver) CloseIdleConnections() {
+	r.Resolver.CloseIdleConnections()
+}
+
 // IsBogon returns whether an IP address is bogon. Passing to this
 // function a non-IP address causes it to return true.
 func IsBogon(address string) bool {

internal/netxlite/bogon_test.go

@@ -1,6 +1,140 @@
 package netxlite
 
-import "testing"
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+)
+
+func TestBogonResolver(t *testing.T) {
+	t.Run("LookupHost", func(t *testing.T) {
+		t.Run("with failure", func(t *testing.T) {
+			expected := errors.New("mocked")
+			reso := &BogonResolver{
+				Resolver: &mocks.Resolver{
+					MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+						return nil, expected
+					},
+				},
+			}
+			ctx := context.Background()
+			addrs, err := reso.LookupHost(ctx, "dns.google")
+			if !errors.Is(err, expected) {
+				t.Fatal("unexpected err", err)
+			}
+			if len(addrs) > 0 {
+				t.Fatal("expected no addrs")
+			}
+		})
+
+		t.Run("with success and no bogon", func(t *testing.T) {
+			expected := []string{"8.8.8.8", "149.112.112.112"}
+			reso := &BogonResolver{
+				Resolver: &mocks.Resolver{
+					MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+						return expected, nil
+					},
+				},
+			}
+			ctx := context.Background()
+			addrs, err := reso.LookupHost(ctx, "dns.google")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if diff := cmp.Diff(expected, addrs); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+
+		t.Run("with success and bogon", func(t *testing.T) {
+			reso := &BogonResolver{
+				Resolver: &mocks.Resolver{
+					MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+						return []string{"8.8.8.8", "10.34.34.35", "149.112.112.112"}, nil
+					},
+				},
+			}
+			ctx := context.Background()
+			addrs, err := reso.LookupHost(ctx, "dns.google")
+			if !errors.Is(err, ErrDNSBogon) {
+				t.Fatal("unexpected err", err)
+			}
+			if len(addrs) > 0 {
+				t.Fatal("expected no addrs")
+			}
+		})
+	})
+
+	t.Run("LookupHTTPS", func(t *testing.T) {
+		ctx := context.Background()
+		reso := &BogonResolver{}
+		https, err := reso.LookupHTTPS(ctx, "dns.google")
+		if !errors.Is(err, ErrNoDNSTransport) {
+			t.Fatal("unexpected err", err)
+		}
+		if https != nil {
+			t.Fatal("expected nil https here")
+		}
+	})
+
+	t.Run("LookupNS", func(t *testing.T) {
+		ctx := context.Background()
+		reso := &BogonResolver{}
+		ns, err := reso.LookupNS(ctx, "dns.google")
+		if !errors.Is(err, ErrNoDNSTransport) {
+			t.Fatal("unexpected err", err)
+		}
+		if len(ns) > 0 {
+			t.Fatal("expected empty ns here")
+		}
+	})
+
+	t.Run("Network", func(t *testing.T) {
+		expected := "antani"
+		reso := &BogonResolver{
+			Resolver: &mocks.Resolver{
+				MockNetwork: func() string {
+					return expected
+				},
+			},
+		}
+		if reso.Network() != expected {
+			t.Fatal("unexpected network")
+		}
+	})
+
+	t.Run("Address", func(t *testing.T) {
+		expected := "antani"
+		reso := &BogonResolver{
+			Resolver: &mocks.Resolver{
+				MockAddress: func() string {
+					return expected
+				},
+			},
+		}
+		if reso.Address() != expected {
+			t.Fatal("unexpected address")
+		}
+	})
+
+	t.Run("CloseIdleConnections", func(t *testing.T) {
+		var called bool
+		reso := &BogonResolver{
+			Resolver: &mocks.Resolver{
+				MockCloseIdleConnections: func() {
+					called = true
+				},
+			},
+		}
+		reso.CloseIdleConnections()
+		if !called {
+			t.Fatal("not called")
+		}
+	})
+}
 
 func TestIsBogon(t *testing.T) {
 	if IsBogon("antani") != true {

internal/netxlite/legacy.go

@@ -20,6 +20,7 @@ var (
 type (
 	DialerResolver            = dialerResolver
 	DialerLogger              = dialerLogger
+	HTTPTransportWrapper      = httpTransportConnectionsCloser
 	HTTPTransportLogger       = httpTransportLogger
 	ErrorWrapperDialer        = dialerErrWrapper
 	ErrorWrapperQUICListener  = quicListenerErrWrapper