diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index ba354156ac..15af7fce90 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -232,7 +232,7 @@ func (e *experiment) OpenReportContext(ctx context.Context) error { // use custom client to have proper byte accounting httpClient := &http.Client{ Transport: bytecounter.WrapHTTPTransport( - e.session.httpDefaultTransport, // proxy is OK + e.session.httpDefaultTransport, e.byteCounter, ), } diff --git a/internal/engine/session.go b/internal/engine/session.go index 014ae90508..fc46a660c3 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io/ioutil" - "net/http" "net/url" "os" "sync" @@ -14,10 +13,10 @@ import ( "github.com/ooni/probe-cli/v3/internal/bytecounter" "github.com/ooni/probe-cli/v3/internal/checkincache" "github.com/ooni/probe-cli/v3/internal/enginelocate" + "github.com/ooni/probe-cli/v3/internal/enginenetx" "github.com/ooni/probe-cli/v3/internal/engineresolver" "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/platform" "github.com/ooni/probe-cli/v3/internal/probeservices" "github.com/ooni/probe-cli/v3/internal/registry" @@ -58,7 +57,7 @@ type Session struct { availableProbeServices []model.OOAPIService availableTestHelpers map[string][]model.OOAPIService byteCounter *bytecounter.Counter - httpDefaultTransport model.HTTPTransport + httpDefaultTransport *enginenetx.HTTPTransport kvStore model.KeyValueStore location *enginelocate.Results logger model.Logger @@ -214,11 +213,12 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) { Logger: sess.logger, ProxyURL: proxyURL, } - txp := netxlite.NewHTTPTransportWithLoggerResolverAndOptionalProxyURL( - sess.logger, sess.resolver, sess.proxyURL, + sess.httpDefaultTransport = enginenetx.NewHTTPTransport( + sess.byteCounter, + sess.logger, + proxyURL, + sess.resolver, ) - txp = bytecounter.WrapHTTPTransport(txp, sess.byteCounter) - sess.httpDefaultTransport = txp return sess, nil } @@ -360,7 +360,7 @@ func (s *Session) GetTestHelpersByName(name string) ([]model.OOAPIService, bool) // DefaultHTTPClient returns the session's default HTTP client. func (s *Session) DefaultHTTPClient() model.HTTPClient { - return &http.Client{Transport: s.httpDefaultTransport} + return s.httpDefaultTransport.NewHTTPClient() } // FetchTorTargets fetches tor targets from the API. diff --git a/internal/enginenetx/doc.go b/internal/enginenetx/doc.go new file mode 100644 index 0000000000..15cdd99920 --- /dev/null +++ b/internal/enginenetx/doc.go @@ -0,0 +1,2 @@ +// Package enginenetx contains engine-specific network-extensions. +package enginenetx diff --git a/internal/enginenetx/http.go b/internal/enginenetx/http.go new file mode 100644 index 0000000000..53924e9182 --- /dev/null +++ b/internal/enginenetx/http.go @@ -0,0 +1,55 @@ +package enginenetx + +import ( + "net/http" + "net/http/cookiejar" + "net/url" + + "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "golang.org/x/net/publicsuffix" +) + +// HTTPTransport is the [model.HTTPTransport] used by the [*engine.Session]. +type HTTPTransport struct { + model.HTTPTransport +} + +// NewHTTPClient is a convenience function for building a [model.HTTPClient] using +// this [*HTTPTransport] and correct cookies configuration. +func (txp *HTTPTransport) NewHTTPClient() *http.Client { + // Note: cookiejar.New cannot fail, so we're using runtimex.Try1 here + return &http.Client{ + Transport: txp, + Jar: runtimex.Try1(cookiejar.New(&cookiejar.Options{ + PublicSuffixList: publicsuffix.List, + })), + } +} + +// NewHTTPTransport creates a new [*HTTPTransport] for the engine. This client MUST NOT be +// used for measuring and implements engine-specific policies. +// +// Arguments: +// +// - counter is the [*bytecounter.Counter] to use. +// +// - logger is the [model.Logger] to use; +// +// - proxyURL is the OPTIONAL proxy URL; +// +// - resolver is the [model.Resolver] to use. +func NewHTTPTransport( + counter *bytecounter.Counter, + logger model.Logger, + proxyURL *url.URL, + resolver model.Resolver, +) *HTTPTransport { + txp := netxlite.NewHTTPTransportWithLoggerResolverAndOptionalProxyURL( + logger, resolver, proxyURL, + ) + txp = bytecounter.WrapHTTPTransport(txp, counter) + return &HTTPTransport{txp} +} diff --git a/internal/enginenetx/http_test.go b/internal/enginenetx/http_test.go new file mode 100644 index 0000000000..c3604cd6d0 --- /dev/null +++ b/internal/enginenetx/http_test.go @@ -0,0 +1,37 @@ +package enginenetx + +import ( + "testing" + + "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" +) + +func TestHTTPTransport(t *testing.T) { + + // TODO(bassosimone): we should replace this integration test with netemx + // as soon as we can sever the hard link between netxlite and this pkg + t.Run("is working as intended", func(t *testing.T) { + txp := NewHTTPTransport( + bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) + client := txp.NewHTTPClient() + resp, err := client.Get("https://www.google.com/robots.txt") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != 200 { + t.Fatal("unexpected status code") + } + }) + + t.Run("NewHTTPClient returns a client with a cookie jar", func(t *testing.T) { + txp := NewHTTPTransport( + bytecounter.New(), model.DiscardLogger, nil, netxlite.NewStdlibResolver(model.DiscardLogger)) + client := txp.NewHTTPClient() + if client.Jar == nil { + t.Fatal("expected non-nil cookie jar") + } + }) +}