cleanup(netxlite): remove first-order implicit-Netx wrappers (#1465)

This diff removes the first-order implicit-Netx wrappers. We define first-order implicit-Netx wrappers the top-level functions that have the same name of netxlite.Netx methods, allocate an empty Netx, and call the corresponding method. The reason why we're doing this now is that it has been relatively hard to implement #1464 because of the ambiguity between those first-order wrappers and the methods. Getting this wrong means that QA tests would behave in a funny way. The overall goal here is to refactor `oohelperd` to only depend on `netxlite.Netx` such that we can remove the code duplication between how we instantiate `oohelperd.Handler` in `oohelperd` and how we instantiate it inside `netemx`. In turn, by doing this, we would ensure we have the same `oohelperd` behavior for QA and production. In turn, with this guarantee, we can write QA tests that ensure we're correctly dealing with 127.0.0.1. The reference issue is ooni/probe#1517.
ooni · Jan 24, 2024 · 70861a6 · 70861a6
1 parent fb87190
commit 70861a6
Show file tree

Hide file tree

Showing 99 changed files with 395 additions and 317 deletions.
diff --git a/internal/cmd/apitool/main.go b/internal/cmd/apitool/main.go
@@ -28,7 +28,8 @@ import (
 func newclient() probeservices.Client {
 	// TODO(https://github.com/ooni/probe/issues/2534): NewHTTPTransportStdlib has QUIRKS but we
 	// don't actually care about those QUIRKS in this context
-	txp := netxlite.NewHTTPTransportStdlib(log.Log)
+	netx := &netxlite.Netx{}
+	txp := netx.NewHTTPTransportStdlib(log.Log)
 	ua := fmt.Sprintf("apitool/%s ooniprobe-engine/%s", version.Version, version.Version)
 	return probeservices.Client{
 		APIClientTemplate: httpx.APIClientTemplate{

diff --git a/internal/cmd/gardener/internal/dnsreport/dnsreport.go b/internal/cmd/gardener/internal/dnsreport/dnsreport.go
@@ -253,7 +253,8 @@ func (s *Subcommand) dnsLookupHost(domain string) ([]string, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	dnsResolver := netxlite.NewParallelDNSOverHTTPSResolver(log.Log, s.DNSOverHTTPSServerURL)
+	netx := &netxlite.Netx{}
+	dnsResolver := netx.NewParallelDNSOverHTTPSResolver(log.Log, s.DNSOverHTTPSServerURL)
 	defer dnsResolver.CloseIdleConnections()
 
 	// lookup for both A and AAAA entries

diff --git a/internal/cmd/oohelper/oohelper.go b/internal/cmd/oohelper/oohelper.go
@@ -28,7 +28,8 @@ func init() {
 	// Use a nonstandard resolver, which is enough to work around the
 	// puzzling https://github.com/ooni/probe/issues/1409 issue.
 	const resolverURL = "https://8.8.8.8/dns-query"
-	resolver = netxlite.NewParallelDNSOverHTTPSResolver(log.Log, resolverURL)
+	netx := &netxlite.Netx{}
+	resolver = netx.NewParallelDNSOverHTTPSResolver(log.Log, resolverURL)
 	// TODO(https://github.com/ooni/probe/issues/2534): the NewHTTPClientWithResolver func has QUIRKS but we don't care.
 	httpClient = netxlite.NewHTTPClientWithResolver(log.Log, resolver)
 }

diff --git a/internal/cmd/ooporthelper/main_test.go b/internal/cmd/ooporthelper/main_test.go
@@ -19,7 +19,8 @@ func TestMainWorkingAsIntended(t *testing.T) {
 		portsMap[port] = false
 	}
 	go main()
-	dialer := netxlite.NewDialerWithoutResolver(model.DiscardLogger)
+	netx := &netxlite.Netx{}
+	dialer := netx.NewDialerWithoutResolver(model.DiscardLogger)
 	for i := 0; i < len(TestPorts); i++ {
 		port := <-srvTestChan
 		addr := net.JoinHostPort("127.0.0.1", port)

diff --git a/internal/enginelocate/cloudflare_test.go b/internal/enginelocate/cloudflare_test.go
@@ -16,12 +16,13 @@ func TestIPLookupWorksUsingcloudlflare(t *testing.T) {
 		t.Skip("skip test in short mode")
 	}
 
+	netx := &netxlite.Netx{}
 	ip, err := cloudflareIPLookup(
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
 		model.HTTPHeaderUserAgent,
-		netxlite.NewStdlibResolver(model.DiscardLogger),
+		netx.NewStdlibResolver(model.DiscardLogger),
 	)
 	if err != nil {
 		t.Fatal(err)

diff --git a/internal/enginelocate/geolocate.go b/internal/enginelocate/geolocate.go
@@ -82,8 +82,9 @@ func NewTask(config Config) *Task {
 	if config.UserAgent == "" {
 		config.UserAgent = fmt.Sprintf("ooniprobe-engine/%s", version.Version)
 	}
+	netx := &netxlite.Netx{}
 	if config.Resolver == nil {
-		config.Resolver = netxlite.NewStdlibResolver(config.Logger)
+		config.Resolver = netx.NewStdlibResolver(config.Logger)
 	}
 	return &Task{
 		countryLookupper:     mmdbLookupper{},

diff --git a/internal/enginelocate/iplookup_test.go b/internal/enginelocate/iplookup_test.go
@@ -17,9 +17,10 @@ func TestIPLookupGood(t *testing.T) {
 		t.Skip("skip test in short mode")
 	}
 
+	netx := &netxlite.Netx{}
 	ip, err := (ipLookupClient{
 		Logger:    log.Log,
-		Resolver:  netxlite.NewStdlibResolver(model.DiscardLogger),
+		Resolver:  netx.NewStdlibResolver(model.DiscardLogger),
 		UserAgent: "ooniprobe-engine/0.1.0",
 	}).LookupProbeIP(context.Background())
 	if err != nil {
@@ -33,9 +34,10 @@ func TestIPLookupGood(t *testing.T) {
 func TestIPLookupAllFailed(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // immediately cancel to cause Do() to fail
+	netx := &netxlite.Netx{}
 	ip, err := (ipLookupClient{
 		Logger:    log.Log,
-		Resolver:  netxlite.NewStdlibResolver(model.DiscardLogger),
+		Resolver:  netx.NewStdlibResolver(model.DiscardLogger),
 		UserAgent: "ooniprobe-engine/0.1.0",
 	}).LookupProbeIP(ctx)
 	if !errors.Is(err, context.Canceled) {
@@ -48,9 +50,10 @@ func TestIPLookupAllFailed(t *testing.T) {
 
 func TestIPLookupInvalidIP(t *testing.T) {
 	ctx := context.Background()
+	netx := &netxlite.Netx{}
 	ip, err := (ipLookupClient{
 		Logger:    log.Log,
-		Resolver:  netxlite.NewStdlibResolver(model.DiscardLogger),
+		Resolver:  netx.NewStdlibResolver(model.DiscardLogger),
 		UserAgent: "ooniprobe-engine/0.1.0",
 	}).doWithCustomFunc(ctx, invalidIPLookup)
 	if !errors.Is(err, ErrInvalidIPAddress) {

diff --git a/internal/enginelocate/resolverlookup.go b/internal/enginelocate/resolverlookup.go
@@ -14,7 +14,8 @@ type resolverLookupClient struct {
 
 func (rlc resolverLookupClient) LookupResolverIP(ctx context.Context) (string, error) {
 	// MUST be the system resolver! See https://github.com/ooni/probe/issues/2360
-	reso := netxlite.NewStdlibResolver(rlc.Logger)
+	netx := &netxlite.Netx{}
+	reso := netx.NewStdlibResolver(rlc.Logger)
 	var ips []string
 	ips, err := reso.LookupHost(ctx, "whoami.v4.powerdns.org")
 	if err != nil {

diff --git a/internal/enginelocate/stun.go b/internal/enginelocate/stun.go
@@ -33,7 +33,8 @@ func stunIPLookup(ctx context.Context, config stunConfig) (string, error) {
 	ip, err := func() (string, error) {
 		dialer := config.Dialer
 		if dialer == nil {
-			dialer = netxlite.NewDialerWithResolver(config.Logger, config.Resolver)
+			netx := &netxlite.Netx{}
+			dialer = netx.NewDialerWithResolver(config.Logger, config.Resolver)
 		}
 		conn, err := dialer.DialContext(ctx, "udp", config.Endpoint)
 		if err != nil {

diff --git a/internal/enginelocate/stun_test.go b/internal/enginelocate/stun_test.go
@@ -18,10 +18,11 @@ import (
 func TestSTUNIPLookupCanceledContext(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // stop immediately
+	netx := &netxlite.Netx{}
 	ip, err := stunIPLookup(ctx, stunConfig{
 		Endpoint: "stun.ekiga.net:3478",
 		Logger:   log.Log,
-		Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
+		Resolver: netx.NewStdlibResolver(model.DiscardLogger),
 	})
 	if !errors.Is(err, context.Canceled) {
 		t.Fatalf("not the error we expected: %+v", err)
@@ -151,12 +152,13 @@ func TestIPLookupWorksUsingSTUNEkiga(t *testing.T) {
 		t.Skip("skip test in short mode")
 	}
 
+	netx := &netxlite.Netx{}
 	ip, err := stunEkigaIPLookup(
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
 		model.HTTPHeaderUserAgent,
-		netxlite.NewStdlibResolver(model.DiscardLogger),
+		netx.NewStdlibResolver(model.DiscardLogger),
 	)
 	if err != nil {
 		t.Fatal(err)
@@ -171,12 +173,13 @@ func TestIPLookupWorksUsingSTUNGoogle(t *testing.T) {
 		t.Skip("skip test in short mode")
 	}
 
+	netx := &netxlite.Netx{}
 	ip, err := stunGoogleIPLookup(
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
 		model.HTTPHeaderUserAgent,
-		netxlite.NewStdlibResolver(model.DiscardLogger),
+		netx.NewStdlibResolver(model.DiscardLogger),
 	)
 	if err != nil {
 		t.Fatal(err)

diff --git a/internal/enginelocate/ubuntu_test.go b/internal/enginelocate/ubuntu_test.go
@@ -14,6 +14,7 @@ import (
 )
 
 func TestUbuntuParseError(t *testing.T) {
+	netx := &netxlite.Netx{}
 	ip, err := ubuntuIPLookup(
 		context.Background(),
 		&http.Client{Transport: FakeTransport{
@@ -24,7 +25,7 @@ func TestUbuntuParseError(t *testing.T) {
 		}},
 		log.Log,
 		model.HTTPHeaderUserAgent,
-		netxlite.NewStdlibResolver(model.DiscardLogger),
+		netx.NewStdlibResolver(model.DiscardLogger),
 	)
 	if err == nil || !strings.HasPrefix(err.Error(), "XML syntax error") {
 		t.Fatalf("not the error we expected: %+v", err)
@@ -39,12 +40,13 @@ func TestIPLookupWorksUsingUbuntu(t *testing.T) {
 		t.Skip("skip test in short mode")
 	}
 
+	netx := &netxlite.Netx{}
 	ip, err := ubuntuIPLookup(
 		context.Background(),
 		http.DefaultClient,
 		log.Log,
 		model.HTTPHeaderUserAgent,
-		netxlite.NewStdlibResolver(model.DiscardLogger),
+		netx.NewStdlibResolver(model.DiscardLogger),
 	)
 	if err != nil {
 		t.Fatal(err)

diff --git a/internal/enginenetx/httpsdialer_test.go b/internal/enginenetx/httpsdialer_test.go
@@ -493,7 +493,8 @@ func TestHTTPSDialerHostNetworkQA(t *testing.T) {
 
 		// The resolver we're creating here reproduces the test case described by
 		// https://github.com/ooni/probe-cli/pull/1295#issuecomment-1731243994
-		resolver := netxlite.MaybeWrapWithBogonResolver(true, netxlite.NewStdlibResolver(log.Log))
+		netx := &netxlite.Netx{}
+		resolver := netxlite.MaybeWrapWithBogonResolver(true, netx.NewStdlibResolver(log.Log))
 
 		httpsDialer := newHTTPSDialer(
 			log.Log,

diff --git a/internal/enginenetx/network.go b/internal/enginenetx/network.go
@@ -90,7 +90,8 @@ func NewNetwork(
 	// Create a dialer ONLY used for dialing unencrypted TCP connections. The common use
 	// case of this Network is to dial encrypted connections. For this reason, here it is
 	// reasonably fine to use the legacy sequential dialer implemented in netxlite.
-	dialer := netxlite.NewDialerWithResolver(logger, resolver)
+	netx := &netxlite.Netx{}
+	dialer := netx.NewDialerWithResolver(logger, resolver)
 
 	// Create manager for keeping track of statistics
 	const trimInterval = 30 * time.Second
@@ -135,12 +136,12 @@ func NewNetwork(
 	// Make sure we count the bytes sent and received as part of the session
 	txp = bytecounter.WrapHTTPTransport(txp, counter)
 
-	netx := &Network{
+	network := &Network{
 		reso:  resolver,
 		stats: stats,
 		txp:   txp,
 	}
-	return netx
+	return network
 }
 
 // newHTTPSDialerPolicy contains the logic to select the [HTTPSDialerPolicy] to use.

diff --git a/internal/enginenetx/network_internal_test.go b/internal/enginenetx/network_internal_test.go
@@ -187,7 +187,7 @@ func TestNetworkUnit(t *testing.T) {
 						tc.kvStore(),
 						log.Log,
 						nil, // proxy URL
-						netxlite.NewStdlibResolver(log.Log),
+						(&netxlite.Netx{}).NewStdlibResolver(log.Log),
 					)
 					defer netx.Close()
 

diff --git a/internal/enginenetx/network_test.go b/internal/enginenetx/network_test.go
@@ -25,12 +25,13 @@ func TestNetworkQA(t *testing.T) {
 		defer env.Close()
 
 		env.Do(func() {
+			netx := &netxlite.Netx{}
 			txp := enginenetx.NewNetwork(
 				bytecounter.New(),
 				&kvstore.Memory{},
 				log.Log,
 				nil,
-				netxlite.NewStdlibResolver(log.Log),
+				netx.NewStdlibResolver(log.Log),
 			)
 			client := txp.NewHTTPClient()
 			resp, err := client.Get("https://www.example.com/")
@@ -62,6 +63,7 @@ func TestNetworkQA(t *testing.T) {
 		defer proxy.Close()
 
 		env.Do(func() {
+			netx := &netxlite.Netx{}
 			txp := enginenetx.NewNetwork(
 				bytecounter.New(),
 				&kvstore.Memory{},
@@ -71,7 +73,7 @@ func TestNetworkQA(t *testing.T) {
 					Host:   net.JoinHostPort(env.ClientStack.IPAddress(), "9050"),
 					Path:   "/",
 				},
-				netxlite.NewStdlibResolver(log.Log),
+				netx.NewStdlibResolver(log.Log),
 			)
 			client := txp.NewHTTPClient()
 
@@ -131,6 +133,7 @@ func TestNetworkQA(t *testing.T) {
 		defer proxy.Close()
 
 		env.Do(func() {
+			netx := &netxlite.Netx{}
 			txp := enginenetx.NewNetwork(
 				bytecounter.New(),
 				&kvstore.Memory{},
@@ -140,7 +143,7 @@ func TestNetworkQA(t *testing.T) {
 					Host:   net.JoinHostPort(env.ClientStack.IPAddress(), "8080"),
 					Path:   "/",
 				},
-				netxlite.NewStdlibResolver(log.Log),
+				netx.NewStdlibResolver(log.Log),
 			)
 			client := txp.NewHTTPClient()
 
@@ -202,6 +205,7 @@ func TestNetworkQA(t *testing.T) {
 		defer proxy.Close()
 
 		env.Do(func() {
+			netx := &netxlite.Netx{}
 			txp := enginenetx.NewNetwork(
 				bytecounter.New(),
 				&kvstore.Memory{},
@@ -211,7 +215,7 @@ func TestNetworkQA(t *testing.T) {
 					Host:   net.JoinHostPort(env.ClientStack.IPAddress(), "4443"),
 					Path:   "/",
 				},
-				netxlite.NewStdlibResolver(log.Log),
+				netx.NewStdlibResolver(log.Log),
 			)
 			client := txp.NewHTTPClient()
 
@@ -257,12 +261,13 @@ func TestNetworkQA(t *testing.T) {
 	})
 
 	t.Run("NewHTTPClient returns a client with a cookie jar", func(t *testing.T) {
+		netx := &netxlite.Netx{}
 		txp := enginenetx.NewNetwork(
 			bytecounter.New(),
 			&kvstore.Memory{},
 			log.Log,
 			nil,
-			netxlite.NewStdlibResolver(log.Log),
+			netx.NewStdlibResolver(log.Log),
 		)
 		client := txp.NewHTTPClient()
 		if client.Jar == nil {

diff --git a/internal/enginenetx/statsmanager_test.go b/internal/enginenetx/statsmanager_test.go
@@ -228,7 +228,7 @@ func TestNetworkCollectsStats(t *testing.T) {
 
 			qa.Do(func() {
 				byteCounter := bytecounter.New()
-				resolver := netxlite.NewStdlibResolver(log.Log)
+				resolver := (&netxlite.Netx{}).NewStdlibResolver(log.Log)
 
 				netx := NewNetwork(byteCounter, kvStore, log.Log, nil, resolver)
 				defer netx.Close()

diff --git a/internal/engineresolver/factory.go b/internal/engineresolver/factory.go
@@ -57,8 +57,9 @@ func newChildResolver(
 	case "http", "https": // http is here for testing
 		reso = newChildResolverHTTPS(logger, URL, http3Enabled, counter, proxyURL)
 	case "system":
+		netx := &netxlite.Netx{}
 		reso = bytecounter.MaybeWrapSystemResolver(
-			netxlite.NewStdlibResolver(logger),
+			netx.NewStdlibResolver(logger),
 			counter, // handles correctly the case where counter is nil
 		)
 	default:
@@ -78,18 +79,19 @@ func newChildResolverHTTPS(
 	proxyURL *url.URL,
 ) model.Resolver {
 	var txp model.HTTPTransport
+	netx := &netxlite.Netx{}
 	switch http3Enabled {
 	case false:
 		dialer := netxlite.NewDialerWithStdlibResolver(logger)
-		thx := netxlite.NewTLSHandshakerStdlib(logger)
+		thx := netx.NewTLSHandshakerStdlib(logger)
 		tlsDialer := netxlite.NewTLSDialer(dialer, thx)
 		txp = netxlite.NewHTTPTransportWithOptions(
 			logger, dialer, tlsDialer,
 			netxlite.HTTPTransportOptionDisableCompression(false), // defaults to true but compression is fine here
 			netxlite.HTTPTransportOptionProxyURL(proxyURL),        // nil here disables using the proxy
 		)
 	case true:
-		txp = netxlite.NewHTTP3TransportStdlib(logger)
+		txp = netx.NewHTTP3TransportStdlib(logger)
 	}
 	txp = bytecounter.MaybeWrapHTTPTransport(txp, counter)
 	dnstxp := netxlite.NewDNSOverHTTPSTransportWithHTTPTransport(txp, URL)

diff --git a/internal/experiment/ndt7/dial.go b/internal/experiment/ndt7/dial.go
@@ -30,8 +30,9 @@ func newDialManager(ndt7URL string, logger model.Logger, userAgent string) dialM
 }
 
 func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*websocket.Conn, error) {
-	reso := netxlite.NewStdlibResolver(mgr.logger)
-	dlr := netxlite.NewDialerWithResolver(mgr.logger, reso)
+	netx := &netxlite.Netx{}
+	reso := netx.NewStdlibResolver(mgr.logger)
+	dlr := netx.NewDialerWithResolver(mgr.logger, reso)
 	dlr = bytecounter.WrapWithContextAwareDialer(dlr)
 	// Implements shaping if the user builds using `-tags shaping`
 	// See https://github.com/ooni/probe/issues/2112

diff --git a/internal/experiment/simplequicping/simplequicping.go b/internal/experiment/simplequicping/simplequicping.go
@@ -172,7 +172,8 @@ func (m *Measurer) quicHandshake(ctx context.Context, index int64,
 	alpn := strings.Split(m.config.alpn(), " ")
 	trace := measurexlite.NewTrace(index, zeroTime)
 	ol := logx.NewOperationLogger(logger, "SimpleQUICPing #%d %s %s %v", index, address, sni, alpn)
-	listener := netxlite.NewUDPListener()
+	netx := &netxlite.Netx{}
+	listener := netx.NewUDPListener()
 	dialer := trace.NewQUICDialerWithoutResolver(listener, logger)
 	// See https://github.com/ooni/probe/issues/2413 to understand
 	// why we're using nil to force netxlite to use the cached

diff --git a/internal/experiment/webconnectivitylte/dnsresolvers.go b/internal/experiment/webconnectivitylte/dnsresolvers.go
@@ -249,7 +249,8 @@ func (t *DNSResolvers) lookupHostUDP(parentCtx context.Context, udpAddress strin
 	)
 
 	// runs the lookup
-	dialer := netxlite.NewDialerWithoutResolver(t.Logger)
+	netx := &netxlite.Netx{}
+	dialer := netx.NewDialerWithoutResolver(t.Logger)
 	reso := trace.NewParallelUDPResolver(t.Logger, dialer, udpAddress)
 	addrs, err := reso.LookupHost(lookupCtx, t.Domain)