diff --git a/internal/netxlite/http.go b/internal/netxlite/http.go index 205bb3e..d550535 100644 --- a/internal/netxlite/http.go +++ b/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, } } diff --git a/internal/netxlite/http_test.go b/internal/netxlite/http_test.go index 56e0ad9..65ef599 100644 --- a/internal/netxlite/http_test.go +++ b/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") } }