feat(netxlite): add flexible HTTP transport factory

internal/netxlite/httpfactory.go

@@ -1,12 +1,96 @@
 package netxlite
 
 import (
- "net/http"
+ "net/url"
 
+ oohttp "github.com/ooni/oohttp"
  "github.com/ooni/probe-cli/v3/internal/model"
 )
 
-// NewHTTPClient creates a new, wrapped HTTPClient using the given transport.
-func NewHTTPClient(txp model.HTTPTransport) model.HTTPClient {
- return WrapHTTPClient(&http.Client{Transport: txp})
+// HTTPTransportOption is an initialization option for [NewHTTPTransport].
+type HTTPTransportOption func(txp *oohttp.Transport)
+
+// NewHTTPTransport is the high-level factory to create a [model.HTTPTransport] using
+// github.com/ooni/oohttp as the HTTP library with HTTP/1.1 and HTTP2 support.
+//
+// This transport is suitable for HTTP2 and HTTP/1.1 using any TLS
+// library, including, e.g., github.com/ooni/oocrypto.
+//
+// This factory clones the default github.com/ooni/oohttp transport and
+// configures the provided dialer and TLS dialer by setting the .DialContext
+// and .DialTLSContext fields of the transport. We also wrap the provided
+// dialers to address https://github.com/ooni/probe/issues/1609.
+//
+// Apart from that, the only non-default options set by this factory are these:
+//
+// 1. the .Proxy field is set to nil, so by default we DO NOT honour the
+// HTTP_PROXY and HTTPS_PROXY environment variables, which is required if
+// we want to use this code for measuring;
+//
+// 2. the .ForceAttemptHTTP2 field is set to true;
+//
+// 3. the .DisableCompression field is set to true, again required if we
+// want to use this code for measuring, and we should make sure the defaults
+// we're using are suitable for measuring, since the impact of making a
+// mistake in measuring code is a data quality issue 😅.
+//
+// The returned transport supports logging and error wrapping because
+// internally this function calls [WrapHTTPTransport] before we return.
+//
+// This factory is the RECOMMENDED way of creating a [model.HTTPTransport].
+func NewHTTPTransportWithOptions(logger model.Logger,
+ dialer model.Dialer, tlsDialer model.TLSDialer, options ...HTTPTransportOption) model.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 = &httpTLSDialerWithReadTimeout{tlsDialer}
+ txp.DialTLSContext = tlsDialer.DialTLSContext
+
+ // As documented, disable proxies and force HTTP/2
+ txp.DisableCompression = true
+ txp.Proxy = nil
+ txp.ForceAttemptHTTP2 = true
+
+ // Apply all the required options
+ for _, option := range options {
+ option(txp)
+ }
+
+ // Return a fully wrapped HTTP transport
+ return WrapHTTPTransport(logger, &httpTransportConnectionsCloser{
+ HTTPTransport: &httpTransportStdlib{&oohttp.StdlibTransport{Transport: txp}},
+ Dialer: dialer,
+ TLSDialer: tlsDialer,
+ })
+}
+
+// HTTPTransportOptionProxyURL configures the transport to use the given proxyURL
+// or disables proxying (already the default) if the proxyURL is nil.
+func HTTPTransportOptionProxyURL(proxyURL *url.URL) HTTPTransportOption {
+ return func(txp *oohttp.Transport) {
+ txp.Proxy = func(r *oohttp.Request) (*url.URL, error) {
+ // "If Proxy is nil or returns a nil *URL, no proxy is used."
+ return proxyURL, nil
+ }
+ }
+}
+
+// HTTPTransportOptionMaxConnsPerHost configures the .MaxConnPerHosts field, which
+// otherwise uses the default set in github.com/ooni/oohttp.
+func HTTPTransportOptionMaxConnsPerHost(value int) HTTPTransportOption {
+ return func(txp *oohttp.Transport) {
+ txp.MaxConnsPerHost = value
+ }
+}
+
+// HTTPTransportOptionDisableCompression configures the .DisableCompression field, which
+// otherwise is set to true, so that this code is ready for measuring out of the box.
+func HTTPTransportOptionDisableCompression(value bool) HTTPTransportOption {
+ return func(txp *oohttp.Transport) {
+ txp.DisableCompression = value
+ }
 }

internal/netxlite/httpfactory_test.go

@@ -0,0 +1,169 @@
+package netxlite
+
+import (
+ "crypto/tls"
+ "net/url"
+ "testing"
+
+ "github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp/cmpopts"
+ oohttp "github.com/ooni/oohttp"
+ "github.com/ooni/probe-cli/v3/internal/mocks"
+ "github.com/ooni/probe-cli/v3/internal/model"
+)
+
+func TestNewHTTPTransportWithOptions(t *testing.T) {
+
+ t.Run("make sure that we get the correct types and settings", func(t *testing.T) {
+ expectDialer := &mocks.Dialer{}
+ expectTLSDialer := &mocks.TLSDialer{}
+ expectLogger := model.DiscardLogger
+ txp := NewHTTPTransportWithOptions(expectLogger, expectDialer, expectTLSDialer)
+
+ // undo the results of the netxlite.WrapTransport function
+ txpLogger := txp.(*httpTransportLogger)
+ if txpLogger.Logger != expectLogger {
+ t.Fatal("invalid logger")
+ }
+ txpErrWrapper := txpLogger.HTTPTransport.(*httpTransportErrWrapper)
+
+ // make sure we correctly configured dialer and TLS dialer
+ txpCloser := txpErrWrapper.HTTPTransport.(*httpTransportConnectionsCloser)
+ timeoutDialer := txpCloser.Dialer.(*httpDialerWithReadTimeout)
+ childDialer := timeoutDialer.Dialer
+ if childDialer != expectDialer {
+ t.Fatal("invalid dialer")
+ }
+ timeoutTLSDialer := txpCloser.TLSDialer.(*httpTLSDialerWithReadTimeout)
+ childTLSDialer := timeoutTLSDialer.TLSDialer
+ if childTLSDialer != expectTLSDialer {
+ t.Fatal("invalid TLS dialer")
+ }
+
+ // make sure there's the stdlib adapter
+ stdlibAdapter := txpCloser.HTTPTransport.(*httpTransportStdlib)
+ oohttpStdlibAdapter := stdlibAdapter.StdlibTransport
+ underlying := oohttpStdlibAdapter.Transport
+
+ // now let's check that everything is configured as intended
+ expectedTxp := oohttp.DefaultTransport.(*oohttp.Transport).Clone()
+ diff := cmp.Diff(
+ expectedTxp,
+ underlying,
+ cmpopts.IgnoreUnexported(oohttp.Transport{}),
+ cmpopts.IgnoreUnexported(tls.Config{}),
+ cmpopts.IgnoreFields(
+ oohttp.Transport{},
+ "DialContext",
+ "DialTLSContext",
+ "DisableCompression",
+ "Proxy",
+ "ForceAttemptHTTP2",
+ ),
+ )
+ if diff != "" {
+ t.Fatal(diff)
+ }
+
+ // finish checking by explicitly inspecting the fields we modify
+ if underlying.DialContext == nil {
+ t.Fatal("expected non-nil .DialContext")
+ }
+ if underlying.DialTLSContext == nil {
+ t.Fatal("expected non-nil .DialTLSContext")
+ }
+ if underlying.Proxy != nil {
+ t.Fatal("expected nil .Proxy")
+ }
+ if !underlying.ForceAttemptHTTP2 {
+ t.Fatal("expected true .ForceAttemptHTTP2")
+ }
+ if !underlying.DisableCompression {
+ t.Fatal("expected true .DisableCompression")
+ }
+ })
+
+ unwrap := func(txp model.HTTPTransport) *oohttp.Transport {
+ txpLogger := txp.(*httpTransportLogger)
+ txpErrWrapper := txpLogger.HTTPTransport.(*httpTransportErrWrapper)
+ txpCloser := txpErrWrapper.HTTPTransport.(*httpTransportConnectionsCloser)
+ stdlibAdapter := txpCloser.HTTPTransport.(*httpTransportStdlib)
+ oohttpStdlibAdapter := stdlibAdapter.StdlibTransport
+ return oohttpStdlibAdapter.Transport
+ }
+
+ t.Run("make sure HTTPTransportOptionProxyURL is WAI", func(t *testing.T) {
+ runWithURL := func(expectedURL *url.URL) {
+ expectDialer := &mocks.Dialer{}
+ expectTLSDialer := &mocks.TLSDialer{}
+ expectLogger := model.DiscardLogger
+ txp := NewHTTPTransportWithOptions(
+ expectLogger,
+ expectDialer,
+ expectTLSDialer,
+ HTTPTransportOptionProxyURL(expectedURL),
+ )
+ underlying := unwrap(txp)
+ if underlying.Proxy == nil {
+ t.Fatal("expected non-nil .Proxy")
+ }
+ got, err := underlying.Proxy(&oohttp.Request{})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got != expectedURL {
+ t.Fatal("not the expected URL")
+ }
+ }
+
+ runWithURL(&url.URL{})
+
+ runWithURL(nil)
+ })
+
+ t.Run("make sure HTTPTransportOptionMaxConnsPerHost is WAI", func(t *testing.T) {
+ runWithValue := func(expectedValue int) {
+ expectDialer := &mocks.Dialer{}
+ expectTLSDialer := &mocks.TLSDialer{}
+ expectLogger := model.DiscardLogger
+ txp := NewHTTPTransportWithOptions(
+ expectLogger,
+ expectDialer,
+ expectTLSDialer,
+ HTTPTransportOptionMaxConnsPerHost(expectedValue),
+ )
+ underlying := unwrap(txp)
+ got := underlying.MaxConnsPerHost
+ if got != expectedValue {
+ t.Fatal("not the expected value")
+ }
+ }
+
+ runWithValue(100)
+
+ runWithValue(10)
+ })
+
+ t.Run("make sure HTTPTransportDisableCompression is WAI", func(t *testing.T) {
+ runWithValue := func(expectedValue bool) {
+ expectDialer := &mocks.Dialer{}
+ expectTLSDialer := &mocks.TLSDialer{}
+ expectLogger := model.DiscardLogger
+ txp := NewHTTPTransportWithOptions(
+ expectLogger,
+ expectDialer,
+ expectTLSDialer,
+ HTTPTransportOptionDisableCompression(expectedValue),
+ )
+ underlying := unwrap(txp)
+ got := underlying.DisableCompression
+ if got != expectedValue {
+ t.Fatal("not the expected value")
+ }
+ }
+
+ runWithValue(true)
+
+ runWithValue(false)
+ })
+}

internal/netxlite/httpquirks.go

@@ -5,8 +5,12 @@ package netxlite
 //
 // Ideally, we should not modify this code or apply minimal and obvious changes.
 //
+// TODO(https://github.com/ooni/probe/issues/2534)
+//
 
 import (
+ "net/http"
+
  oohttp "github.com/ooni/oohttp"
  "github.com/ooni/probe-cli/v3/internal/model"
 )
@@ -129,3 +133,12 @@ func NewHTTPClientStdlib(logger model.DebugLogger) model.HTTPClient {
 func NewHTTPClientWithResolver(logger model.Logger, reso model.Resolver) model.HTTPClient {
  return NewHTTPClient(NewHTTPTransportWithResolver(logger, reso))
 }
+
+// NewHTTPClient creates a new, wrapped HTTPClient using the given transport.
+//
+// This function behavior is QUIRKY because it does not configure a cookie jar, which
+// is probably not the right thing to do in many cases, but legacy code MAY depend
+// on this behavior. TODO(https://github.com/ooni/probe/issues/2534).
+func NewHTTPClient(txp model.HTTPTransport) model.HTTPClient {
+ return WrapHTTPClient(&http.Client{Transport: txp})
+}