feat(netxlite): integrate websteps code to use ooni/oohttp (#466)

Part of https://github.com/ooni/probe/issues/1591

internal/netxlite/http.go

@@ -5,6 +5,8 @@ import (
 	"net"
 	"net/http"
 	"time"
+
+	oohttp "github.com/ooni/oohttp"
 )
 
 // HTTPTransport is an http.Transport-like structure.
@@ -87,30 +89,58 @@ func (txp *httpTransportConnectionsCloser) CloseIdleConnections() {
 // We need a TLS handshaker here, as opposed to a TLSDialer, because we
 // wrap the dialer we'll use to enforce timeouts for HTTP idle
 // connections (see https://github.com/ooni/probe/issues/1609 for more info).
-func NewHTTPTransport(dialer Dialer, tlsHandshaker TLSHandshaker) HTTPTransport {
-	// TODO(bassosimone): here we should copy code living inside the
-	// websteps prototype to use the oohttp library.
-	txp := http.DefaultTransport.(*http.Transport).Clone()
+//
+// The returned transport will use the given Logger for logging.
+//
+// The returned transport will gracefully handle TLS connections
+// created using gitlab.com/yawning/utls.git.
+//
+// The returned transport will not have a configured proxy, not
+// even the proxy configurable from the environment.
+//
+// The returned transport will disable transparent decompression
+// of compressed response bodies (and will not automatically
+// ask for such compression, though you can always do that manually).
+func NewHTTPTransport(logger Logger, dialer Dialer, tlsHandshaker TLSHandshaker) HTTPTransport {
+	// Using oohttp to support any TLS library.
+	txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone()
+
 	// This wrapping ensures that we always have a timeout when we
 	// are using HTTP; see https://github.com/ooni/probe/issues/1609.
 	dialer = &httpDialerWithReadTimeout{dialer}
 	txp.DialContext = dialer.DialContext
 	tlsDialer := NewTLSDialer(dialer, tlsHandshaker)
 	txp.DialTLSContext = tlsDialer.DialTLSContext
+
+	// We are using a different strategy to implement proxy: we
+	// use a specific dialer that knows about proxying.
+	txp.Proxy = nil
+
 	// Better for Cloudflare DNS and also better because we have less
 	// noisy events and we can better understand what happened.
+	//
+	// UNDOCUMENTED: I am wondering whether we can relax this constraint.
 	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
+
+	// Required to enable using HTTP/2 (which will be anyway forced
+	// upon us when we are using TLS parroting).
 	txp.ForceAttemptHTTP2 = true
-	// Ensure we correctly forward CloseIdleConnections.
-	return &httpTransportConnectionsCloser{
-		HTTPTransport: txp,
-		Dialer:        dialer,
-		TLSDialer:     tlsDialer,
+
+	// Ensure we correctly forward CloseIdleConnections and compose
+	// with a logging transport thus enabling logging.
+	return &httpTransportLogger{
+		HTTPTransport: &httpTransportConnectionsCloser{
+			HTTPTransport: &oohttp.StdlibTransport{Transport: txp},
+			Dialer:        dialer,
+			TLSDialer:     tlsDialer,
+		},
+		Logger: logger,
 	}
 }
 

internal/netxlite/http_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/apex/log"
+	oohttp "github.com/ooni/oohttp"
 	"github.com/ooni/probe-cli/v3/internal/atomicx"
 	"github.com/ooni/probe-cli/v3/internal/netxlite/iox"
 	"github.com/ooni/probe-cli/v3/internal/netxlite/mocks"
@@ -110,7 +111,7 @@ func TestHTTPTransportLoggerCloseIdleConnections(t *testing.T) {
 
 func TestHTTPTransportWorks(t *testing.T) {
 	d := NewDialerWithResolver(log.Log, NewResolverSystem(log.Log))
-	txp := NewHTTPTransport(d, NewTLSHandshakerStdlib(log.Log))
+	txp := NewHTTPTransport(log.Log, d, NewTLSHandshakerStdlib(log.Log))
 	client := &http.Client{Transport: txp}
 	defer client.CloseIdleConnections()
 	resp, err := client.Get("https://www.google.com/robots.txt")
@@ -135,7 +136,7 @@ func TestHTTPTransportWithFailingDialer(t *testing.T) {
 		},
 		Resolver: NewResolverSystem(log.Log),
 	}
-	txp := NewHTTPTransport(d, NewTLSHandshakerStdlib(log.Log))
+	txp := NewHTTPTransport(log.Log, d, NewTLSHandshakerStdlib(log.Log))
 	client := &http.Client{Transport: txp}
 	resp, err := client.Get("https://www.google.com/robots.txt")
 	if !errors.Is(err, expected) {
@@ -153,8 +154,15 @@ func TestHTTPTransportWithFailingDialer(t *testing.T) {
 func TestNewHTTPTransport(t *testing.T) {
 	d := &mocks.Dialer{}
 	th := &mocks.TLSHandshaker{}
-	txp := NewHTTPTransport(d, th)
-	txpcc, okay := txp.(*httpTransportConnectionsCloser)
+	txp := NewHTTPTransport(log.Log, d, th)
+	logtxp, okay := txp.(*httpTransportLogger)
+	if !okay {
+		t.Fatal("invalid type")
+	}
+	if logtxp.Logger != log.Log {
+		t.Fatal("invalid logger")
+	}
+	txpcc, okay := logtxp.HTTPTransport.(*httpTransportConnectionsCloser)
 	if !okay {
 		t.Fatal("invalid type")
 	}
@@ -168,23 +176,23 @@ func TestNewHTTPTransport(t *testing.T) {
 	if txpcc.TLSDialer.(*tlsDialer).TLSHandshaker != th {
 		t.Fatal("invalid tls handshaker")
 	}
-	htxp, okay := txpcc.HTTPTransport.(*http.Transport)
+	stdwtxp, okay := txpcc.HTTPTransport.(*oohttp.StdlibTransport)
 	if !okay {
 		t.Fatal("invalid type")
 	}
-	if !htxp.ForceAttemptHTTP2 {
+	if !stdwtxp.Transport.ForceAttemptHTTP2 {
 		t.Fatal("invalid ForceAttemptHTTP2")
 	}
-	if !htxp.DisableCompression {
+	if !stdwtxp.Transport.DisableCompression {
 		t.Fatal("invalid DisableCompression")
 	}
-	if htxp.MaxConnsPerHost != 1 {
+	if stdwtxp.Transport.MaxConnsPerHost != 1 {
 		t.Fatal("invalid MaxConnPerHost")
 	}
-	if htxp.DialTLSContext == nil {
+	if stdwtxp.Transport.DialTLSContext == nil {
 		t.Fatal("invalid DialTLSContext")
 	}
-	if htxp.DialContext == nil {
+	if stdwtxp.Transport.DialContext == nil {
 		t.Fatal("invalid DialContext")
 	}
 }