diff --git a/internal/cmd/oohelperd/internal/nwcth/measure.go b/internal/cmd/oohelperd/internal/nwcth/measure.go index 73e2316..02eb8f4 100644 --- a/internal/cmd/oohelperd/internal/nwcth/measure.go +++ b/internal/cmd/oohelperd/internal/nwcth/measure.go @@ -91,6 +91,6 @@ func newResolver() netxlite.Resolver { childResolver, err := netx.NewDNSClient(netx.Config{Logger: log.Log}, "doh://google") runtimex.PanicOnError(err, "NewDNSClient failed") var r netxlite.Resolver = childResolver - r = &netxlite.IDNAResolver{Resolver: r} + r = &netxlite.ResolverIDNA{Resolver: r} return r } diff --git a/internal/engine/experiment/websteps/dns.go b/internal/engine/experiment/websteps/dns.go index e521059..00b298f 100644 --- a/internal/engine/experiment/websteps/dns.go +++ b/internal/engine/experiment/websteps/dns.go @@ -21,7 +21,7 @@ func DNSDo(ctx context.Context, config DNSConfig) ([]string, error) { childResolver, err := netx.NewDNSClient(netx.Config{Logger: log.Log}, "doh://google") runtimex.PanicOnError(err, "NewDNSClient failed") var resolver netxlite.Resolver = childResolver - resolver = &netxlite.IDNAResolver{Resolver: resolver} + resolver = &netxlite.ResolverIDNA{Resolver: resolver} } return config.Resolver.LookupHost(ctx, config.Domain) } diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index a4db396..73ce898 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -139,7 +139,7 @@ func NewResolver(config Config) Resolver { if config.ResolveSaver != nil { r = resolver.SaverResolver{Resolver: r, Saver: config.ResolveSaver} } - return resolver.IDNAResolver{Resolver: r} + return &resolver.IDNAResolver{Resolver: r} } // NewDialer creates a new Dialer from the specified config diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index 21506e1..3d29a85 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -20,7 +20,7 @@ import ( func TestNewResolverVanilla(t *testing.T) { r := netx.NewResolver(netx.Config{}) - ir, ok := r.(resolver.IDNAResolver) + ir, ok := r.(*resolver.IDNAResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -44,7 +44,7 @@ func TestNewResolverSpecificResolver(t *testing.T) { // not initialized because it doesn't matter in this context }, }) - ir, ok := r.(resolver.IDNAResolver) + ir, ok := r.(*resolver.IDNAResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -66,7 +66,7 @@ func TestNewResolverWithBogonFilter(t *testing.T) { r := netx.NewResolver(netx.Config{ BogonIsError: true, }) - ir, ok := r.(resolver.IDNAResolver) + ir, ok := r.(*resolver.IDNAResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -92,7 +92,7 @@ func TestNewResolverWithLogging(t *testing.T) { r := netx.NewResolver(netx.Config{ Logger: log.Log, }) - ir, ok := r.(resolver.IDNAResolver) + ir, ok := r.(*resolver.IDNAResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -122,7 +122,7 @@ func TestNewResolverWithSaver(t *testing.T) { r := netx.NewResolver(netx.Config{ ResolveSaver: saver, }) - ir, ok := r.(resolver.IDNAResolver) + ir, ok := r.(*resolver.IDNAResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -151,7 +151,7 @@ func TestNewResolverWithReadWriteCache(t *testing.T) { r := netx.NewResolver(netx.Config{ CacheResolutions: true, }) - ir, ok := r.(resolver.IDNAResolver) + ir, ok := r.(*resolver.IDNAResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -182,7 +182,7 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { "dns.google.com": {"8.8.8.8"}, }, }) - ir, ok := r.(resolver.IDNAResolver) + ir, ok := r.(*resolver.IDNAResolver) if !ok { t.Fatal("not the resolver we expected") } diff --git a/internal/engine/netx/resolver/idna.go b/internal/engine/netx/resolver/idna.go index 1fed5a7..2da888e 100644 --- a/internal/engine/netx/resolver/idna.go +++ b/internal/engine/netx/resolver/idna.go @@ -1,34 +1,11 @@ package resolver import ( - "context" - - "golang.org/x/net/idna" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) // IDNAResolver is to support resolving Internationalized Domain Names. // See RFC3492 for more information. -type IDNAResolver struct { - Resolver -} +type IDNAResolver = netxlite.ResolverIDNA -// LookupHost implements Resolver.LookupHost -func (r IDNAResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { - host, err := idna.ToASCII(hostname) - if err != nil { - return nil, err - } - return r.Resolver.LookupHost(ctx, host) -} - -// Network implements Resolver.Network. -func (r IDNAResolver) Network() string { - return "idna" -} - -// Address implements Resolver.Address. -func (r IDNAResolver) Address() string { - return "" -} - -var _ Resolver = IDNAResolver{} +var _ Resolver = &IDNAResolver{} diff --git a/internal/engine/netx/resolver/idna_test.go b/internal/engine/netx/resolver/idna_test.go deleted file mode 100644 index a3865ed..0000000 --- a/internal/engine/netx/resolver/idna_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package resolver_test - -import ( - "context" - "errors" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" -) - -var ErrUnexpectedPunycode = errors.New("unexpected punycode value") - -type CheckIDNAResolver struct { - Addresses []string - Error error - Expect string -} - -func (resolv CheckIDNAResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { - if resolv.Error != nil { - return nil, resolv.Error - } - if hostname != resolv.Expect { - return nil, ErrUnexpectedPunycode - } - return resolv.Addresses, nil -} - -func (r CheckIDNAResolver) Network() string { - return "checkidna" -} - -func (r CheckIDNAResolver) Address() string { - return "" -} - -func TestIDNAResolverSuccess(t *testing.T) { - expectedIPs := []string{"77.88.55.66"} - resolv := resolver.IDNAResolver{Resolver: CheckIDNAResolver{ - Addresses: expectedIPs, - Expect: "xn--d1acpjx3f.xn--p1ai", - }} - addrs, err := resolv.LookupHost(context.Background(), "яндекс.рф") - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(expectedIPs, addrs); diff != "" { - t.Fatal(diff) - } -} - -func TestIDNAResolverFailure(t *testing.T) { - resolv := resolver.IDNAResolver{Resolver: CheckIDNAResolver{ - Error: errors.New("we should not arrive here"), - }} - // See https://www.farsightsecurity.com/blog/txt-record/punycode-20180711/ - addrs, err := resolv.LookupHost(context.Background(), "xn--0000h") - if err == nil || !strings.HasPrefix(err.Error(), "idna: invalid label") { - t.Fatal("not the error we expected") - } - if addrs != nil { - t.Fatal("expected no response here") - } -} - -func TestIDNAResolverTransportOK(t *testing.T) { - resolv := resolver.IDNAResolver{Resolver: CheckIDNAResolver{}} - if resolv.Network() != "idna" { - t.Fatal("invalid network") - } - if resolv.Address() != "" { - t.Fatal("invalid address") - } -} diff --git a/internal/engine/netx/resolver/integration_test.go b/internal/engine/netx/resolver/integration_test.go index 96038df..7de5a7e 100644 --- a/internal/engine/netx/resolver/integration_test.go +++ b/internal/engine/netx/resolver/integration_test.go @@ -44,8 +44,8 @@ func testresolverquickidna(t *testing.T, reso resolver.Resolver) { if testing.Short() { t.Skip("skip test in short mode") } - reso = resolver.IDNAResolver{ - &netxlite.ResolverLogger{Logger: log.Log, Resolver: reso}, + reso = &resolver.IDNAResolver{ + Resolver: &netxlite.ResolverLogger{Logger: log.Log, Resolver: reso}, } addrs, err := reso.LookupHost(context.Background(), "яндекс.рф") if err != nil { diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go index 393be5e..ce70b8c 100644 --- a/internal/netxlite/resolver.go +++ b/internal/netxlite/resolver.go @@ -83,14 +83,15 @@ func (r *ResolverLogger) Address() string { return "" } -// IDNAResolver is to support resolving Internationalized Domain Names. +// ResolverIDNA supports resolving Internationalized Domain Names. +// // See RFC3492 for more information. -type IDNAResolver struct { +type ResolverIDNA struct { Resolver } -// LookupHost implements Resolver.LookupHost -func (r *IDNAResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { +// LookupHost implements Resolver.LookupHost. +func (r *ResolverIDNA) LookupHost(ctx context.Context, hostname string) ([]string, error) { host, err := idna.ToASCII(hostname) if err != nil { return nil, err @@ -99,7 +100,7 @@ func (r *IDNAResolver) LookupHost(ctx context.Context, hostname string) ([]strin } // Network implements Resolver.Network. -func (r *IDNAResolver) Network() string { +func (r *ResolverIDNA) Network() string { if rn, ok := r.Resolver.(resolverNetworker); ok { return rn.Network() } @@ -107,7 +108,7 @@ func (r *IDNAResolver) Network() string { } // Address implements Resolver.Address. -func (r *IDNAResolver) Address() string { +func (r *ResolverIDNA) Address() string { if ra, ok := r.Resolver.(resolverAddresser); ok { return ra.Address() } diff --git a/internal/netxlite/resolver_test.go b/internal/netxlite/resolver_test.go index 430bb25..3596ec5 100644 --- a/internal/netxlite/resolver_test.go +++ b/internal/netxlite/resolver_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "net" + "strings" "testing" "github.com/apex/log" @@ -89,3 +90,64 @@ func TestResolverLoggerNoChildNetworkAddress(t *testing.T) { t.Fatal("invalid Address") } } + +func TestResolverIDNAWorksAsIntended(t *testing.T) { + expectedIPs := []string{"77.88.55.66"} + r := &ResolverIDNA{ + Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + if domain != "xn--d1acpjx3f.xn--p1ai" { + return nil, errors.New("passed invalid domain") + } + return expectedIPs, nil + }, + }, + } + ctx := context.Background() + addrs, err := r.LookupHost(ctx, "яндекс.рф") + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(expectedIPs, addrs); diff != "" { + t.Fatal(diff) + } +} + +func TestIDNAResolverWithInvalidPunycode(t *testing.T) { + r := &ResolverIDNA{Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return nil, errors.New("should not happen") + }, + }} + // See https://www.farsightsecurity.com/blog/txt-record/punycode-20180711/ + ctx := context.Background() + addrs, err := r.LookupHost(ctx, "xn--0000h") + if err == nil || !strings.HasPrefix(err.Error(), "idna: invalid label") { + t.Fatal("not the error we expected") + } + if addrs != nil { + t.Fatal("expected no response here") + } +} + +func TestResolverIDNAChildNetworkAddress(t *testing.T) { + r := &ResolverIDNA{ + Resolver: DefaultResolver, + } + if v := r.Network(); v != "system" { + t.Fatal("invalid network", v) + } + if v := r.Address(); v != "" { + t.Fatal("invalid address", v) + } +} + +func TestResolverIDNANoChildNetworkAddress(t *testing.T) { + r := &ResolverIDNA{} + if v := r.Network(); v != "idna" { + t.Fatal("invalid network", v) + } + if v := r.Address(); v != "" { + t.Fatal("invalid address", v) + } +}