From e126e73de7a8c369742939539d8a0ebbd37ffd5c Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 13 May 2022 18:26:15 +0200 Subject: [PATCH] fix(netxlite): LookupHTTPS: short circuit IP addr (#725) This diff fixes the short-circuit-IP-addr resolver to correctly handle IP addrs during LookupHTTPS. The original diff was: https://github.com/bassosimone/websteps-illustrated/commit/2b51d144bf642f10237102bdc79a0defc30c1579 See https://github.com/ooni/probe/issues/2096 While there, add unit tests for IPv6. --- internal/netxlite/resolver.go | 13 +++++ internal/netxlite/resolver_test.go | 88 ++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go index 401cb62..8b7451e 100644 --- a/internal/netxlite/resolver.go +++ b/internal/netxlite/resolver.go @@ -209,6 +209,19 @@ func (r *resolverShortCircuitIPAddr) LookupHost(ctx context.Context, hostname st return r.Resolver.LookupHost(ctx, hostname) } +func (r *resolverShortCircuitIPAddr) LookupHTTPS(ctx context.Context, hostname string) (*model.HTTPSSvc, error) { + if net.ParseIP(hostname) != nil { + https := &model.HTTPSSvc{} + if isIPv6(hostname) { + https.IPv6 = append(https.IPv6, hostname) + } else { + https.IPv4 = append(https.IPv4, hostname) + } + return https, nil + } + return r.Resolver.LookupHTTPS(ctx, hostname) +} + // IsIPv6 returns true if the given candidate is a valid IP address // representation and such representation is IPv6. func IsIPv6(candidate string) (bool, error) { diff --git a/internal/netxlite/resolver_test.go b/internal/netxlite/resolver_test.go index 17a2677..750074e 100644 --- a/internal/netxlite/resolver_test.go +++ b/internal/netxlite/resolver_test.go @@ -440,6 +440,94 @@ func TestResolverShortCircuitIPAddr(t *testing.T) { } }) }) + + t.Run("LookupHTTPS", func(t *testing.T) { + t.Run("with IPv4 addr", func(t *testing.T) { + r := &resolverShortCircuitIPAddr{ + Resolver: &mocks.Resolver{ + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("mocked error") + }, + }, + } + ctx := context.Background() + https, err := r.LookupHTTPS(ctx, "8.8.8.8") + if err != nil { + t.Fatal(err) + } + if len(https.IPv4) != 1 || https.IPv4[0] != "8.8.8.8" { + t.Fatal("invalid result") + } + }) + + t.Run("with IPv6 addr", func(t *testing.T) { + r := &resolverShortCircuitIPAddr{ + Resolver: &mocks.Resolver{ + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("mocked error") + }, + }, + } + ctx := context.Background() + https, err := r.LookupHTTPS(ctx, "::1") + if err != nil { + t.Fatal(err) + } + if len(https.IPv6) != 1 || https.IPv6[0] != "::1" { + t.Fatal("invalid result") + } + }) + + t.Run("with domain", func(t *testing.T) { + r := &resolverShortCircuitIPAddr{ + Resolver: &mocks.Resolver{ + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("mocked error") + }, + }, + } + ctx := context.Background() + https, err := r.LookupHTTPS(ctx, "dns.google") + if err == nil || err.Error() != "mocked error" { + t.Fatal("not the error we expected", err) + } + if https != nil { + t.Fatal("invalid result") + } + }) + }) +} + +func TestIsIPv6(t *testing.T) { + t.Run("with neither IPv4 nor IPv6 as input", func(t *testing.T) { + ipv6, err := IsIPv6("example.com") + if !errors.Is(err, ErrInvalidIP) { + t.Fatal("not the error we expected", err) + } + if ipv6 { + t.Fatal("expected false") + } + }) + + t.Run("with IPv4 as input", func(t *testing.T) { + ipv6, err := IsIPv6("1.2.3.4") + if err != nil { + t.Fatal(err) + } + if ipv6 { + t.Fatal("expected false") + } + }) + + t.Run("with IPv6 as input", func(t *testing.T) { + ipv6, err := IsIPv6("::1") + if err != nil { + t.Fatal(err) + } + if !ipv6 { + t.Fatal("expected true") + } + }) } func TestNullResolver(t *testing.T) {