refactor(oohelperd,netmx): reduce construction diff to zero (#1467)

This diff modifies how we construct oohelperd.Handler inside the
oohelperd package and inside the netemx package such that we're now
constructing using equivalent code. We know that construction is
equivalent for HTTP clients because previosuly we made sure it was the
case in #1464.

So, the next step would be removing the custom construction code inside
of netemx and always use oohelperd.NewHandler.

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.

internal/cmd/oohelperd/main.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
 	"github.com/ooni/probe-cli/v3/internal/oohelperd"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 	"github.com/ooni/probe-cli/v3/internal/version"
@@ -92,7 +93,7 @@ func main() {
 	mux := http.NewServeMux()
 
 	// add the main oohelperd handler to the mux
-	mux.Handle("/", oohelperd.NewHandler())
+	mux.Handle("/", oohelperd.NewHandler(log.Log, &netxlite.Netx{}))
 
 	// create a listening server for serving ooniprobe requests
 	srv := &http.Server{Addr: *apiEndpoint, Handler: mux}

internal/netemx/oohelperd.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/apex/log"
 	"github.com/ooni/netem"
 	"github.com/ooni/probe-cli/v3/internal/logx"
 	"github.com/ooni/probe-cli/v3/internal/model"
@@ -20,12 +21,11 @@ var _ HTTPHandlerFactory = &OOHelperDFactory{}
 // NewHandler implements QAEnvHTTPHandlerFactory.NewHandler.
 func (f *OOHelperDFactory) NewHandler(env NetStackServerFactoryEnv, unet *netem.UNetStack) http.Handler {
 	netx := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: unet}}
-	handler := oohelperd.NewHandler()
-
-	handler.BaseLogger = &logx.PrefixLogger{
+	logger := &logx.PrefixLogger{
 		Prefix: fmt.Sprintf("%-16s", "TH_HANDLER"),
-		Logger: handler.BaseLogger,
+		Logger: log.Log,
 	}
+	handler := oohelperd.NewHandler(logger, netx)
 
 	handler.NewDialer = func(logger model.Logger) model.Dialer {
 		return netx.NewDialerWithResolver(logger, netx.NewStdlibResolver(logger))
@@ -46,23 +46,14 @@ func (f *OOHelperDFactory) NewHandler(env NetStackServerFactoryEnv, unet *netem.
 	handler.NewHTTPClient = func(logger model.Logger) model.HTTPClient {
 		return oohelperd.NewHTTPClientWithTransportFactory(
 			netx, logger,
-			func(netx *netxlite.Netx, dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
-				dialer := netx.NewDialerWithResolver(dl, r)
-				tlsDialer := netxlite.NewTLSDialer(dialer, netx.NewTLSHandshakerStdlib(dl))
-				// TODO(https://github.com/ooni/probe/issues/2534): NewHTTPTransport is QUIRKY but
-				// we probably don't care about using a QUIRKY function here
-				return netxlite.NewHTTPTransport(dl, dialer, tlsDialer)
-			},
+			netxlite.NewHTTPTransportWithResolver,
 		)
 	}
 
 	handler.NewHTTP3Client = func(logger model.Logger) model.HTTPClient {
 		return oohelperd.NewHTTPClientWithTransportFactory(
 			netx, logger,
-			func(netx *netxlite.Netx, dl model.DebugLogger, r model.Resolver) model.HTTPTransport {
-				qd := netx.NewQUICDialerWithResolver(netx.NewUDPListener(), dl, r)
-				return netxlite.NewHTTP3Transport(dl, qd, nil)
-			},
+			netxlite.NewHTTP3TransportWithResolver,
 		)
 	}
 

internal/oohelperd/handler.go

@@ -15,7 +15,6 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/apex/log"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
@@ -69,10 +68,9 @@ type Handler struct {
 var _ http.Handler = &Handler{}
 
 // NewHandler constructs the [handler].
-func NewHandler() *Handler {
-	netx := &netxlite.Netx{}
+func NewHandler(logger model.Logger, netx *netxlite.Netx) *Handler {
 	return &Handler{
-		BaseLogger:        log.Log,
+		BaseLogger:        logger,
 		CountRequests:     &atomic.Int64{},
 		Indexer:           &atomic.Int64{},
 		MaxAcceptableBody: MaxAcceptableBodySize,
@@ -103,7 +101,9 @@ func NewHandler() *Handler {
 				logger,
 			)
 		},
-		NewResolver: newResolver,
+		NewResolver: func(logger model.Logger) model.Resolver {
+			return newResolver(logger, netx)
+		},
 		NewTLSHandshaker: func(logger model.Logger) model.TLSHandshaker {
 			return netx.NewTLSHandshakerStdlib(logger)
 		},
@@ -202,11 +202,10 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 
 // newResolver creates a new [model.Resolver] suitable for serving
 // requests coming from ooniprobe clients.
-func newResolver(logger model.Logger) model.Resolver {
+func newResolver(logger model.Logger, netx *netxlite.Netx) model.Resolver {
 	// Implementation note: pin to a specific resolver so we don't depend upon the
 	// default resolver configured by the box. Also, use an encrypted transport thus
 	// we're less vulnerable to any policy implemented by the box's provider.
-	netx := &netxlite.Netx{}
 	resolver := netx.NewParallelDNSOverHTTPSResolver(logger, "https://dns.google/dns-query")
 	return resolver
 }
@@ -242,7 +241,7 @@ func NewHTTPClientWithTransportFactory(
 	// So, it's better to consider this as a possible corner case.
 	reso := netxlite.MaybeWrapWithBogonResolver(
 		true, // enabled
-		newResolver(logger),
+		newResolver(logger, netx),
 	)
 
 	// fix: We MUST set a cookie jar for measuring HTTP. See

internal/oohelperd/handler_test.go

@@ -10,8 +10,10 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/apex/log"
 	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
 	"github.com/ooni/probe-cli/v3/internal/version"
 )
 
@@ -186,7 +188,7 @@ func TestHandlerWorkingAsIntended(t *testing.T) {
 	for _, expect := range expectations {
 		t.Run(expect.name, func(t *testing.T) {
 			// create handler and possibly override .Measure
-			handler := NewHandler()
+			handler := NewHandler(log.Log, &netxlite.Netx{})
 			if expect.measureFn != nil {
 				handler.Measure = expect.measureFn
 			}