diff --git a/internal/engine/geolocate/cloudflare.go b/internal/engine/geolocate/cloudflare.go index b98c1e6cc2..3de61a538b 100644 --- a/internal/engine/geolocate/cloudflare.go +++ b/internal/engine/geolocate/cloudflare.go @@ -15,6 +15,7 @@ func cloudflareIPLookup( httpClient *http.Client, logger model.Logger, userAgent string, + resolver model.Resolver, ) (string, error) { data, err := (&httpx.APIClientTemplate{ BaseURL: "https://www.cloudflare.com", diff --git a/internal/engine/geolocate/cloudflare_test.go b/internal/engine/geolocate/cloudflare_test.go index 49c50b7c15..4f0b44da41 100644 --- a/internal/engine/geolocate/cloudflare_test.go +++ b/internal/engine/geolocate/cloudflare_test.go @@ -8,6 +8,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestIPLookupWorksUsingcloudlflare(t *testing.T) { @@ -16,6 +17,7 @@ func TestIPLookupWorksUsingcloudlflare(t *testing.T) { http.DefaultClient, log.Log, model.HTTPHeaderUserAgent, + netxlite.NewStdlibResolver(model.DiscardLogger), ) if err != nil { t.Fatal(err) diff --git a/internal/engine/geolocate/geolocate.go b/internal/engine/geolocate/geolocate.go index 175c7db033..0b88bbaa04 100644 --- a/internal/engine/geolocate/geolocate.go +++ b/internal/engine/geolocate/geolocate.go @@ -90,7 +90,9 @@ func NewTask(config Config) *Task { probeIPLookupper: ipLookupClient(config), probeASNLookupper: mmdbLookupper{}, resolverASNLookupper: mmdbLookupper{}, - resolverIPLookupper: resolverLookupClient{}, + resolverIPLookupper: resolverLookupClient{ + Resolver: config.Resolver, + }, } } diff --git a/internal/engine/geolocate/invalid_test.go b/internal/engine/geolocate/invalid_test.go index aade7c71de..bb62404328 100644 --- a/internal/engine/geolocate/invalid_test.go +++ b/internal/engine/geolocate/invalid_test.go @@ -12,6 +12,7 @@ func invalidIPLookup( httpClient *http.Client, logger model.Logger, userAgent string, + resolver model.Resolver, ) (string, error) { return "invalid IP", nil } diff --git a/internal/engine/geolocate/iplookup.go b/internal/engine/geolocate/iplookup.go index b5b40b069b..3d9711f323 100644 --- a/internal/engine/geolocate/iplookup.go +++ b/internal/engine/geolocate/iplookup.go @@ -27,6 +27,7 @@ var ( type lookupFunc func( ctx context.Context, client *http.Client, logger model.Logger, userAgent string, + resolver model.Resolver, ) (string, error) type method struct { @@ -89,7 +90,7 @@ func (c ipLookupClient) doWithCustomFunc( txp := netxlite.NewHTTPTransportWithResolver(c.Logger, c.Resolver) clnt := &http.Client{Transport: txp} defer clnt.CloseIdleConnections() - ip, err := fn(ctx, clnt, c.Logger, c.UserAgent) + ip, err := fn(ctx, clnt, c.Logger, c.UserAgent, c.Resolver) if err != nil { return model.DefaultProbeIP, err } diff --git a/internal/engine/geolocate/resolverlookup.go b/internal/engine/geolocate/resolverlookup.go index fb25eb1a97..ef6835b225 100644 --- a/internal/engine/geolocate/resolverlookup.go +++ b/internal/engine/geolocate/resolverlookup.go @@ -3,7 +3,8 @@ package geolocate import ( "context" "errors" - "net" + + "github.com/ooni/probe-cli/v3/internal/model" ) var ( @@ -16,7 +17,9 @@ type dnsResolver interface { LookupHost(ctx context.Context, host string) (addrs []string, err error) } -type resolverLookupClient struct{} +type resolverLookupClient struct { + Resolver model.Resolver +} func (rlc resolverLookupClient) do(ctx context.Context, r dnsResolver) (string, error) { var ips []string @@ -31,5 +34,5 @@ func (rlc resolverLookupClient) do(ctx context.Context, r dnsResolver) (string, } func (rlc resolverLookupClient) LookupResolverIP(ctx context.Context) (ip string, err error) { - return rlc.do(ctx, &net.Resolver{}) + return rlc.do(ctx, rlc.Resolver) } diff --git a/internal/engine/geolocate/resolverlookup_test.go b/internal/engine/geolocate/resolverlookup_test.go index 03331610ee..3d34c3fb74 100644 --- a/internal/engine/geolocate/resolverlookup_test.go +++ b/internal/engine/geolocate/resolverlookup_test.go @@ -4,10 +4,16 @@ import ( "context" "errors" "testing" + + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestLookupResolverIP(t *testing.T) { - addr, err := (resolverLookupClient{}).LookupResolverIP(context.Background()) + rlc := resolverLookupClient{ + Resolver: netxlite.NewStdlibResolver(model.DiscardLogger), + } + addr, err := rlc.LookupResolverIP(context.Background()) if err != nil { t.Fatal(err) } @@ -26,7 +32,9 @@ func (bhl brokenHostLookupper) LookupHost(ctx context.Context, host string) ([]s func TestLookupResolverIPFailure(t *testing.T) { expected := errors.New("mocked error") - rlc := resolverLookupClient{} + rlc := resolverLookupClient{ + Resolver: netxlite.NewStdlibResolver(model.DiscardLogger), + } addr, err := rlc.do(context.Background(), brokenHostLookupper{ err: expected, }) @@ -39,7 +47,9 @@ func TestLookupResolverIPFailure(t *testing.T) { } func TestLookupResolverIPNoAddressReturned(t *testing.T) { - rlc := resolverLookupClient{} + rlc := resolverLookupClient{ + Resolver: netxlite.NewStdlibResolver(model.DiscardLogger), + } addr, err := rlc.do(context.Background(), brokenHostLookupper{}) if !errors.Is(err, ErrNoIPAddressReturned) { t.Fatalf("not the error we expected: %+v", err) diff --git a/internal/engine/geolocate/stun.go b/internal/engine/geolocate/stun.go index cb3746d80c..d6133a1096 100644 --- a/internal/engine/geolocate/stun.go +++ b/internal/engine/geolocate/stun.go @@ -2,41 +2,49 @@ package geolocate import ( "context" + "net" "net/http" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/pion/stun" ) -// TODO(bassosimone): we should modify the stun code to use -// the session resolver rather than using its own. -// -// See https://github.com/ooni/probe/issues/1383. - type stunClient interface { Close() error Start(m *stun.Message, h stun.Handler) error } type stunConfig struct { - Dial func(network string, address string) (stunClient, error) - Endpoint string - Logger model.Logger + Dialer model.Dialer // optional + Endpoint string + Logger model.Logger + NewClient func(conn net.Conn) (stunClient, error) // optional + Resolver model.Resolver } -func stunDialer(network string, address string) (stunClient, error) { - return stun.Dial(network, address) +func stunNewClient(conn net.Conn) (stunClient, error) { + return stun.NewClient(conn) } func stunIPLookup(ctx context.Context, config stunConfig) (string, error) { config.Logger.Debugf("STUNIPLookup: start using %s", config.Endpoint) ip, err := func() (string, error) { - dial := config.Dial - if dial == nil { - dial = stunDialer + dialer := config.Dialer + if dialer == nil { + dialer = netxlite.NewDialerWithResolver(config.Logger, config.Resolver) + } + conn, err := dialer.DialContext(ctx, "udp", config.Endpoint) + if err != nil { + return model.DefaultProbeIP, err + } + newClient := config.NewClient + if newClient == nil { + newClient = stunNewClient } - clnt, err := dial("udp", config.Endpoint) + clnt, err := newClient(conn) if err != nil { + conn.Close() return model.DefaultProbeIP, err } defer clnt.Close() @@ -78,10 +86,12 @@ func stunEkigaIPLookup( httpClient *http.Client, logger model.Logger, userAgent string, + resolver model.Resolver, ) (string, error) { return stunIPLookup(ctx, stunConfig{ Endpoint: "stun.ekiga.net:3478", Logger: logger, + Resolver: resolver, }) } @@ -90,9 +100,11 @@ func stunGoogleIPLookup( httpClient *http.Client, logger model.Logger, userAgent string, + resolver model.Resolver, ) (string, error) { return stunIPLookup(ctx, stunConfig{ Endpoint: "stun.l.google.com:19302", Logger: logger, + Resolver: resolver, }) } diff --git a/internal/engine/geolocate/stun_test.go b/internal/engine/geolocate/stun_test.go index d89a5bfbaf..c161c069f0 100644 --- a/internal/engine/geolocate/stun_test.go +++ b/internal/engine/geolocate/stun_test.go @@ -10,6 +10,8 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/model/mocks" + "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/pion/stun" ) @@ -19,6 +21,7 @@ func TestSTUNIPLookupCanceledContext(t *testing.T) { ip, err := stunIPLookup(ctx, stunConfig{ Endpoint: "stun.ekiga.net:3478", Logger: log.Log, + Resolver: netxlite.NewStdlibResolver(model.DiscardLogger), }) if !errors.Is(err, context.Canceled) { t.Fatalf("not the error we expected: %+v", err) @@ -32,8 +35,10 @@ func TestSTUNIPLookupDialFailure(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() ip, err := stunIPLookup(ctx, stunConfig{ - Dial: func(network, address string) (stunClient, error) { - return nil, expected + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + return nil, expected + }, }, Endpoint: "stun.ekiga.net:3478", Logger: log.Log, @@ -70,11 +75,17 @@ func TestSTUNIPLookupStartReturnsError(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() ip, err := stunIPLookup(ctx, stunConfig{ - Dial: func(network, address string) (stunClient, error) { - return MockableSTUNClient{StartErr: expected}, nil + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + conn := &mocks.Conn{} + return conn, nil + }, }, Endpoint: "stun.ekiga.net:3478", Logger: log.Log, + NewClient: func(conn net.Conn) (stunClient, error) { + return MockableSTUNClient{StartErr: expected}, nil + }, }) if !errors.Is(err, expected) { t.Fatalf("not the error we expected: %+v", err) @@ -88,13 +99,19 @@ func TestSTUNIPLookupStunEventContainsError(t *testing.T) { expected := errors.New("mocked error") ctx := context.Background() ip, err := stunIPLookup(ctx, stunConfig{ - Dial: func(network, address string) (stunClient, error) { + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + conn := &mocks.Conn{} + return conn, nil + }, + }, + Endpoint: "stun.ekiga.net:3478", + Logger: log.Log, + NewClient: func(conn net.Conn) (stunClient, error) { return MockableSTUNClient{Event: stun.Event{ Error: expected, }}, nil }, - Endpoint: "stun.ekiga.net:3478", - Logger: log.Log, }) if !errors.Is(err, expected) { t.Fatalf("not the error we expected: %+v", err) @@ -107,13 +124,19 @@ func TestSTUNIPLookupStunEventContainsError(t *testing.T) { func TestSTUNIPLookupCannotDecodeMessage(t *testing.T) { ctx := context.Background() ip, err := stunIPLookup(ctx, stunConfig{ - Dial: func(network, address string) (stunClient, error) { + Dialer: &mocks.Dialer{ + MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + conn := &mocks.Conn{} + return conn, nil + }, + }, + Endpoint: "stun.ekiga.net:3478", + Logger: log.Log, + NewClient: func(conn net.Conn) (stunClient, error) { return MockableSTUNClient{Event: stun.Event{ Message: &stun.Message{}, }}, nil }, - Endpoint: "stun.ekiga.net:3478", - Logger: log.Log, }) if !errors.Is(err, stun.ErrAttributeNotFound) { t.Fatalf("not the error we expected: %+v", err) @@ -129,6 +152,7 @@ func TestIPLookupWorksUsingSTUNEkiga(t *testing.T) { http.DefaultClient, log.Log, model.HTTPHeaderUserAgent, + netxlite.NewStdlibResolver(model.DiscardLogger), ) if err != nil { t.Fatal(err) @@ -144,6 +168,7 @@ func TestIPLookupWorksUsingSTUNGoogle(t *testing.T) { http.DefaultClient, log.Log, model.HTTPHeaderUserAgent, + netxlite.NewStdlibResolver(model.DiscardLogger), ) if err != nil { t.Fatal(err) diff --git a/internal/engine/geolocate/ubuntu.go b/internal/engine/geolocate/ubuntu.go index c37e8c6079..0f75afb4e0 100644 --- a/internal/engine/geolocate/ubuntu.go +++ b/internal/engine/geolocate/ubuntu.go @@ -19,6 +19,7 @@ func ubuntuIPLookup( httpClient *http.Client, logger model.Logger, userAgent string, + resolver model.Resolver, ) (string, error) { data, err := (&httpx.APIClientTemplate{ BaseURL: "https://geoip.ubuntu.com/", diff --git a/internal/engine/geolocate/ubuntu_test.go b/internal/engine/geolocate/ubuntu_test.go index 22e1d287f6..d6edf7c015 100644 --- a/internal/engine/geolocate/ubuntu_test.go +++ b/internal/engine/geolocate/ubuntu_test.go @@ -10,6 +10,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) func TestUbuntuParseError(t *testing.T) { @@ -23,6 +24,7 @@ func TestUbuntuParseError(t *testing.T) { }}, log.Log, model.HTTPHeaderUserAgent, + netxlite.NewStdlibResolver(model.DiscardLogger), ) if err == nil || !strings.HasPrefix(err.Error(), "XML syntax error") { t.Fatalf("not the error we expected: %+v", err) @@ -38,6 +40,7 @@ func TestIPLookupWorksUsingUbuntu(t *testing.T) { http.DefaultClient, log.Log, model.HTTPHeaderUserAgent, + netxlite.NewStdlibResolver(model.DiscardLogger), ) if err != nil { t.Fatal(err)