From 566c6b246aa95feadd76a5849d9943025da4c368 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 7 Jan 2022 18:33:37 +0100 Subject: [PATCH] cleanup: remove unnecessary legacy interfaces (#656) This diff addresses another point of https://github.com/ooni/probe/issues/1956: > - [ ] observe that we're still using a bunch of private interfaces for common interfaces such as the `Dialer`, so we can get rid of these private interfaces and always use the ones in `model`, which allows us to remove a bunch of legacy wrappers Additional cleanups may still be possible. The more I cleanup, the more I see there's extra legacy code we can dispose of (which seems good?). --- internal/cmd/jafar/uncensored/uncensored.go | 17 +- internal/cmd/oohelper/internal/fake_test.go | 13 +- internal/cmd/oohelper/oohelper.go | 3 +- .../oohelperd/internal/webconnectivity/dns.go | 4 +- .../internal/webconnectivity/fake_test.go | 13 +- .../internal/webconnectivity/measure.go | 6 +- .../internal/webconnectivity/tcpconnect.go | 4 +- .../webconnectivity/webconnectivity.go | 6 +- .../webconnectivity/webconnectivity_test.go | 3 +- .../oohelperd/internal/websteps/explore.go | 6 +- .../oohelperd/internal/websteps/generate.go | 8 +- .../internal/websteps/generate_test.go | 4 + .../internal/websteps/initialchecks.go | 4 +- .../oohelperd/internal/websteps/measure.go | 11 +- internal/cmd/oohelperd/oohelperd.go | 5 +- internal/engine/experiment.go | 4 +- .../engine/experiment/dnscheck/dnscheck.go | 2 +- internal/engine/experiment/hhfm/hhfm.go | 3 +- internal/engine/experiment/hirl/fake_test.go | 2 + internal/engine/experiment/hirl/hirl.go | 2 +- internal/engine/experiment/hirl/hirl_test.go | 8 +- internal/engine/experiment/ndt7/dial.go | 5 +- .../stunreachability/stunreachability.go | 3 +- .../experiment/tlstool/internal/dialer.go | 4 +- .../experiment/tlstool/internal/fake_test.go | 2 + .../experiment/tlstool/internal/internal.go | 4 +- .../tlstool/internal/internal_test.go | 3 +- internal/engine/experiment/tlstool/tlstool.go | 2 +- internal/engine/experiment/websteps/dns.go | 5 +- .../engine/experiment/websteps/factory.go | 12 +- internal/engine/experiment/websteps/quic.go | 5 +- internal/engine/experiment/websteps/tcp.go | 5 +- internal/engine/geolocate/geolocate.go | 9 +- internal/engine/geolocate/iplookup.go | 2 +- .../sessionresolver/sessionresolver.go | 5 + internal/engine/netx/dialer/bytecounter.go | 3 +- .../engine/netx/dialer/bytecounter_test.go | 2 +- internal/engine/netx/dialer/dialer.go | 24 +-- internal/engine/netx/dialer/dialer_test.go | 15 +- internal/engine/netx/dialer/example_test.go | 4 +- .../engine/netx/dialer/integration_test.go | 4 +- internal/engine/netx/dialer/proxy.go | 5 +- internal/engine/netx/dialer/saver.go | 9 +- .../engine/netx/dialer/shaping_disabled.go | 4 +- .../engine/netx/dialer/shaping_enabled.go | 4 +- internal/engine/netx/dialer/shaping_test.go | 5 +- internal/engine/netx/fake_test.go | 2 + .../engine/netx/httptransport/bytecounter.go | 7 +- .../netx/httptransport/bytecounter_test.go | 6 +- .../netx/httptransport/http3transport.go | 6 +- .../netx/httptransport/httptransport.go | 39 +---- internal/engine/netx/httptransport/saver.go | 25 +-- .../engine/netx/httptransport/saver_test.go | 28 ++-- internal/engine/netx/httptransport/system.go | 6 +- internal/engine/netx/netx.go | 91 ++++------- internal/engine/netx/netx_test.go | 148 +++++------------ internal/engine/netx/quicdialer/quicdialer.go | 20 --- internal/engine/netx/quicdialer/saver.go | 7 +- internal/engine/netx/quicdialer/saver_test.go | 7 +- internal/engine/netx/quicdialer/system.go | 8 +- internal/engine/netx/resolver/address.go | 9 -- internal/engine/netx/resolver/bogon.go | 5 +- internal/engine/netx/resolver/cache.go | 4 +- internal/engine/netx/resolver/cache_test.go | 9 +- internal/engine/netx/resolver/chain.go | 24 ++- internal/engine/netx/resolver/fake_test.go | 11 +- internal/engine/netx/resolver/idna.go | 11 -- .../engine/netx/resolver/integration_test.go | 15 +- internal/engine/netx/resolver/resolver.go | 18 --- internal/engine/netx/resolver/saver.go | 5 +- internal/engine/netx/tlsdialer/saver.go | 5 +- internal/engine/netx/tlsdialer/saver_test.go | 5 +- internal/engine/netx/tlsdialer/tls.go | 19 --- internal/engine/session.go | 2 +- internal/netxlite/legacy.go | 149 ------------------ internal/netxlite/legacy_test.go | 100 ------------ 76 files changed, 328 insertions(+), 736 deletions(-) delete mode 100644 internal/engine/netx/quicdialer/quicdialer.go delete mode 100644 internal/engine/netx/resolver/address.go delete mode 100644 internal/engine/netx/resolver/idna.go delete mode 100644 internal/engine/netx/resolver/resolver.go delete mode 100644 internal/engine/netx/tlsdialer/tls.go delete mode 100644 internal/netxlite/legacy_test.go diff --git a/internal/cmd/jafar/uncensored/uncensored.go b/internal/cmd/jafar/uncensored/uncensored.go index ab43125..c16cd9d 100644 --- a/internal/cmd/jafar/uncensored/uncensored.go +++ b/internal/cmd/jafar/uncensored/uncensored.go @@ -4,20 +4,22 @@ package uncensored import ( "context" + "errors" "net" "net/http" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) // Client is DNS, HTTP, and TCP client. type Client struct { dnsClient *netx.DNSClient - httpTransport netx.HTTPRoundTripper - dialer netx.Dialer + httpTransport model.HTTPTransport + dialer model.Dialer } // NewClient creates a new Client. @@ -48,7 +50,7 @@ func Must(client *Client, err error) *Client { // DefaultClient is the default client for DNS, HTTP, and TCP. var DefaultClient = Must(NewClient("")) -var _ netx.Resolver = DefaultClient +var _ model.Resolver = DefaultClient // Address implements netx.Resolver.Address func (c *Client) Address() string { @@ -60,19 +62,24 @@ func (c *Client) LookupHost(ctx context.Context, domain string) ([]string, error return c.dnsClient.LookupHost(ctx, domain) } +// LookupHTTPS implements model.Resolver.LookupHTTPS. +func (c *Client) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("not implemented") +} + // Network implements netx.Resolver.Network func (c *Client) Network() string { return c.dnsClient.Network() } -var _ netx.Dialer = DefaultClient +var _ model.Dialer = DefaultClient // DialContext implements netx.Dialer.DialContext func (c *Client) DialContext(ctx context.Context, network, address string) (net.Conn, error) { return c.dialer.DialContext(ctx, network, address) } -var _ netx.HTTPRoundTripper = DefaultClient +var _ model.HTTPTransport = DefaultClient // CloseIdleConnections implement netx.HTTPRoundTripper.CloseIdleConnections func (c *Client) CloseIdleConnections() { diff --git a/internal/cmd/oohelper/internal/fake_test.go b/internal/cmd/oohelper/internal/fake_test.go index e6687e5..4553664 100644 --- a/internal/cmd/oohelper/internal/fake_test.go +++ b/internal/cmd/oohelper/internal/fake_test.go @@ -2,13 +2,14 @@ package internal import ( "context" + "errors" "io" "net" "net/http" "time" "github.com/ooni/probe-cli/v3/internal/atomicx" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -49,7 +50,13 @@ func (c FakeResolver) Address() string { return "" } -var _ netx.Resolver = FakeResolver{} +func (c FakeResolver) CloseIdleConnections() {} + +func (c FakeResolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("not implemented") +} + +var _ model.Resolver = FakeResolver{} type FakeTransport struct { Err error @@ -75,7 +82,7 @@ func (txp FakeTransport) RoundTrip(req *http.Request) (*http.Response, error) { func (txp FakeTransport) CloseIdleConnections() {} -var _ netx.HTTPRoundTripper = FakeTransport{} +var _ model.HTTPTransport = FakeTransport{} type FakeBody struct { Data []byte diff --git a/internal/cmd/oohelper/oohelper.go b/internal/cmd/oohelper/oohelper.go index 12d54a1..40c2c34 100644 --- a/internal/cmd/oohelper/oohelper.go +++ b/internal/cmd/oohelper/oohelper.go @@ -14,6 +14,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/webstepsx" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/measurex" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -21,7 +22,7 @@ var ( ctx, cancel = context.WithCancel(context.Background()) debug = flag.Bool("debug", false, "Toggle debug mode") httpClient *http.Client - resolver netx.Resolver + resolver model.Resolver server = flag.String("server", "", "URL of the test helper") target = flag.String("target", "", "Target URL for the test helper") fwebsteps = flag.Bool("websteps", false, "Use the websteps TH") diff --git a/internal/cmd/oohelperd/internal/webconnectivity/dns.go b/internal/cmd/oohelperd/internal/webconnectivity/dns.go index 1125958..ebe7864 100644 --- a/internal/cmd/oohelperd/internal/webconnectivity/dns.go +++ b/internal/cmd/oohelperd/internal/webconnectivity/dns.go @@ -5,8 +5,8 @@ import ( "sync" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -21,7 +21,7 @@ type CtrlDNSResult = webconnectivity.ControlDNSResult type DNSConfig struct { Domain string Out chan CtrlDNSResult - Resolver netx.Resolver + Resolver model.Resolver Wg *sync.WaitGroup } diff --git a/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go b/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go index 7bcab42..d4e6e72 100644 --- a/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go +++ b/internal/cmd/oohelperd/internal/webconnectivity/fake_test.go @@ -2,13 +2,14 @@ package webconnectivity import ( "context" + "errors" "io" "net" "net/http" "time" "github.com/ooni/probe-cli/v3/internal/atomicx" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -49,7 +50,13 @@ func (c FakeResolver) Address() string { return "" } -var _ netx.Resolver = FakeResolver{} +func (c FakeResolver) CloseIdleConnections() {} + +func (c FakeResolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("not implemented") +} + +var _ model.Resolver = FakeResolver{} type FakeTransport struct { Err error @@ -75,7 +82,7 @@ func (txp FakeTransport) RoundTrip(req *http.Request) (*http.Response, error) { func (txp FakeTransport) CloseIdleConnections() {} -var _ netx.HTTPRoundTripper = FakeTransport{} +var _ model.HTTPTransport = FakeTransport{} type FakeBody struct { Data []byte diff --git a/internal/cmd/oohelperd/internal/webconnectivity/measure.go b/internal/cmd/oohelperd/internal/webconnectivity/measure.go index 6be2cd8..ccd218b 100644 --- a/internal/cmd/oohelperd/internal/webconnectivity/measure.go +++ b/internal/cmd/oohelperd/internal/webconnectivity/measure.go @@ -8,7 +8,7 @@ import ( "sync" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" ) type ( @@ -22,9 +22,9 @@ type ( // MeasureConfig contains configuration for Measure. type MeasureConfig struct { Client *http.Client - Dialer netx.Dialer + Dialer model.Dialer MaxAcceptableBody int64 - Resolver netx.Resolver + Resolver model.Resolver } // Measure performs the measurement described by the request and diff --git a/internal/cmd/oohelperd/internal/webconnectivity/tcpconnect.go b/internal/cmd/oohelperd/internal/webconnectivity/tcpconnect.go index 6a58be7..a741a97 100644 --- a/internal/cmd/oohelperd/internal/webconnectivity/tcpconnect.go +++ b/internal/cmd/oohelperd/internal/webconnectivity/tcpconnect.go @@ -5,7 +5,7 @@ import ( "sync" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -20,7 +20,7 @@ type TCPResultPair struct { // TCPConfig configures the TCP connect check. type TCPConfig struct { - Dialer netx.Dialer + Dialer model.Dialer Endpoint string Out chan TCPResultPair Wg *sync.WaitGroup diff --git a/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity.go b/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity.go index 44a855f..947241a 100644 --- a/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity.go +++ b/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity.go @@ -6,7 +6,7 @@ import ( "io" "net/http" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/version" @@ -15,9 +15,9 @@ import ( // Handler implements the Web Connectivity test helper HTTP API. type Handler struct { Client *http.Client - Dialer netx.Dialer + Dialer model.Dialer MaxAcceptableBody int64 - Resolver netx.Resolver + Resolver model.Resolver } func (h Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { diff --git a/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity_test.go b/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity_test.go index a4881df..8adc62f 100644 --- a/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity_test.go +++ b/internal/cmd/oohelperd/internal/webconnectivity/webconnectivity_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "net" "net/http" "net/http/httptest" "strings" @@ -52,7 +51,7 @@ const requestWithoutDomainName = `{ func TestWorkingAsIntended(t *testing.T) { handler := Handler{ Client: http.DefaultClient, - Dialer: new(net.Dialer), + Dialer: netxlite.DefaultDialer, MaxAcceptableBody: 1 << 24, Resolver: &netxlite.ResolverSystem{}, } diff --git a/internal/cmd/oohelperd/internal/websteps/explore.go b/internal/cmd/oohelperd/internal/websteps/explore.go index 9dad1b5..9fc1c17 100644 --- a/internal/cmd/oohelperd/internal/websteps/explore.go +++ b/internal/cmd/oohelperd/internal/websteps/explore.go @@ -10,6 +10,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/experiment/websteps" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" utls "gitlab.com/yawning/utls.git" @@ -28,7 +29,7 @@ type Explorer interface { // DefaultExplorer is the default Explorer. type DefaultExplorer struct { - resolver netxlite.ResolverLegacy + resolver model.Resolver } // Explore returns a list of round trips sorted so that the first @@ -138,8 +139,7 @@ func (e *DefaultExplorer) getH3(h3URL *h3URL, headers map[string][]string) (*htt } // Rationale for using log.Log here: we're already using log.Log // in this package, so it seems fair to use it also here - transport := netxlite.NewHTTP3Transport(log.Log, - netxlite.NewQUICDialerFromContextDialerAdapter(dialer), tlsConf) + transport := netxlite.NewHTTP3Transport(log.Log, dialer, tlsConf) // TODO(bassosimone): here we should use runtimex.PanicOnError jarjar, _ := cookiejar.New(nil) clnt := &http.Client{ diff --git a/internal/cmd/oohelperd/internal/websteps/generate.go b/internal/cmd/oohelperd/internal/websteps/generate.go index dee55c9..246acc1 100644 --- a/internal/cmd/oohelperd/internal/websteps/generate.go +++ b/internal/cmd/oohelperd/internal/websteps/generate.go @@ -8,7 +8,7 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/engine/experiment/websteps" - "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/model" ) // Generate is the third step of the algorithm. Given the @@ -22,9 +22,9 @@ type Generator interface { // DefaultGenerator is the default Generator. type DefaultGenerator struct { - dialer netxlite.DialerLegacy - quicDialer netxlite.QUICContextDialer - resolver netxlite.ResolverLegacy + dialer model.Dialer + quicDialer model.QUICDialer + resolver model.Resolver transport http.RoundTripper } diff --git a/internal/cmd/oohelperd/internal/websteps/generate_test.go b/internal/cmd/oohelperd/internal/websteps/generate_test.go index 8242e13..6fb35be 100644 --- a/internal/cmd/oohelperd/internal/websteps/generate_test.go +++ b/internal/cmd/oohelperd/internal/websteps/generate_test.go @@ -36,6 +36,8 @@ func (d fakeQUICDialer) DialContext(ctx context.Context, network, address string return nil, d.err } +func (d fakeQUICDialer) CloseIdleConnections() {} + type fakeDialer struct { err error } @@ -44,6 +46,8 @@ func (d fakeDialer) DialContext(ctx context.Context, network, address string) (n return nil, d.err } +func (d fakeDialer) CloseIdleConnections() {} + func TestGenerateDNSFailure(t *testing.T) { u, err := url.Parse("https://www.google.google") runtimex.PanicOnError(err, "url.Parse failed") diff --git a/internal/cmd/oohelperd/internal/websteps/initialchecks.go b/internal/cmd/oohelperd/internal/websteps/initialchecks.go index 183a243..a0d058b 100644 --- a/internal/cmd/oohelperd/internal/websteps/initialchecks.go +++ b/internal/cmd/oohelperd/internal/websteps/initialchecks.go @@ -5,7 +5,7 @@ import ( "errors" "net/url" - "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/model" ) // InitialChecks is the first step of the test helper algorithm. We @@ -31,7 +31,7 @@ type InitChecker interface { // DefaultInitChecker is the default InitChecker. type DefaultInitChecker struct { - resolver netxlite.ResolverLegacy + resolver model.Resolver } // InitialChecks checks whether the URL is valid and whether the diff --git a/internal/cmd/oohelperd/internal/websteps/measure.go b/internal/cmd/oohelperd/internal/websteps/measure.go index 601fb27..da0bb3c 100644 --- a/internal/cmd/oohelperd/internal/websteps/measure.go +++ b/internal/cmd/oohelperd/internal/websteps/measure.go @@ -8,6 +8,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/experiment/websteps" "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -24,7 +25,7 @@ type Config struct { checker InitChecker explorer Explorer generator Generator - resolver netxlite.ResolverLegacy + resolver model.Resolver } // Measure performs the three consecutive steps of the testhelper algorithm: @@ -90,12 +91,10 @@ func newDNSFailedResponse(err error, URL string) *ControlResponse { } // newResolver creates a new DNS resolver instance -func newResolver() netxlite.ResolverLegacy { +func newResolver() model.Resolver { childResolver, err := netx.NewDNSClient(netx.Config{Logger: log.Log}, "doh://google") runtimex.PanicOnError(err, "NewDNSClient failed") - var r netxlite.ResolverLegacy = childResolver - r = &netxlite.ResolverIDNA{ - Resolver: netxlite.NewResolverLegacyAdapter(r), - } + var r model.Resolver = childResolver + r = &netxlite.ResolverIDNA{Resolver: r} return r } diff --git a/internal/cmd/oohelperd/oohelperd.go b/internal/cmd/oohelperd/oohelperd.go index 2e095b5..8f89b96 100644 --- a/internal/cmd/oohelperd/oohelperd.go +++ b/internal/cmd/oohelperd/oohelperd.go @@ -13,16 +13,17 @@ import ( "github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal/websteps" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webstepsx" "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) const maxAcceptableBody = 1 << 24 var ( - dialer netx.Dialer + dialer model.Dialer endpoint = flag.String("endpoint", ":8080", "Endpoint where to listen") httpx *http.Client - resolver netx.Resolver + resolver model.Resolver srvcancel context.CancelFunc srvctx context.Context srvwg = new(sync.WaitGroup) diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index 6b4bf12..87be9be 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -286,8 +286,8 @@ func (e *Experiment) OpenReportContext(ctx context.Context) error { // use custom client to have proper byte accounting httpClient := &http.Client{ Transport: &httptransport.ByteCountingTransport{ - RoundTripper: e.session.httpDefaultTransport, // proxy is OK - Counter: e.byteCounter, + HTTPTransport: e.session.httpDefaultTransport, // proxy is OK + Counter: e.byteCounter, }, } client, err := e.session.NewProbeServicesClient(ctx) diff --git a/internal/engine/experiment/dnscheck/dnscheck.go b/internal/engine/experiment/dnscheck/dnscheck.go index 8c04c08..ade7c1e 100644 --- a/internal/engine/experiment/dnscheck/dnscheck.go +++ b/internal/engine/experiment/dnscheck/dnscheck.go @@ -234,7 +234,7 @@ func (m *Measurer) Run( return nil } -func (m *Measurer) lookupHost(ctx context.Context, hostname string, r netx.Resolver) ([]string, error) { +func (m *Measurer) lookupHost(ctx context.Context, hostname string, r model.Resolver) ([]string, error) { ctx, cancel := context.WithTimeout(ctx, 20*time.Second) defer cancel() return r.LookupHost(ctx, hostname) diff --git a/internal/engine/experiment/hhfm/hhfm.go b/internal/engine/experiment/hhfm/hhfm.go index 59fddf4..aa7a5c1 100644 --- a/internal/engine/experiment/hhfm/hhfm.go +++ b/internal/engine/experiment/hhfm/hhfm.go @@ -17,7 +17,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/httpheader" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/randx" @@ -311,7 +310,7 @@ type JSONHeaders struct { // guarantee that the connection is used for a single request and that // such a request does not contain any body. type Dialer struct { - Dialer dialer.Dialer // used for testing + Dialer model.SimpleDialer // used for testing Headers map[string]string } diff --git a/internal/engine/experiment/hirl/fake_test.go b/internal/engine/experiment/hirl/fake_test.go index e6ee632..967556b 100644 --- a/internal/engine/experiment/hirl/fake_test.go +++ b/internal/engine/experiment/hirl/fake_test.go @@ -17,6 +17,8 @@ func (d FakeDialer) DialContext(ctx context.Context, network, address string) (n return d.Conn, d.Err } +func (d FakeDialer) CloseIdleConnections() {} + type FakeConn struct { ReadError error ReadData []byte diff --git a/internal/engine/experiment/hirl/hirl.go b/internal/engine/experiment/hirl/hirl.go index b63501e..3f55429 100644 --- a/internal/engine/experiment/hirl/hirl.go +++ b/internal/engine/experiment/hirl/hirl.go @@ -251,7 +251,7 @@ func (meth squidCacheManager) Run(ctx context.Context, config MethodConfig) { type RunMethodConfig struct { MethodConfig Name string - NewDialer func(config netx.Config) netx.Dialer + NewDialer func(config netx.Config) model.Dialer RequestLine string } diff --git a/internal/engine/experiment/hirl/hirl_test.go b/internal/engine/experiment/hirl/hirl_test.go index 6c4afec..9766819 100644 --- a/internal/engine/experiment/hirl/hirl_test.go +++ b/internal/engine/experiment/hirl/hirl_test.go @@ -388,7 +388,7 @@ func TestRunMethodDialFailure(t *testing.T) { Out: out, }, Name: "random_invalid_version_number", - NewDialer: func(config netx.Config) netx.Dialer { + NewDialer: func(config netx.Config) model.Dialer { return FakeDialer{Err: expected} }, RequestLine: "GET / HTTP/ABC", @@ -435,7 +435,7 @@ func TestRunMethodSetDeadlineFailure(t *testing.T) { Out: out, }, Name: "random_invalid_version_number", - NewDialer: func(config netx.Config) netx.Dialer { + NewDialer: func(config netx.Config) model.Dialer { return FakeDialer{Conn: &FakeConn{ SetDeadlineError: expected, }} @@ -484,7 +484,7 @@ func TestRunMethodWriteFailure(t *testing.T) { Out: out, }, Name: "random_invalid_version_number", - NewDialer: func(config netx.Config) netx.Dialer { + NewDialer: func(config netx.Config) model.Dialer { return FakeDialer{Conn: &FakeConn{ WriteError: expected, }} @@ -532,7 +532,7 @@ func TestRunMethodReadEOFWithWrongData(t *testing.T) { Out: out, }, Name: "random_invalid_version_number", - NewDialer: func(config netx.Config) netx.Dialer { + NewDialer: func(config netx.Config) model.Dialer { return FakeDialer{Conn: &FakeConn{ ReadData: []byte("0xdeadbeef"), }} diff --git a/internal/engine/experiment/ndt7/dial.go b/internal/engine/experiment/ndt7/dial.go index 224cc35..520c2a8 100644 --- a/internal/engine/experiment/ndt7/dial.go +++ b/internal/engine/experiment/ndt7/dial.go @@ -8,7 +8,6 @@ import ( "github.com/gorilla/websocket" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" - "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -34,9 +33,9 @@ func newDialManager(ndt7URL string, logger model.Logger, userAgent string) dialM } func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*websocket.Conn, error) { - var reso resolver.Resolver = &netxlite.ResolverSystem{} + var reso model.Resolver = &netxlite.ResolverSystem{} reso = &netxlite.ResolverLogger{ - Resolver: netxlite.NewResolverLegacyAdapter(reso), + Resolver: reso, Logger: mgr.logger, } dlr := dialer.New(&dialer.Config{ diff --git a/internal/engine/experiment/stunreachability/stunreachability.go b/internal/engine/experiment/stunreachability/stunreachability.go index 8caf333..4a3f000 100644 --- a/internal/engine/experiment/stunreachability/stunreachability.go +++ b/internal/engine/experiment/stunreachability/stunreachability.go @@ -13,7 +13,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" @@ -134,7 +133,7 @@ func (tk *TestKeys) run( } func (tk *TestKeys) do( - ctx context.Context, config Config, dialer dialer.Dialer, endpoint string) error { + ctx context.Context, config Config, dialer model.Dialer, endpoint string) error { dialContext := dialer.DialContext if config.dialContext != nil { dialContext = config.dialContext diff --git a/internal/engine/experiment/tlstool/internal/dialer.go b/internal/engine/experiment/tlstool/internal/dialer.go index 322f064..cf57f12 100644 --- a/internal/engine/experiment/tlstool/internal/dialer.go +++ b/internal/engine/experiment/tlstool/internal/dialer.go @@ -5,14 +5,14 @@ import ( "net" "time" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" ) // Dialer creates net.Conn instances where (1) we delay writes if // a delay is configured and (2) we split outgoing buffers if there // is a configured splitter function. type Dialer struct { - netx.Dialer + model.Dialer Delay time.Duration Splitter func([]byte) [][]byte } diff --git a/internal/engine/experiment/tlstool/internal/fake_test.go b/internal/engine/experiment/tlstool/internal/fake_test.go index 06ddb16..b3814ef 100644 --- a/internal/engine/experiment/tlstool/internal/fake_test.go +++ b/internal/engine/experiment/tlstool/internal/fake_test.go @@ -17,6 +17,8 @@ func (d FakeDialer) DialContext(ctx context.Context, network, address string) (n return d.Conn, d.Err } +func (d FakeDialer) CloseIdleConnections() {} + type FakeConn struct { ReadError error ReadData []byte diff --git a/internal/engine/experiment/tlstool/internal/internal.go b/internal/engine/experiment/tlstool/internal/internal.go index 7d17d2d..a4d4983 100644 --- a/internal/engine/experiment/tlstool/internal/internal.go +++ b/internal/engine/experiment/tlstool/internal/internal.go @@ -4,12 +4,12 @@ package internal import ( "time" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" ) // DialerConfig contains the config for creating a dialer type DialerConfig struct { - Dialer netx.Dialer + Dialer model.Dialer Delay time.Duration SNI string } diff --git a/internal/engine/experiment/tlstool/internal/internal_test.go b/internal/engine/experiment/tlstool/internal/internal_test.go index 779e307..b5b6df0 100644 --- a/internal/engine/experiment/tlstool/internal/internal_test.go +++ b/internal/engine/experiment/tlstool/internal/internal_test.go @@ -6,6 +6,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/tlstool/internal" "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" ) var config = internal.DialerConfig{ @@ -14,7 +15,7 @@ var config = internal.DialerConfig{ SNI: "dns.google", } -func dial(t *testing.T, d netx.Dialer) { +func dial(t *testing.T, d model.Dialer) { td := netx.NewTLSDialer(netx.Config{Dialer: d}) conn, err := td.DialTLSContext(context.Background(), "tcp", "dns.google:853") if err != nil { diff --git a/internal/engine/experiment/tlstool/tlstool.go b/internal/engine/experiment/tlstool/tlstool.go index 4416d41..cf63812 100644 --- a/internal/engine/experiment/tlstool/tlstool.go +++ b/internal/engine/experiment/tlstool/tlstool.go @@ -107,7 +107,7 @@ func (m Measurer) Run( return nil // return nil so we always submit the measurement } -func (m Measurer) newDialer(logger model.Logger) netx.Dialer { +func (m Measurer) newDialer(logger model.Logger) model.Dialer { // TODO(bassosimone): this is a resolver that should hopefully work // in many places. Maybe allow to configure it? resolver, err := netx.NewDNSClientWithOverrides(netx.Config{Logger: logger}, diff --git a/internal/engine/experiment/websteps/dns.go b/internal/engine/experiment/websteps/dns.go index c07531f..650e508 100644 --- a/internal/engine/experiment/websteps/dns.go +++ b/internal/engine/experiment/websteps/dns.go @@ -5,13 +5,14 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" ) type DNSConfig struct { Domain string - Resolver netxlite.ResolverLegacy + Resolver model.Resolver } // DNSDo performs the DNS check. @@ -22,7 +23,7 @@ func DNSDo(ctx context.Context, config DNSConfig) ([]string, error) { runtimex.PanicOnError(err, "NewDNSClient failed") resolver = childResolver resolver = &netxlite.ResolverIDNA{ - Resolver: netxlite.NewResolverLegacyAdapter(resolver), + Resolver: resolver, } } return resolver.LookupHost(ctx, config.Domain) diff --git a/internal/engine/experiment/websteps/factory.go b/internal/engine/experiment/websteps/factory.go index d75fc52..fb785b8 100644 --- a/internal/engine/experiment/websteps/factory.go +++ b/internal/engine/experiment/websteps/factory.go @@ -32,11 +32,11 @@ func NewRequest(ctx context.Context, URL *url.URL, headers http.Header) *http.Re // NewDialerResolver contructs a new dialer for TCP connections, // with default, errorwrapping and resolve functionalities -func NewDialerResolver(resolver netxlite.ResolverLegacy) model.Dialer { +func NewDialerResolver(resolver model.Resolver) model.Dialer { var d model.Dialer = netxlite.DefaultDialer d = &netxlite.ErrorWrapperDialer{Dialer: d} d = &netxlite.DialerResolver{ - Resolver: netxlite.NewResolverLegacyAdapter(resolver), + Resolver: resolver, Dialer: d, } return d @@ -44,7 +44,7 @@ func NewDialerResolver(resolver netxlite.ResolverLegacy) model.Dialer { // NewQUICDialerResolver creates a new QUICDialerResolver // with default, errorwrapping and resolve functionalities -func NewQUICDialerResolver(resolver netxlite.ResolverLegacy) model.QUICDialer { +func NewQUICDialerResolver(resolver model.Resolver) model.QUICDialer { var ql model.QUICListener = &netxlite.QUICListenerStdlib{} ql = &netxlite.ErrorWrapperQUICListener{QUICListener: ql} var dialer model.QUICDialer = &netxlite.QUICDialerQUICGo{ @@ -52,7 +52,7 @@ func NewQUICDialerResolver(resolver netxlite.ResolverLegacy) model.QUICDialer { } dialer = &netxlite.ErrorWrapperQUICDialer{QUICDialer: dialer} dialer = &netxlite.QUICDialerResolver{ - Resolver: netxlite.NewResolverLegacyAdapter(resolver), + Resolver: resolver, Dialer: dialer, } return dialer @@ -79,12 +79,12 @@ func NewSingleTransport(conn net.Conn) http.RoundTripper { } // NewSingleTransport creates a new HTTP transport with a custom dialer and handshaker. -func NewTransportWithDialer(dialer netxlite.DialerLegacy, tlsConfig *tls.Config, handshaker model.TLSHandshaker) http.RoundTripper { +func NewTransportWithDialer(dialer model.Dialer, tlsConfig *tls.Config, handshaker model.TLSHandshaker) http.RoundTripper { transport := newBaseTransport() transport.DialContext = dialer.DialContext transport.DialTLSContext = (&netxlite.TLSDialerLegacy{ Config: tlsConfig, - Dialer: netxlite.NewDialerLegacyAdapter(dialer), + Dialer: dialer, TLSHandshaker: handshaker, }).DialTLSContext return transport diff --git a/internal/engine/experiment/websteps/quic.go b/internal/engine/experiment/websteps/quic.go index 70a4b04..021217d 100644 --- a/internal/engine/experiment/websteps/quic.go +++ b/internal/engine/experiment/websteps/quic.go @@ -5,13 +5,14 @@ import ( "crypto/tls" "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) type QUICConfig struct { Endpoint string - QUICDialer netxlite.QUICContextDialer - Resolver netxlite.ResolverLegacy + QUICDialer model.QUICDialer + Resolver model.Resolver TLSConf *tls.Config } diff --git a/internal/engine/experiment/websteps/tcp.go b/internal/engine/experiment/websteps/tcp.go index a8b34c7..a0bf7e5 100644 --- a/internal/engine/experiment/websteps/tcp.go +++ b/internal/engine/experiment/websteps/tcp.go @@ -4,13 +4,14 @@ import ( "context" "net" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) type TCPConfig struct { - Dialer netxlite.DialerLegacy + Dialer model.Dialer Endpoint string - Resolver netxlite.ResolverLegacy + Resolver model.Resolver } // TCPDo performs the TCP check. diff --git a/internal/engine/geolocate/geolocate.go b/internal/engine/geolocate/geolocate.go index ca81cba..d21af5a 100644 --- a/internal/engine/geolocate/geolocate.go +++ b/internal/engine/geolocate/geolocate.go @@ -89,19 +89,12 @@ type resolverIPLookupper interface { LookupResolverIP(ctx context.Context) (addr string, err error) } -// Resolver is a DNS resolver. -type Resolver interface { - LookupHost(ctx context.Context, domain string) ([]string, error) - Network() string - Address() string -} - // Config contains configuration for a geolocate Task. type Config struct { // Resolver is the resolver we should use when // making requests for discovering the IP. When // this field is not set, we use the stdlib. - Resolver Resolver + Resolver model.Resolver // Logger is the logger to use. If not set, then we will // use a logger that discards all messages. diff --git a/internal/engine/geolocate/iplookup.go b/internal/engine/geolocate/iplookup.go index 03048e9..4cb46f0 100644 --- a/internal/engine/geolocate/iplookup.go +++ b/internal/engine/geolocate/iplookup.go @@ -65,7 +65,7 @@ var ( type ipLookupClient struct { // Resolver is the resolver to use for HTTP. - Resolver Resolver + Resolver model.Resolver // Logger is the logger to use Logger model.Logger diff --git a/internal/engine/internal/sessionresolver/sessionresolver.go b/internal/engine/internal/sessionresolver/sessionresolver.go index 48f930f..d947d2a 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver.go +++ b/internal/engine/internal/sessionresolver/sessionresolver.go @@ -110,6 +110,11 @@ func (r *Resolver) Stats() string { return fmt.Sprintf("sessionresolver: %s", string(data)) } +// LookupHTTPS implements Resolver.LookupHTTPS. +func (r *Resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("not implemented") +} + // ErrLookupHost indicates that LookupHost failed. var ErrLookupHost = errors.New("sessionresolver: LookupHost failed") diff --git a/internal/engine/netx/dialer/bytecounter.go b/internal/engine/netx/dialer/bytecounter.go index 41476f1..4544510 100644 --- a/internal/engine/netx/dialer/bytecounter.go +++ b/internal/engine/netx/dialer/bytecounter.go @@ -5,12 +5,13 @@ import ( "net" "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/model" ) // byteCounterDialer is a byte-counting-aware dialer. To perform byte counting, you // should make sure that you insert this dialer in the dialing chain. type byteCounterDialer struct { - Dialer + model.Dialer } // DialContext implements Dialer.DialContext diff --git a/internal/engine/netx/dialer/bytecounter_test.go b/internal/engine/netx/dialer/bytecounter_test.go index b1e251f..95a8eae 100644 --- a/internal/engine/netx/dialer/bytecounter_test.go +++ b/internal/engine/netx/dialer/bytecounter_test.go @@ -16,7 +16,7 @@ import ( func dorequest(ctx context.Context, url string) error { txp := http.DefaultTransport.(*http.Transport).Clone() defer txp.CloseIdleConnections() - dialer := &byteCounterDialer{Dialer: new(net.Dialer)} + dialer := &byteCounterDialer{Dialer: netxlite.DefaultDialer} txp.DialContext = dialer.DialContext client := &http.Client{Transport: txp} req, err := http.NewRequestWithContext(ctx, "GET", "http://www.google.com", nil) diff --git a/internal/engine/netx/dialer/dialer.go b/internal/engine/netx/dialer/dialer.go index e704009..e0df83e 100644 --- a/internal/engine/netx/dialer/dialer.go +++ b/internal/engine/netx/dialer/dialer.go @@ -1,8 +1,6 @@ package dialer import ( - "context" - "net" "net/url" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" @@ -10,18 +8,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite" ) -// Dialer establishes network connections. -type Dialer interface { - // DialContext behaves like net.Dialer.DialContext. - DialContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// Resolver is the interface we expect from a DNS resolver. -type Resolver interface { - // LookupHost behaves like net.Resolver.LookupHost. - LookupHost(ctx context.Context, hostname string) (addrs []string, err error) -} - // Config contains the settings for New. type Config struct { // ContextByteCounting optionally configures context-based @@ -59,11 +45,11 @@ type Config struct { } // New creates a new Dialer from the specified config and resolver. -func New(config *Config, resolver Resolver) Dialer { - var d Dialer = &netxlite.ErrorWrapperDialer{Dialer: netxlite.DefaultDialer} +func New(config *Config, resolver model.Resolver) model.Dialer { + var d model.Dialer = &netxlite.ErrorWrapperDialer{Dialer: netxlite.DefaultDialer} if config.Logger != nil { d = &netxlite.DialerLogger{ - Dialer: netxlite.NewDialerLegacyAdapter(d), + Dialer: d, DebugLogger: config.Logger, } } @@ -74,8 +60,8 @@ func New(config *Config, resolver Resolver) Dialer { d = &saverConnDialer{Dialer: d, Saver: config.ReadWriteSaver} } d = &netxlite.DialerResolver{ - Resolver: netxlite.NewResolverLegacyAdapter(resolver), - Dialer: netxlite.NewDialerLegacyAdapter(d), + Resolver: resolver, + Dialer: d, } d = &proxyDialer{ProxyURL: config.ProxyURL, Dialer: d} if config.ContextByteCounting { diff --git a/internal/engine/netx/dialer/dialer_test.go b/internal/engine/netx/dialer/dialer_test.go index eab23f1..d53e1cf 100644 --- a/internal/engine/netx/dialer/dialer_test.go +++ b/internal/engine/netx/dialer/dialer_test.go @@ -1,7 +1,6 @@ package dialer import ( - "net" "net/url" "testing" @@ -18,7 +17,7 @@ func TestNewCreatesTheExpectedChain(t *testing.T) { Logger: log.Log, ProxyURL: &url.URL{}, ReadWriteSaver: saver, - }, &net.Resolver{}) + }, netxlite.DefaultResolver) shd, ok := dlr.(*shapingDialer) if !ok { t.Fatal("not a shapingDialer") @@ -35,11 +34,7 @@ func TestNewCreatesTheExpectedChain(t *testing.T) { if !ok { t.Fatal("not a dnsDialer") } - dad, ok := dnsd.Dialer.(*netxlite.DialerLegacyAdapter) - if !ok { - t.Fatal("invalid type") - } - scd, ok := dad.DialerLegacy.(*saverConnDialer) + scd, ok := dnsd.Dialer.(*saverConnDialer) if !ok { t.Fatal("not a saverConnDialer") } @@ -51,11 +46,7 @@ func TestNewCreatesTheExpectedChain(t *testing.T) { if !ok { t.Fatal("not a loggingDialer") } - dad, ok = ld.Dialer.(*netxlite.DialerLegacyAdapter) - if !ok { - t.Fatal("invalid type") - } - ewd, ok := dad.DialerLegacy.(*netxlite.ErrorWrapperDialer) + ewd, ok := ld.Dialer.(*netxlite.ErrorWrapperDialer) if !ok { t.Fatal("not an errorWrappingDialer") } diff --git a/internal/engine/netx/dialer/example_test.go b/internal/engine/netx/dialer/example_test.go index c141354..f505ec0 100644 --- a/internal/engine/netx/dialer/example_test.go +++ b/internal/engine/netx/dialer/example_test.go @@ -2,11 +2,11 @@ package dialer_test import ( "context" - "net" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func Example() { @@ -16,7 +16,7 @@ func Example() { DialSaver: saver, Logger: log.Log, ReadWriteSaver: saver, - }, &net.Resolver{}) + }, netxlite.DefaultResolver) ctx := context.Background() conn, err := dlr.DialContext(ctx, "tcp", "8.8.8.8:53") diff --git a/internal/engine/netx/dialer/integration_test.go b/internal/engine/netx/dialer/integration_test.go index aa26b82..ed691ea 100644 --- a/internal/engine/netx/dialer/integration_test.go +++ b/internal/engine/netx/dialer/integration_test.go @@ -1,12 +1,12 @@ package dialer_test import ( - "net" "net/http" "testing" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestDialerNewSuccess(t *testing.T) { @@ -14,7 +14,7 @@ func TestDialerNewSuccess(t *testing.T) { t.Skip("skip test in short mode") } log.SetLevel(log.DebugLevel) - d := dialer.New(&dialer.Config{Logger: log.Log}, &net.Resolver{}) + d := dialer.New(&dialer.Config{Logger: log.Log}, netxlite.DefaultResolver) txp := &http.Transport{DialContext: d.DialContext} client := &http.Client{Transport: txp} resp, err := client.Get("http://www.google.com") diff --git a/internal/engine/netx/dialer/proxy.go b/internal/engine/netx/dialer/proxy.go index ba4c768..384a903 100644 --- a/internal/engine/netx/dialer/proxy.go +++ b/internal/engine/netx/dialer/proxy.go @@ -6,6 +6,7 @@ import ( "net" "net/url" + "github.com/ooni/probe-cli/v3/internal/model" "golang.org/x/net/proxy" ) @@ -13,7 +14,7 @@ import ( // dialer is a passthrough for the next Dialer in chain. Otherwise, it will internally // create a SOCKS5 dialer that will connect to the proxy using the underlying Dialer. type proxyDialer struct { - Dialer + model.Dialer ProxyURL *url.URL } @@ -47,7 +48,7 @@ func (d *proxyDialer) dial( // // See https://git.io/JfJ4g. type proxyDialerWrapper struct { - Dialer + model.Dialer } func (d *proxyDialerWrapper) Dial(network, address string) (net.Conn, error) { diff --git a/internal/engine/netx/dialer/saver.go b/internal/engine/netx/dialer/saver.go index 8f9345c..42f2e5c 100644 --- a/internal/engine/netx/dialer/saver.go +++ b/internal/engine/netx/dialer/saver.go @@ -6,12 +6,13 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) // saverDialer saves events occurring during the dial type saverDialer struct { - Dialer + model.Dialer Saver *trace.Saver } @@ -34,7 +35,7 @@ func (d *saverDialer) DialContext(ctx context.Context, network, address string) // saverConnDialer wraps the returned connection such that we // collect all the read/write events that occur. type saverConnDialer struct { - Dialer + model.Dialer Saver *trace.Saver } @@ -82,6 +83,6 @@ func (c *saverConn) Write(p []byte) (int, error) { return count, err } -var _ Dialer = &saverDialer{} -var _ Dialer = &saverConnDialer{} +var _ model.Dialer = &saverDialer{} +var _ model.Dialer = &saverConnDialer{} var _ net.Conn = &saverConn{} diff --git a/internal/engine/netx/dialer/shaping_disabled.go b/internal/engine/netx/dialer/shaping_disabled.go index 6da82b3..28f1238 100644 --- a/internal/engine/netx/dialer/shaping_disabled.go +++ b/internal/engine/netx/dialer/shaping_disabled.go @@ -6,13 +6,15 @@ package dialer import ( "context" "net" + + "github.com/ooni/probe-cli/v3/internal/model" ) // shapingDialer ensures we don't use too much bandwidth // when using integration tests at GitHub. To select // the implementation with shaping use `-tags shaping`. type shapingDialer struct { - Dialer + model.Dialer } // DialContext implements Dialer.DialContext diff --git a/internal/engine/netx/dialer/shaping_enabled.go b/internal/engine/netx/dialer/shaping_enabled.go index a6368c5..5581ce7 100644 --- a/internal/engine/netx/dialer/shaping_enabled.go +++ b/internal/engine/netx/dialer/shaping_enabled.go @@ -7,13 +7,15 @@ import ( "context" "net" "time" + + "github.com/ooni/probe-cli/v3/internal/model" ) // shapingDialer ensures we don't use too much bandwidth // when using integration tests at GitHub. To select // the implementation with shaping use `-tags shaping`. type shapingDialer struct { - Dialer + model.Dialer } // DialContext implements Dialer.DialContext diff --git a/internal/engine/netx/dialer/shaping_test.go b/internal/engine/netx/dialer/shaping_test.go index ffad493..71e0401 100644 --- a/internal/engine/netx/dialer/shaping_test.go +++ b/internal/engine/netx/dialer/shaping_test.go @@ -1,13 +1,14 @@ package dialer import ( - "net" "net/http" "testing" + + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestShapingDialerGood(t *testing.T) { - d := &shapingDialer{Dialer: &net.Dialer{}} + d := &shapingDialer{Dialer: netxlite.DefaultDialer} txp := &http.Transport{DialContext: d.DialContext} client := &http.Client{Transport: txp} resp, err := client.Get("https://www.google.com/") diff --git a/internal/engine/netx/fake_test.go b/internal/engine/netx/fake_test.go index bdfcce2..a0d94d2 100644 --- a/internal/engine/netx/fake_test.go +++ b/internal/engine/netx/fake_test.go @@ -19,6 +19,8 @@ func (d FakeDialer) DialContext(ctx context.Context, network, address string) (n return d.Conn, d.Err } +func (d FakeDialer) CloseIdleConnections() {} + type FakeTransport struct { Err error Func func(*http.Request) (*http.Response, error) diff --git a/internal/engine/netx/httptransport/bytecounter.go b/internal/engine/netx/httptransport/bytecounter.go index 67ac40b..d950d28 100644 --- a/internal/engine/netx/httptransport/bytecounter.go +++ b/internal/engine/netx/httptransport/bytecounter.go @@ -5,11 +5,12 @@ import ( "net/http" "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/model" ) // ByteCountingTransport is a RoundTripper that counts bytes. type ByteCountingTransport struct { - RoundTripper + model.HTTPTransport Counter *bytecounter.Counter } @@ -20,7 +21,7 @@ func (txp ByteCountingTransport) RoundTrip(req *http.Request) (*http.Response, e ReadCloser: req.Body, Account: txp.Counter.CountBytesSent} } txp.estimateRequestMetadata(req) - resp, err := txp.RoundTripper.RoundTrip(req) + resp, err := txp.HTTPTransport.RoundTrip(req) if err != nil { return nil, err } @@ -70,4 +71,4 @@ func (r byteCountingBody) Read(p []byte) (int, error) { return count, err } -var _ RoundTripper = ByteCountingTransport{} +var _ model.HTTPTransport = ByteCountingTransport{} diff --git a/internal/engine/netx/httptransport/bytecounter_test.go b/internal/engine/netx/httptransport/bytecounter_test.go index eadf040..2c0c72f 100644 --- a/internal/engine/netx/httptransport/bytecounter_test.go +++ b/internal/engine/netx/httptransport/bytecounter_test.go @@ -17,7 +17,7 @@ func TestByteCounterFailure(t *testing.T) { counter := bytecounter.New() txp := httptransport.ByteCountingTransport{ Counter: counter, - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Err: io.EOF, }, } @@ -47,7 +47,7 @@ func TestByteCounterSuccess(t *testing.T) { counter := bytecounter.New() txp := httptransport.ByteCountingTransport{ Counter: counter, - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Resp: &http.Response{ Body: io.NopCloser(strings.NewReader("1234567")), Header: http.Header{ @@ -89,7 +89,7 @@ func TestByteCounterSuccessWithEOF(t *testing.T) { counter := bytecounter.New() txp := httptransport.ByteCountingTransport{ Counter: counter, - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Resp: &http.Response{ Body: bodyReaderWithEOF{}, Header: http.Header{ diff --git a/internal/engine/netx/httptransport/http3transport.go b/internal/engine/netx/httptransport/http3transport.go index 70ef0d7..1aaf931 100644 --- a/internal/engine/netx/httptransport/http3transport.go +++ b/internal/engine/netx/httptransport/http3transport.go @@ -1,6 +1,7 @@ package httptransport import ( + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -9,12 +10,11 @@ import ( // Deprecation warning // // New code should use netxlite.NewHTTP3Transport instead. -func NewHTTP3Transport(config Config) RoundTripper { +func NewHTTP3Transport(config Config) model.HTTPTransport { // Rationale for using NoLogger here: previously this code did // not use a logger as well, so it's fine to keep it as is. return netxlite.NewHTTP3Transport(&NoLogger{}, - netxlite.NewQUICDialerFromContextDialerAdapter(config.QUICDialer), - config.TLSConfig) + config.QUICDialer, config.TLSConfig) } type NoLogger struct{} diff --git a/internal/engine/netx/httptransport/httptransport.go b/internal/engine/netx/httptransport/httptransport.go index 6378444..e1e71ba 100644 --- a/internal/engine/netx/httptransport/httptransport.go +++ b/internal/engine/netx/httptransport/httptransport.go @@ -2,46 +2,15 @@ package httptransport import ( - "context" "crypto/tls" - "net" - "net/http" - "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/model" ) // Config contains the configuration required for constructing an HTTP transport type Config struct { - Dialer Dialer - QUICDialer QUICDialer - TLSDialer TLSDialer + Dialer model.Dialer + QUICDialer model.QUICDialer + TLSDialer model.TLSDialer TLSConfig *tls.Config } - -// Dialer is the definition of dialer assumed by this package. -type Dialer interface { - DialContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// TLSDialer is the definition of a TLS dialer assumed by this package. -type TLSDialer interface { - DialTLSContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// QUICDialer is the definition of dialer for QUIC assumed by this package. -type QUICDialer interface { - DialContext(ctx context.Context, network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) -} - -// RoundTripper is the definition of http.RoundTripper used by this package. -type RoundTripper interface { - RoundTrip(req *http.Request) (*http.Response, error) - CloseIdleConnections() -} - -// Resolver is the interface we expect from a resolver -type Resolver interface { - LookupHost(ctx context.Context, hostname string) (addrs []string, err error) - Network() string - Address() string -} diff --git a/internal/engine/netx/httptransport/saver.go b/internal/engine/netx/httptransport/saver.go index c8b4534..3ee3d93 100644 --- a/internal/engine/netx/httptransport/saver.go +++ b/internal/engine/netx/httptransport/saver.go @@ -10,13 +10,14 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) // SaverPerformanceHTTPTransport is a RoundTripper that saves // performance events occurring during the round trip type SaverPerformanceHTTPTransport struct { - RoundTripper + model.HTTPTransport Saver *trace.Saver } @@ -38,13 +39,13 @@ func (txp SaverPerformanceHTTPTransport) RoundTrip(req *http.Request) (*http.Res } req = req.WithContext(httptrace.WithClientTrace(req.Context(), tracep)) } - return txp.RoundTripper.RoundTrip(req) + return txp.HTTPTransport.RoundTrip(req) } // SaverMetadataHTTPTransport is a RoundTripper that saves // events related to HTTP request and response metadata type SaverMetadataHTTPTransport struct { - RoundTripper + model.HTTPTransport Saver *trace.Saver Transport string } @@ -59,7 +60,7 @@ func (txp SaverMetadataHTTPTransport) RoundTrip(req *http.Request) (*http.Respon Name: "http_request_metadata", Time: time.Now(), }) - resp, err := txp.RoundTripper.RoundTrip(req) + resp, err := txp.HTTPTransport.RoundTrip(req) if err != nil { return nil, err } @@ -88,7 +89,7 @@ func (txp SaverMetadataHTTPTransport) CloneHeaders(req *http.Request) http.Heade // SaverTransactionHTTPTransport is a RoundTripper that saves // events related to the HTTP transaction type SaverTransactionHTTPTransport struct { - RoundTripper + model.HTTPTransport Saver *trace.Saver } @@ -98,7 +99,7 @@ func (txp SaverTransactionHTTPTransport) RoundTrip(req *http.Request) (*http.Res Name: "http_transaction_start", Time: time.Now(), }) - resp, err := txp.RoundTripper.RoundTrip(req) + resp, err := txp.HTTPTransport.RoundTrip(req) txp.Saver.Write(trace.Event{ Err: err, Name: "http_transaction_done", @@ -110,7 +111,7 @@ func (txp SaverTransactionHTTPTransport) RoundTrip(req *http.Request) (*http.Res // SaverBodyHTTPTransport is a RoundTripper that saves // body events occurring during the round trip type SaverBodyHTTPTransport struct { - RoundTripper + model.HTTPTransport Saver *trace.Saver SnapshotSize int } @@ -135,7 +136,7 @@ func (txp SaverBodyHTTPTransport) RoundTrip(req *http.Request) (*http.Response, Time: time.Now(), }) } - resp, err := txp.RoundTripper.RoundTrip(req) + resp, err := txp.HTTPTransport.RoundTrip(req) if err != nil { return nil, err } @@ -184,7 +185,7 @@ type saverReadCloser struct { io.Reader } -var _ RoundTripper = SaverPerformanceHTTPTransport{} -var _ RoundTripper = SaverMetadataHTTPTransport{} -var _ RoundTripper = SaverBodyHTTPTransport{} -var _ RoundTripper = SaverTransactionHTTPTransport{} +var _ model.HTTPTransport = SaverPerformanceHTTPTransport{} +var _ model.HTTPTransport = SaverMetadataHTTPTransport{} +var _ model.HTTPTransport = SaverBodyHTTPTransport{} +var _ model.HTTPTransport = SaverTransactionHTTPTransport{} diff --git a/internal/engine/netx/httptransport/saver_test.go b/internal/engine/netx/httptransport/saver_test.go index 9682b6a..7a93eb9 100644 --- a/internal/engine/netx/httptransport/saver_test.go +++ b/internal/engine/netx/httptransport/saver_test.go @@ -22,12 +22,12 @@ func TestSaverPerformanceNoMultipleEvents(t *testing.T) { saver := &trace.Saver{} // register twice - do we see events twice? txp := httptransport.SaverPerformanceHTTPTransport{ - RoundTripper: http.DefaultTransport.(*http.Transport), - Saver: saver, + HTTPTransport: http.DefaultTransport.(*http.Transport), + Saver: saver, } txp = httptransport.SaverPerformanceHTTPTransport{ - RoundTripper: txp, - Saver: saver, + HTTPTransport: txp, + Saver: saver, } req, err := http.NewRequest("GET", "https://www.google.com", nil) if err != nil { @@ -67,8 +67,8 @@ func TestSaverMetadataSuccess(t *testing.T) { } saver := &trace.Saver{} txp := httptransport.SaverMetadataHTTPTransport{ - RoundTripper: http.DefaultTransport.(*http.Transport), - Saver: saver, + HTTPTransport: http.DefaultTransport.(*http.Transport), + Saver: saver, } req, err := http.NewRequest("GET", "https://www.google.com", nil) if err != nil { @@ -121,7 +121,7 @@ func TestSaverMetadataFailure(t *testing.T) { expected := errors.New("mocked error") saver := &trace.Saver{} txp := httptransport.SaverMetadataHTTPTransport{ - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Err: expected, }, Saver: saver, @@ -165,8 +165,8 @@ func TestSaverTransactionSuccess(t *testing.T) { } saver := &trace.Saver{} txp := httptransport.SaverTransactionHTTPTransport{ - RoundTripper: http.DefaultTransport.(*http.Transport), - Saver: saver, + HTTPTransport: http.DefaultTransport.(*http.Transport), + Saver: saver, } req, err := http.NewRequest("GET", "https://www.google.com", nil) if err != nil { @@ -206,7 +206,7 @@ func TestSaverTransactionFailure(t *testing.T) { expected := errors.New("mocked error") saver := &trace.Saver{} txp := httptransport.SaverTransactionHTTPTransport{ - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Err: expected, }, Saver: saver, @@ -246,7 +246,7 @@ func TestSaverTransactionFailure(t *testing.T) { func TestSaverBodySuccess(t *testing.T) { saver := new(trace.Saver) txp := httptransport.SaverBodyHTTPTransport{ - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Func: func(req *http.Request) (*http.Response, error) { data, err := netxlite.ReadAllContext(context.Background(), req.Body) if err != nil { @@ -317,7 +317,7 @@ func TestSaverBodySuccess(t *testing.T) { func TestSaverBodyRequestReadError(t *testing.T) { saver := new(trace.Saver) txp := httptransport.SaverBodyHTTPTransport{ - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Func: func(req *http.Request) (*http.Response, error) { panic("should not be called") }, @@ -348,7 +348,7 @@ func TestSaverBodyRoundTripError(t *testing.T) { saver := new(trace.Saver) expected := errors.New("mocked error") txp := httptransport.SaverBodyHTTPTransport{ - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Err: expected, }, SnapshotSize: 4, @@ -388,7 +388,7 @@ func TestSaverBodyResponseReadError(t *testing.T) { saver := new(trace.Saver) expected := errors.New("mocked error") txp := httptransport.SaverBodyHTTPTransport{ - RoundTripper: httptransport.FakeTransport{ + HTTPTransport: httptransport.FakeTransport{ Func: func(req *http.Request) (*http.Response, error) { return &http.Response{ StatusCode: 200, diff --git a/internal/engine/netx/httptransport/system.go b/internal/engine/netx/httptransport/system.go index 442cf79..0d92fcb 100644 --- a/internal/engine/netx/httptransport/system.go +++ b/internal/engine/netx/httptransport/system.go @@ -2,6 +2,8 @@ package httptransport import ( "net/http" + + "github.com/ooni/probe-cli/v3/internal/model" ) // NewSystemTransport creates a new "system" HTTP transport. That is a transport @@ -10,7 +12,7 @@ import ( // Deprecation warning // // New code should use netxlite.NewHTTPTransport instead. -func NewSystemTransport(config Config) RoundTripper { +func NewSystemTransport(config Config) model.HTTPTransport { txp := http.DefaultTransport.(*http.Transport).Clone() txp.DialContext = config.Dialer.DialContext txp.DialTLSContext = config.TLSDialer.DialTLSContext @@ -25,4 +27,4 @@ func NewSystemTransport(config Config) RoundTripper { return txp } -var _ RoundTripper = &http.Transport{} +var _ model.HTTPTransport = &http.Transport{} diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 180820f..a515407 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -30,7 +30,6 @@ import ( "net/http" "net/url" - "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/bytecounter" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport" @@ -42,41 +41,13 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite" ) -// Dialer is the definition of dialer assumed by this package. -type Dialer interface { - DialContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// QUICDialer is the definition of a dialer for QUIC assumed by this package. -type QUICDialer interface { - DialContext(ctx context.Context, network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) -} - -// TLSDialer is the definition of a TLS dialer assumed by this package. -type TLSDialer interface { - DialTLSContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// HTTPRoundTripper is the definition of http.HTTPRoundTripper used by this package. -type HTTPRoundTripper interface { - RoundTrip(req *http.Request) (*http.Response, error) - CloseIdleConnections() -} - -// Resolver is the interface we expect from a resolver -type Resolver interface { - LookupHost(ctx context.Context, hostname string) (addrs []string, err error) - Network() string - Address() string -} - // Config contains configuration for creating a new transport. When any // field of Config is nil/empty, we will use a suitable default. // // We use different savers for different kind of events such that the // user of this library can choose what to save. type Config struct { - BaseResolver Resolver // default: system resolver + BaseResolver model.Resolver // default: system resolver BogonIsError bool // default: bogon is not error ByteCounter *bytecounter.Counter // default: no explicit byte counting CacheResolutions bool // default: no caching @@ -84,9 +55,9 @@ type Config struct { ContextByteCounting bool // default: no implicit byte counting DNSCache map[string][]string // default: cache is empty DialSaver *trace.Saver // default: not saving dials - Dialer Dialer // default: dialer.DNSDialer - FullResolver Resolver // default: base resolver + goodies - QUICDialer QUICDialer // default: quicdialer.DNSDialer + Dialer model.Dialer // default: dialer.DNSDialer + FullResolver model.Resolver // default: base resolver + goodies + QUICDialer model.QUICDialer // default: quicdialer.DNSDialer HTTP3Enabled bool // default: disabled HTTPSaver *trace.Saver // default: not saving HTTP Logger model.DebugLogger // default: no logging @@ -95,7 +66,7 @@ type Config struct { ReadWriteSaver *trace.Saver // default: not saving read/write ResolveSaver *trace.Saver // default: not saving resolves TLSConfig *tls.Config // default: attempt using h2 - TLSDialer TLSDialer // default: dialer.TLSDialer + TLSDialer model.TLSDialer // default: dialer.TLSDialer TLSSaver *trace.Saver // default: not saving TLS } @@ -107,13 +78,13 @@ type tlsHandshaker interface { var defaultCertPool *x509.CertPool = netxlite.NewDefaultCertPool() // NewResolver creates a new resolver from the specified config -func NewResolver(config Config) Resolver { +func NewResolver(config Config) model.Resolver { if config.BaseResolver == nil { config.BaseResolver = &netxlite.ResolverSystem{} } - var r Resolver = config.BaseResolver - r = &resolver.AddressResolver{ - Resolver: netxlite.NewResolverLegacyAdapter(r), + var r model.Resolver = config.BaseResolver + r = &netxlite.AddressResolver{ + Resolver: r, } if config.CacheResolutions { r = &resolver.CacheResolver{Resolver: r} @@ -128,21 +99,21 @@ func NewResolver(config Config) Resolver { if config.BogonIsError { r = resolver.BogonResolver{Resolver: r} } - r = &netxlite.ErrorWrapperResolver{Resolver: netxlite.NewResolverLegacyAdapter(r)} + r = &netxlite.ErrorWrapperResolver{Resolver: r} if config.Logger != nil { r = &netxlite.ResolverLogger{ Logger: config.Logger, - Resolver: netxlite.NewResolverLegacyAdapter(r), + Resolver: r, } } if config.ResolveSaver != nil { r = resolver.SaverResolver{Resolver: r, Saver: config.ResolveSaver} } - return &resolver.IDNAResolver{Resolver: netxlite.NewResolverLegacyAdapter(r)} + return &netxlite.ResolverIDNA{Resolver: r} } // NewDialer creates a new Dialer from the specified config -func NewDialer(config Config) Dialer { +func NewDialer(config Config) model.Dialer { if config.FullResolver == nil { config.FullResolver = NewResolver(config) } @@ -156,11 +127,11 @@ func NewDialer(config Config) Dialer { } // NewQUICDialer creates a new DNS Dialer for QUIC, with the resolver from the specified config -func NewQUICDialer(config Config) QUICDialer { +func NewQUICDialer(config Config) model.QUICDialer { if config.FullResolver == nil { config.FullResolver = NewResolver(config) } - var ql quicdialer.QUICListener = &netxlite.QUICListenerStdlib{} + var ql model.QUICListener = &netxlite.QUICListenerStdlib{} ql = &netxlite.ErrorWrapperQUICListener{QUICListener: ql} if config.ReadWriteSaver != nil { ql = &quicdialer.QUICListenerSaver{ @@ -168,24 +139,24 @@ func NewQUICDialer(config Config) QUICDialer { Saver: config.ReadWriteSaver, } } - var d quicdialer.ContextDialer = &netxlite.QUICDialerQUICGo{ + var d model.QUICDialer = &netxlite.QUICDialerQUICGo{ QUICListener: ql, } d = &netxlite.ErrorWrapperQUICDialer{ - QUICDialer: netxlite.NewQUICDialerFromContextDialerAdapter(d), + QUICDialer: d, } if config.TLSSaver != nil { - d = quicdialer.HandshakeSaver{Saver: config.TLSSaver, Dialer: d} + d = quicdialer.HandshakeSaver{Saver: config.TLSSaver, QUICDialer: d} } d = &netxlite.QUICDialerResolver{ - Resolver: netxlite.NewResolverLegacyAdapter(config.FullResolver), - Dialer: netxlite.NewQUICDialerFromContextDialerAdapter(d), + Resolver: config.FullResolver, + Dialer: d, } return d } // NewTLSDialer creates a new TLSDialer from the specified config -func NewTLSDialer(config Config) TLSDialer { +func NewTLSDialer(config Config) model.TLSDialer { if config.Dialer == nil { config.Dialer = NewDialer(config) } @@ -207,14 +178,14 @@ func NewTLSDialer(config Config) TLSDialer { config.TLSConfig.InsecureSkipVerify = config.NoTLSVerify return &netxlite.TLSDialerLegacy{ Config: config.TLSConfig, - Dialer: netxlite.NewDialerLegacyAdapter(config.Dialer), + Dialer: config.Dialer, TLSHandshaker: h, } } // NewHTTPTransport creates a new HTTPRoundTripper. You can further extend the returned // HTTPRoundTripper before wrapping it into an http.Client. -func NewHTTPTransport(config Config) HTTPRoundTripper { +func NewHTTPTransport(config Config) model.HTTPTransport { if config.Dialer == nil { config.Dialer = NewDialer(config) } @@ -233,27 +204,27 @@ func NewHTTPTransport(config Config) HTTPRoundTripper { if config.ByteCounter != nil { txp = httptransport.ByteCountingTransport{ - Counter: config.ByteCounter, RoundTripper: txp} + Counter: config.ByteCounter, HTTPTransport: txp} } if config.Logger != nil { txp = &netxlite.HTTPTransportLogger{Logger: config.Logger, HTTPTransport: txp} } if config.HTTPSaver != nil { txp = httptransport.SaverMetadataHTTPTransport{ - RoundTripper: txp, Saver: config.HTTPSaver, Transport: transport} + HTTPTransport: txp, Saver: config.HTTPSaver, Transport: transport} txp = httptransport.SaverBodyHTTPTransport{ - RoundTripper: txp, Saver: config.HTTPSaver} + HTTPTransport: txp, Saver: config.HTTPSaver} txp = httptransport.SaverPerformanceHTTPTransport{ - RoundTripper: txp, Saver: config.HTTPSaver} + HTTPTransport: txp, Saver: config.HTTPSaver} txp = httptransport.SaverTransactionHTTPTransport{ - RoundTripper: txp, Saver: config.HTTPSaver} + HTTPTransport: txp, Saver: config.HTTPSaver} } return txp } // httpTransportInfo contains the constructing function as well as the transport name type httpTransportInfo struct { - Factory func(httptransport.Config) httptransport.RoundTripper + Factory func(httptransport.Config) model.HTTPTransport TransportName string } @@ -271,7 +242,7 @@ var allTransportsInfo = map[bool]httpTransportInfo{ // DNSClient is a DNS client. It wraps a Resolver and it possibly // also wraps an HTTP client, but only when we're using DoH. type DNSClient struct { - Resolver + model.Resolver httpClient *http.Client } @@ -351,7 +322,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride, return c, err } var txp resolver.RoundTripper = resolver.NewDNSOverUDP( - netxlite.NewDialerLegacyAdapter(dialer), endpoint) + dialer, endpoint) if config.ResolveSaver != nil { txp = resolver.SaverDNSTransport{ RoundTripper: txp, diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index c5e3f90..f8987c1 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -22,27 +22,19 @@ import ( func TestNewResolverVanilla(t *testing.T) { r := netx.NewResolver(netx.Config{}) - ir, ok := r.(*resolver.IDNAResolver) + ir, ok := r.(*netxlite.ResolverIDNA) if !ok { t.Fatal("not the resolver we expected") } - rla, ok := ir.Resolver.(*netxlite.ResolverLegacyAdapter) + ewr, ok := ir.Resolver.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) + ar, ok := ewr.Resolver.(*netxlite.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - arw, ok := ar.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - _, ok = arw.ResolverLegacy.(*netxlite.ResolverSystem) + _, ok = ar.Resolver.(*netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -54,27 +46,19 @@ func TestNewResolverSpecificResolver(t *testing.T) { // not initialized because it doesn't matter in this context }, }) - ir, ok := r.(*resolver.IDNAResolver) + ir, ok := r.(*netxlite.ResolverIDNA) if !ok { t.Fatal("not the resolver we expected") } - rla, ok := ir.Resolver.(*netxlite.ResolverLegacyAdapter) + ewr, ok := ir.Resolver.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) + ar, ok := ewr.Resolver.(*netxlite.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - arw, ok := ar.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - _, ok = arw.ResolverLegacy.(resolver.BogonResolver) + _, ok = ar.Resolver.(resolver.BogonResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -84,31 +68,23 @@ func TestNewResolverWithBogonFilter(t *testing.T) { r := netx.NewResolver(netx.Config{ BogonIsError: true, }) - ir, ok := r.(*resolver.IDNAResolver) + ir, ok := r.(*netxlite.ResolverIDNA) if !ok { t.Fatal("not the resolver we expected") } - rla, ok := ir.Resolver.(*netxlite.ResolverLegacyAdapter) + ewr, ok := ir.Resolver.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) + br, ok := ewr.Resolver.(resolver.BogonResolver) if !ok { t.Fatal("not the resolver we expected") } - br, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(resolver.BogonResolver) + ar, ok := br.Resolver.(*netxlite.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - ar, ok := br.Resolver.(*resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - arw, ok := ar.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - _, ok = arw.ResolverLegacy.(*netxlite.ResolverSystem) + _, ok = ar.Resolver.(*netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -118,46 +94,26 @@ func TestNewResolverWithLogging(t *testing.T) { r := netx.NewResolver(netx.Config{ Logger: log.Log, }) - ir, ok := r.(*resolver.IDNAResolver) + ir, ok := r.(*netxlite.ResolverIDNA) if !ok { t.Fatal("not the resolver we expected") } - rla, ok := ir.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - lr, ok := rla.ResolverLegacy.(*netxlite.ResolverLogger) + lr, ok := ir.Resolver.(*netxlite.ResolverLogger) if !ok { t.Fatal("not the resolver we expected") } if lr.Logger != log.Log { t.Fatal("not the logger we expected") } - rla, ok = ir.Resolver.(*netxlite.ResolverLegacyAdapter) + ewr, ok := lr.Resolver.(*netxlite.ErrorWrapperResolver) + if !ok { + t.Fatalf("not the resolver we expected") + } + ar, ok := ewr.Resolver.(*netxlite.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - lr, ok = rla.ResolverLegacy.(*netxlite.ResolverLogger) - if !ok { - t.Fatal("not the resolver we expected") - } - rla, ok = lr.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) - if !ok { - t.Fatalf("not the resolver we expected %T", rla.ResolverLegacy) - } - ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - arw, ok := ar.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - _, ok = arw.ResolverLegacy.(*netxlite.ResolverSystem) + _, ok = ar.Resolver.(*netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -168,15 +124,11 @@ func TestNewResolverWithSaver(t *testing.T) { r := netx.NewResolver(netx.Config{ ResolveSaver: saver, }) - ir, ok := r.(*resolver.IDNAResolver) + ir, ok := r.(*netxlite.ResolverIDNA) if !ok { t.Fatal("not the resolver we expected") } - rla, ok := ir.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - sr, ok := rla.ResolverLegacy.(resolver.SaverResolver) + sr, ok := ir.Resolver.(resolver.SaverResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -187,15 +139,11 @@ func TestNewResolverWithSaver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) + ar, ok := ewr.Resolver.(*netxlite.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - arw, ok := ar.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - _, ok = arw.ResolverLegacy.(*netxlite.ResolverSystem) + _, ok = ar.Resolver.(*netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -205,34 +153,26 @@ func TestNewResolverWithReadWriteCache(t *testing.T) { r := netx.NewResolver(netx.Config{ CacheResolutions: true, }) - ir, ok := r.(*resolver.IDNAResolver) + ir, ok := r.(*netxlite.ResolverIDNA) if !ok { t.Fatal("not the resolver we expected") } - rla, ok := ir.Resolver.(*netxlite.ResolverLegacyAdapter) + ewr, ok := ir.Resolver.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - cr, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.CacheResolver) + cr, ok := ewr.Resolver.(*resolver.CacheResolver) if !ok { t.Fatal("not the resolver we expected") } if cr.ReadOnly != false { t.Fatal("expected readwrite cache here") } - ar, ok := cr.Resolver.(*resolver.AddressResolver) + ar, ok := cr.Resolver.(*netxlite.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - arw, ok := ar.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - _, ok = arw.ResolverLegacy.(*netxlite.ResolverSystem) + _, ok = ar.Resolver.(*netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -244,19 +184,15 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { "dns.google.com": {"8.8.8.8"}, }, }) - ir, ok := r.(*resolver.IDNAResolver) + ir, ok := r.(*netxlite.ResolverIDNA) if !ok { t.Fatal("not the resolver we expected") } - rla, ok := ir.Resolver.(*netxlite.ResolverLegacyAdapter) + ewr, ok := ir.Resolver.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) - if !ok { - t.Fatal("not the resolver we expected") - } - cr, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.CacheResolver) + cr, ok := ewr.Resolver.(*resolver.CacheResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -266,15 +202,11 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { if cr.Get("dns.google.com")[0] != "8.8.8.8" { t.Fatal("cache not correctly prefilled") } - ar, ok := cr.Resolver.(*resolver.AddressResolver) + ar, ok := cr.Resolver.(*netxlite.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } - arw, ok := ar.Resolver.(*netxlite.ResolverLegacyAdapter) - if !ok { - t.Fatal("not the resolver we expected") - } - _, ok = arw.ResolverLegacy.(*netxlite.ResolverSystem) + _, ok = ar.Resolver.(*netxlite.ResolverSystem) if !ok { t.Fatal("not the resolver we expected") } @@ -548,7 +480,7 @@ func TestNewWithByteCounter(t *testing.T) { if bctxp.Counter != counter { t.Fatal("not the byte counter we expected") } - if _, ok := bctxp.RoundTripper.(*http.Transport); !ok { + if _, ok := bctxp.HTTPTransport.(*http.Transport); !ok { t.Fatal("not the transport we expected") } } @@ -581,28 +513,28 @@ func TestNewWithSaver(t *testing.T) { if stxptxp.Saver != saver { t.Fatal("not the logger we expected") } - sptxp, ok := stxptxp.RoundTripper.(httptransport.SaverPerformanceHTTPTransport) + sptxp, ok := stxptxp.HTTPTransport.(httptransport.SaverPerformanceHTTPTransport) if !ok { t.Fatal("not the transport we expected") } if sptxp.Saver != saver { t.Fatal("not the logger we expected") } - sbtxp, ok := sptxp.RoundTripper.(httptransport.SaverBodyHTTPTransport) + sbtxp, ok := sptxp.HTTPTransport.(httptransport.SaverBodyHTTPTransport) if !ok { t.Fatal("not the transport we expected") } if sbtxp.Saver != saver { t.Fatal("not the logger we expected") } - smtxp, ok := sbtxp.RoundTripper.(httptransport.SaverMetadataHTTPTransport) + smtxp, ok := sbtxp.HTTPTransport.(httptransport.SaverMetadataHTTPTransport) if !ok { t.Fatal("not the transport we expected") } if smtxp.Saver != saver { t.Fatal("not the logger we expected") } - if _, ok := smtxp.RoundTripper.(*http.Transport); !ok { + if _, ok := smtxp.HTTPTransport.(*http.Transport); !ok { t.Fatal("not the transport we expected") } } diff --git a/internal/engine/netx/quicdialer/quicdialer.go b/internal/engine/netx/quicdialer/quicdialer.go deleted file mode 100644 index 1babc87..0000000 --- a/internal/engine/netx/quicdialer/quicdialer.go +++ /dev/null @@ -1,20 +0,0 @@ -package quicdialer - -import ( - "context" - "crypto/tls" - - "github.com/lucas-clemente/quic-go" -) - -// ContextDialer is a dialer for QUIC using Context. -type ContextDialer interface { - // Note: assumes that tlsCfg and cfg are not nil. - DialContext(ctx context.Context, network, host string, - tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) -} - -// Resolver is the interface we expect from a resolver. -type Resolver interface { - LookupHost(ctx context.Context, hostname string) (addrs []string, err error) -} diff --git a/internal/engine/netx/quicdialer/saver.go b/internal/engine/netx/quicdialer/saver.go index 76bd28b..3018694 100644 --- a/internal/engine/netx/quicdialer/saver.go +++ b/internal/engine/netx/quicdialer/saver.go @@ -7,13 +7,14 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) // HandshakeSaver saves events occurring during the handshake type HandshakeSaver struct { - Saver *trace.Saver - Dialer ContextDialer + Saver *trace.Saver + model.QUICDialer } // DialContext implements ContextDialer.DialContext @@ -31,7 +32,7 @@ func (h HandshakeSaver) DialContext(ctx context.Context, network string, TLSServerName: tlsCfg.ServerName, Time: start, }) - sess, err := h.Dialer.DialContext(ctx, network, host, tlsCfg, cfg) + sess, err := h.QUICDialer.DialContext(ctx, network, host, tlsCfg, cfg) stop := time.Now() if err != nil { h.Saver.Write(trace.Event{ diff --git a/internal/engine/netx/quicdialer/saver_test.go b/internal/engine/netx/quicdialer/saver_test.go index a158be5..eb92201 100644 --- a/internal/engine/netx/quicdialer/saver_test.go +++ b/internal/engine/netx/quicdialer/saver_test.go @@ -11,12 +11,13 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/engine/netx/quicdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" ) type MockDialer struct { - Dialer quicdialer.ContextDialer + Dialer model.QUICDialer Sess quic.EarlySession Err error } @@ -38,7 +39,7 @@ func TestHandshakeSaverSuccess(t *testing.T) { } saver := &trace.Saver{} dlr := quicdialer.HandshakeSaver{ - Dialer: &netxlite.QUICDialerQUICGo{ + QUICDialer: &netxlite.QUICDialerQUICGo{ QUICListener: &netxlite.QUICListenerStdlib{}, }, Saver: saver, @@ -96,7 +97,7 @@ func TestHandshakeSaverHostNameError(t *testing.T) { } saver := &trace.Saver{} dlr := quicdialer.HandshakeSaver{ - Dialer: &netxlite.QUICDialerQUICGo{ + QUICDialer: &netxlite.QUICDialerQUICGo{ QUICListener: &netxlite.QUICListenerStdlib{}, }, Saver: saver, diff --git a/internal/engine/netx/quicdialer/system.go b/internal/engine/netx/quicdialer/system.go index 97d63dd..a805bd7 100644 --- a/internal/engine/netx/quicdialer/system.go +++ b/internal/engine/netx/quicdialer/system.go @@ -9,16 +9,10 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite" ) -// QUICListener listens for QUIC connections. -type QUICListener interface { - // Listen creates a new listening UDPConn. - Listen(addr *net.UDPAddr) (model.UDPLikeConn, error) -} - // QUICListenerSaver is a QUICListener that also implements saving events. type QUICListenerSaver struct { // QUICListener is the underlying QUICListener. - QUICListener QUICListener + model.QUICListener // Saver is the underlying Saver. Saver *trace.Saver diff --git a/internal/engine/netx/resolver/address.go b/internal/engine/netx/resolver/address.go deleted file mode 100644 index cae77eb..0000000 --- a/internal/engine/netx/resolver/address.go +++ /dev/null @@ -1,9 +0,0 @@ -package resolver - -import ( - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -// AddressResolver is a resolver that knows how to correctly -// resolve IP addresses to themselves. -type AddressResolver = netxlite.AddressResolver diff --git a/internal/engine/netx/resolver/bogon.go b/internal/engine/netx/resolver/bogon.go index 5b8c4ef..e308688 100644 --- a/internal/engine/netx/resolver/bogon.go +++ b/internal/engine/netx/resolver/bogon.go @@ -3,6 +3,7 @@ package resolver import ( "context" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -14,7 +15,7 @@ import ( // This resolver is deprecated. The right thing to do would be to check // for bogons right after a domain name resolution in the nettest. type BogonResolver struct { - Resolver + model.Resolver } // LookupHost implements Resolver.LookupHost @@ -28,4 +29,4 @@ func (r BogonResolver) LookupHost(ctx context.Context, hostname string) ([]strin return addrs, err } -var _ Resolver = BogonResolver{} +var _ model.Resolver = BogonResolver{} diff --git a/internal/engine/netx/resolver/cache.go b/internal/engine/netx/resolver/cache.go index 86eae9b..34f5e02 100644 --- a/internal/engine/netx/resolver/cache.go +++ b/internal/engine/netx/resolver/cache.go @@ -3,12 +3,14 @@ package resolver import ( "context" "sync" + + "github.com/ooni/probe-cli/v3/internal/model" ) // CacheResolver is a resolver that caches successful replies. type CacheResolver struct { ReadOnly bool - Resolver + model.Resolver mu sync.Mutex cache map[string][]string } diff --git a/internal/engine/netx/resolver/cache_test.go b/internal/engine/netx/resolver/cache_test.go index 05dea44..d1d549f 100644 --- a/internal/engine/netx/resolver/cache_test.go +++ b/internal/engine/netx/resolver/cache_test.go @@ -6,11 +6,12 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/model" ) func TestCacheFailure(t *testing.T) { expected := errors.New("mocked error") - var r resolver.Resolver = resolver.FakeResolver{ + var r model.Resolver = resolver.FakeResolver{ Err: expected, } cache := &resolver.CacheResolver{Resolver: r} @@ -27,7 +28,7 @@ func TestCacheFailure(t *testing.T) { } func TestCacheHitSuccess(t *testing.T) { - var r resolver.Resolver = resolver.FakeResolver{ + var r model.Resolver = resolver.FakeResolver{ Err: errors.New("mocked error"), } cache := &resolver.CacheResolver{Resolver: r} @@ -42,7 +43,7 @@ func TestCacheHitSuccess(t *testing.T) { } func TestCacheMissSuccess(t *testing.T) { - var r resolver.Resolver = resolver.FakeResolver{ + var r model.Resolver = resolver.FakeResolver{ Result: []string{"8.8.8.8"}, } cache := &resolver.CacheResolver{Resolver: r} @@ -59,7 +60,7 @@ func TestCacheMissSuccess(t *testing.T) { } func TestCacheReadonlySuccess(t *testing.T) { - var r resolver.Resolver = resolver.FakeResolver{ + var r model.Resolver = resolver.FakeResolver{ Result: []string{"8.8.8.8"}, } cache := &resolver.CacheResolver{Resolver: r, ReadOnly: true} diff --git a/internal/engine/netx/resolver/chain.go b/internal/engine/netx/resolver/chain.go index def7534..d963782 100644 --- a/internal/engine/netx/resolver/chain.go +++ b/internal/engine/netx/resolver/chain.go @@ -2,13 +2,15 @@ package resolver import ( "context" + + "github.com/ooni/probe-cli/v3/internal/model" ) // ChainResolver is a chain resolver. The primary resolver is used first and, if that // fails, we then attempt with the secondary resolver. type ChainResolver struct { - Primary Resolver - Secondary Resolver + Primary model.Resolver + Secondary model.Resolver } // LookupHost implements Resolver.LookupHost @@ -30,4 +32,20 @@ func (c ChainResolver) Address() string { return "" } -var _ Resolver = ChainResolver{} +// CloseIdleConnections implements Resolver.CloseIdleConnections. +func (c ChainResolver) CloseIdleConnections() { + c.Primary.CloseIdleConnections() + c.Secondary.CloseIdleConnections() +} + +// LookupHTTPS implements Resolver.LookupHTTPS +func (c ChainResolver) LookupHTTPS( + ctx context.Context, domain string) (*model.HTTPSSvc, error) { + https, err := c.Primary.LookupHTTPS(ctx, domain) + if err != nil { + https, err = c.Secondary.LookupHTTPS(ctx, domain) + } + return https, err +} + +var _ model.Resolver = ChainResolver{} diff --git a/internal/engine/netx/resolver/fake_test.go b/internal/engine/netx/resolver/fake_test.go index b74bbec..154ddf8 100644 --- a/internal/engine/netx/resolver/fake_test.go +++ b/internal/engine/netx/resolver/fake_test.go @@ -2,11 +2,13 @@ package resolver import ( "context" + "errors" "io" "net" "time" "github.com/ooni/probe-cli/v3/internal/atomicx" + "github.com/ooni/probe-cli/v3/internal/model" ) type FakeDialer struct { @@ -143,4 +145,11 @@ func (c FakeResolver) Address() string { return "" } -var _ Resolver = FakeResolver{} +func (c FakeResolver) CloseIdleConnections() {} + +func (c FakeResolver) LookupHTTPS( + ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("not implemented") +} + +var _ model.Resolver = FakeResolver{} diff --git a/internal/engine/netx/resolver/idna.go b/internal/engine/netx/resolver/idna.go deleted file mode 100644 index 2da888e..0000000 --- a/internal/engine/netx/resolver/idna.go +++ /dev/null @@ -1,11 +0,0 @@ -package resolver - -import ( - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -// IDNAResolver is to support resolving Internationalized Domain Names. -// See RFC3492 for more information. -type IDNAResolver = netxlite.ResolverIDNA - -var _ Resolver = &IDNAResolver{} diff --git a/internal/engine/netx/resolver/integration_test.go b/internal/engine/netx/resolver/integration_test.go index 6181e5e..3896317 100644 --- a/internal/engine/netx/resolver/integration_test.go +++ b/internal/engine/netx/resolver/integration_test.go @@ -8,6 +8,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -15,13 +16,13 @@ func init() { log.SetLevel(log.DebugLevel) } -func testresolverquick(t *testing.T, reso resolver.Resolver) { +func testresolverquick(t *testing.T, reso model.Resolver) { if testing.Short() { t.Skip("skip test in short mode") } reso = &netxlite.ResolverLogger{ Logger: log.Log, - Resolver: netxlite.NewResolverLegacyAdapter(reso), + Resolver: reso, } addrs, err := reso.LookupHost(context.Background(), "dns.google.com") if err != nil { @@ -43,14 +44,14 @@ func testresolverquick(t *testing.T, reso resolver.Resolver) { } // Ensuring we can handle Internationalized Domain Names (IDNs) without issues -func testresolverquickidna(t *testing.T, reso resolver.Resolver) { +func testresolverquickidna(t *testing.T, reso model.Resolver) { if testing.Short() { t.Skip("skip test in short mode") } - reso = &resolver.IDNAResolver{ + reso = &netxlite.ResolverIDNA{ Resolver: &netxlite.ResolverLogger{ Logger: log.Log, - Resolver: netxlite.NewResolverLegacyAdapter(reso), + Resolver: reso, }, } addrs, err := reso.LookupHost(context.Background(), "яндекс.рф") @@ -70,14 +71,14 @@ func TestNewResolverSystem(t *testing.T) { func TestNewResolverUDPAddress(t *testing.T) { reso := resolver.NewSerialResolver( - resolver.NewDNSOverUDP(netxlite.NewDialerLegacyAdapter(&net.Dialer{}), "8.8.8.8:53")) + resolver.NewDNSOverUDP(netxlite.DefaultDialer, "8.8.8.8:53")) testresolverquick(t, reso) testresolverquickidna(t, reso) } func TestNewResolverUDPDomain(t *testing.T) { reso := resolver.NewSerialResolver( - resolver.NewDNSOverUDP(netxlite.NewDialerLegacyAdapter(&net.Dialer{}), "dns.google.com:53")) + resolver.NewDNSOverUDP(netxlite.DefaultDialer, "dns.google.com:53")) testresolverquick(t, reso) testresolverquickidna(t, reso) } diff --git a/internal/engine/netx/resolver/resolver.go b/internal/engine/netx/resolver/resolver.go deleted file mode 100644 index 96db0de..0000000 --- a/internal/engine/netx/resolver/resolver.go +++ /dev/null @@ -1,18 +0,0 @@ -package resolver - -import ( - "context" -) - -// Resolver is a DNS resolver. The *net.Resolver used by Go implements -// this interface, but other implementations are possible. -type Resolver interface { - // LookupHost resolves a hostname to a list of IP addresses. - LookupHost(ctx context.Context, hostname string) (addrs []string, err error) - - // Network returns the network being used by the resolver - Network() string - - // Address returns the address being used by the resolver - Address() string -} diff --git a/internal/engine/netx/resolver/saver.go b/internal/engine/netx/resolver/saver.go index f16f941..26388d1 100644 --- a/internal/engine/netx/resolver/saver.go +++ b/internal/engine/netx/resolver/saver.go @@ -5,11 +5,12 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/model" ) // SaverResolver is a resolver that saves events type SaverResolver struct { - Resolver + model.Resolver Saver *trace.Saver } @@ -69,5 +70,5 @@ func (txp SaverDNSTransport) RoundTrip(ctx context.Context, query []byte) ([]byt return reply, err } -var _ Resolver = SaverResolver{} +var _ model.Resolver = SaverResolver{} var _ RoundTripper = SaverDNSTransport{} diff --git a/internal/engine/netx/tlsdialer/saver.go b/internal/engine/netx/tlsdialer/saver.go index 10f3a8d..e7a50a6 100644 --- a/internal/engine/netx/tlsdialer/saver.go +++ b/internal/engine/netx/tlsdialer/saver.go @@ -7,12 +7,13 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) // SaverTLSHandshaker saves events occurring during the handshake type SaverTLSHandshaker struct { - TLSHandshaker + model.TLSHandshaker Saver *trace.Saver } @@ -46,4 +47,4 @@ func (h SaverTLSHandshaker) Handshake( return tlsconn, state, err } -var _ TLSHandshaker = SaverTLSHandshaker{} +var _ model.TLSHandshaker = SaverTLSHandshaker{} diff --git a/internal/engine/netx/tlsdialer/saver_test.go b/internal/engine/netx/tlsdialer/saver_test.go index 7af6d0d..29d4657 100644 --- a/internal/engine/netx/tlsdialer/saver_test.go +++ b/internal/engine/netx/tlsdialer/saver_test.go @@ -3,7 +3,6 @@ package tlsdialer_test import ( "context" "crypto/tls" - "net" "reflect" "testing" "time" @@ -23,9 +22,7 @@ func TestSaverTLSHandshakerSuccessWithReadWrite(t *testing.T) { saver := &trace.Saver{} tlsdlr := &netxlite.TLSDialerLegacy{ Config: &tls.Config{NextProtos: nextprotos}, - Dialer: netxlite.NewDialerLegacyAdapter( - dialer.New(&dialer.Config{ReadWriteSaver: saver}, &net.Resolver{}), - ), + Dialer: dialer.New(&dialer.Config{ReadWriteSaver: saver}, netxlite.DefaultResolver), TLSHandshaker: tlsdialer.SaverTLSHandshaker{ TLSHandshaker: &netxlite.TLSHandshakerConfigurable{}, Saver: saver, diff --git a/internal/engine/netx/tlsdialer/tls.go b/internal/engine/netx/tlsdialer/tls.go deleted file mode 100644 index fcdf676..0000000 --- a/internal/engine/netx/tlsdialer/tls.go +++ /dev/null @@ -1,19 +0,0 @@ -// Package tlsdialer contains code to establish TLS connections. -package tlsdialer - -import ( - "context" - "crypto/tls" - "net" -) - -// UnderlyingDialer is the underlying dialer type. -type UnderlyingDialer interface { - DialContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// TLSHandshaker is the generic TLS handshaker -type TLSHandshaker interface { - Handshake(ctx context.Context, conn net.Conn, config *tls.Config) ( - net.Conn, tls.ConnectionState, error) -} diff --git a/internal/engine/session.go b/internal/engine/session.go index 037608a..4efb4bf 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -51,7 +51,7 @@ type Session struct { availableProbeServices []model.OOAPIService availableTestHelpers map[string][]model.OOAPIService byteCounter *bytecounter.Counter - httpDefaultTransport netx.HTTPRoundTripper + httpDefaultTransport model.HTTPTransport kvStore model.KeyValueStore location *geolocate.Results logger model.Logger diff --git a/internal/netxlite/legacy.go b/internal/netxlite/legacy.go index 515795f..d213220 100644 --- a/internal/netxlite/legacy.go +++ b/internal/netxlite/legacy.go @@ -1,14 +1,5 @@ package netxlite -import ( - "context" - "crypto/tls" - "net" - - "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/model" -) - // These vars export internal names to legacy ooni/probe-cli code. // // Deprecated: do not use these names in new code. @@ -44,143 +35,3 @@ type ( TLSDialerLegacy = tlsDialer AddressResolver = resolverShortCircuitIPAddr ) - -// ResolverLegacy performs domain name resolutions. -// -// Deprecated: new code should use Resolver. -// -// Existing code in ooni/probe-cli is still using this definition. -type ResolverLegacy interface { - // LookupHost behaves like net.Resolver.LookupHost. - LookupHost(ctx context.Context, hostname string) (addrs []string, err error) -} - -// NewResolverLegacyAdapter adapts a ResolverLegacy to -// become compatible with the Resolver definition. -func NewResolverLegacyAdapter(reso ResolverLegacy) model.Resolver { - return &ResolverLegacyAdapter{reso} -} - -// ResolverLegacyAdapter makes a ResolverLegacy behave like a Resolver. -type ResolverLegacyAdapter struct { - ResolverLegacy -} - -var _ model.Resolver = &ResolverLegacyAdapter{} - -type resolverLegacyNetworker interface { - Network() string -} - -// Network implements Resolver.Network. -func (r *ResolverLegacyAdapter) Network() string { - if rn, ok := r.ResolverLegacy.(resolverLegacyNetworker); ok { - return rn.Network() - } - return "adapter" -} - -type resolverLegacyAddresser interface { - Address() string -} - -// Address implements Resolver.Address. -func (r *ResolverLegacyAdapter) Address() string { - if ra, ok := r.ResolverLegacy.(resolverLegacyAddresser); ok { - return ra.Address() - } - return "" -} - -type resolverLegacyIdleConnectionsCloser interface { - CloseIdleConnections() -} - -// CloseIdleConnections implements Resolver.CloseIdleConnections. -func (r *ResolverLegacyAdapter) CloseIdleConnections() { - if ra, ok := r.ResolverLegacy.(resolverLegacyIdleConnectionsCloser); ok { - ra.CloseIdleConnections() - } -} - -// LookupHTTPS always returns ErrDNSNoTransport. -func (r *ResolverLegacyAdapter) LookupHTTPS( - ctx context.Context, domain string) (*model.HTTPSSvc, error) { - return nil, ErrNoDNSTransport -} - -// DialerLegacy establishes network connections. -// -// Deprecated: please use Dialer instead. -// -// Existing code in probe-cli can use it until we -// have finished refactoring it. -type DialerLegacy interface { - // DialContext behaves like net.Dialer.DialContext. - DialContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// NewDialerLegacyAdapter adapts a DialerrLegacy to -// become compatible with the Dialer definition. -// -// Deprecated: do not use this function in new code. -func NewDialerLegacyAdapter(d DialerLegacy) model.Dialer { - return &DialerLegacyAdapter{d} -} - -// DialerLegacyAdapter makes a DialerLegacy behave like -// it was a Dialer type. If DialerLegacy is actually also -// a Dialer, this adapter will just forward missing calls, -// otherwise it will implement a sensible default action. -type DialerLegacyAdapter struct { - DialerLegacy -} - -var _ model.Dialer = &DialerLegacyAdapter{} - -type dialerLegacyIdleConnectionsCloser interface { - CloseIdleConnections() -} - -// CloseIdleConnections implements Dialer.CloseIdleConnections. -func (d *DialerLegacyAdapter) CloseIdleConnections() { - if ra, ok := d.DialerLegacy.(dialerLegacyIdleConnectionsCloser); ok { - ra.CloseIdleConnections() - } -} - -// QUICContextDialer is a dialer for QUIC using Context. -// -// Deprecated: new code should use QUICDialer. -// -// Use NewQUICDialerFromContextDialerAdapter if you need to -// adapt to QUICDialer. -type QUICContextDialer interface { - // DialContext establishes a new QUIC session using the given - // network and address. The tlsConfig and the quicConfig arguments - // MUST NOT be nil. Returns either the session or an error. - DialContext(ctx context.Context, network, address string, - tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlySession, error) -} - -// NewQUICDialerFromContextDialerAdapter creates a new -// QUICDialer from a QUICContextDialer. -func NewQUICDialerFromContextDialerAdapter(d QUICContextDialer) model.QUICDialer { - return &QUICContextDialerAdapter{d} -} - -// QUICContextDialerAdapter adapts a QUICContextDialer to be a QUICDialer. -type QUICContextDialerAdapter struct { - QUICContextDialer -} - -type quicContextDialerConnectionsCloser interface { - CloseIdleConnections() -} - -// CloseIdleConnections implements QUICDialer.CloseIdleConnections. -func (d *QUICContextDialerAdapter) CloseIdleConnections() { - if o, ok := d.QUICContextDialer.(quicContextDialerConnectionsCloser); ok { - o.CloseIdleConnections() - } -} diff --git a/internal/netxlite/legacy_test.go b/internal/netxlite/legacy_test.go deleted file mode 100644 index 5725f64..0000000 --- a/internal/netxlite/legacy_test.go +++ /dev/null @@ -1,100 +0,0 @@ -package netxlite - -import ( - "context" - "errors" - "net" - "testing" - - "github.com/ooni/probe-cli/v3/internal/model/mocks" - nlmocks "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" -) - -func TestResolverLegacyAdapter(t *testing.T) { - t.Run("with compatible type", func(t *testing.T) { - var called bool - r := NewResolverLegacyAdapter(&mocks.Resolver{ - MockNetwork: func() string { - return "network" - }, - MockAddress: func() string { - return "address" - }, - MockCloseIdleConnections: func() { - called = true - }, - }) - if r.Network() != "network" { - t.Fatal("invalid Network") - } - if r.Address() != "address" { - t.Fatal("invalid Address") - } - r.CloseIdleConnections() - if !called { - t.Fatal("not called") - } - }) - - t.Run("with incompatible type", func(t *testing.T) { - r := NewResolverLegacyAdapter(&net.Resolver{}) - if r.Network() != "adapter" { - t.Fatal("invalid Network") - } - if r.Address() != "" { - t.Fatal("invalid Address") - } - r.CloseIdleConnections() // does not crash - }) - - t.Run("for LookupHTTPS", func(t *testing.T) { - r := NewResolverLegacyAdapter(&net.Resolver{}) - https, err := r.LookupHTTPS(context.Background(), "x.org") - if !errors.Is(err, ErrNoDNSTransport) { - t.Fatal("not the error we expected") - } - if https != nil { - t.Fatal("expected nil result") - } - }) -} - -func TestDialerLegacyAdapter(t *testing.T) { - t.Run("with compatible type", func(t *testing.T) { - var called bool - r := NewDialerLegacyAdapter(&mocks.Dialer{ - MockCloseIdleConnections: func() { - called = true - }, - }) - r.CloseIdleConnections() - if !called { - t.Fatal("not called") - } - }) - - t.Run("with incompatible type", func(t *testing.T) { - r := NewDialerLegacyAdapter(&net.Dialer{}) - r.CloseIdleConnections() // does not crash - }) -} - -func TestQUICContextDialerAdapter(t *testing.T) { - t.Run("with compatible type", func(t *testing.T) { - var called bool - d := NewQUICDialerFromContextDialerAdapter(&mocks.QUICDialer{ - MockCloseIdleConnections: func() { - called = true - }, - }) - d.CloseIdleConnections() - if !called { - t.Fatal("not called") - } - }) - - t.Run("with incompatible type", func(t *testing.T) { - d := NewQUICDialerFromContextDialerAdapter(&nlmocks.QUICContextDialer{}) - d.CloseIdleConnections() // does not crash - }) -}