Put everything resolver-related in internal/resolver

internal/dnsconf/dnsconf.go

@@ -2,98 +2,15 @@
 package dnsconf
 
 import (
-	"errors"
-	"net"
-	"net/http"
-	"strings"
-	"time"
-
 	"github.com/ooni/netx/internal/dialerapi"
-	"github.com/ooni/netx/internal/dnstransport/dnsoverhttps"
-	"github.com/ooni/netx/internal/dnstransport/dnsovertcp"
-	"github.com/ooni/netx/internal/dnstransport/dnsoverudp"
-	"github.com/ooni/netx/internal/httptransport"
-	"github.com/ooni/netx/internal/resolver/ooniresolver"
-	"github.com/ooni/netx/model"
+	"github.com/ooni/netx/internal/resolver"
 )
 
 // ConfigureDNS implements netx.Dialer.ConfigureDNS.
 func ConfigureDNS(dialer *dialerapi.Dialer, network, address string) error {
-	r, err := NewResolver(dialer, network, address)
+	r, err := resolver.New(dialer.Beginning, dialer.Handler, network, address)
 	if err == nil {
 		dialer.LookupHost = r.LookupHost
 	}
 	return err
 }
-
-func newHTTPClientForDoH(beginning time.Time, handler model.Handler) *http.Client {
-	dialer := dialerapi.NewDialer(beginning, handler)
-	transport := httptransport.NewTransport(dialer.Beginning, dialer.Handler)
-	// Logic to make sure we'll use the dialer in the new HTTP transport. We have
-	// an already well configured config that works for http2 (as explained in a
-	// comment there). Here we just use it because it's what we need.
-	dialer.TLSConfig = transport.TLSClientConfig
-	// Arrange the configuration such that we always use `dialer` for dialing.
-	transport.Dial = dialer.Dial
-	transport.DialContext = dialer.DialContext
-	transport.DialTLS = dialer.DialTLS
-	transport.MaxConnsPerHost = 1 // seems to be better for cloudflare DNS
-	return &http.Client{Transport: transport}
-}
-
-func withPort(address, port string) string {
-	// Handle the case where port was not specified. We have written in
-	// a bunch of places that we can just pass a domain in this case and
-	// so we need to gracefully ensure this is still possible.
-	_, _, err := net.SplitHostPort(address)
-	if err != nil && strings.Contains(err.Error(), "missing port in address") {
-		address = net.JoinHostPort(address, port)
-	}
-	return address
-}
-
-// NewResolver returns a new resolver using this Dialer as dialer for
-// creating new network connections used for resolving.
-func NewResolver(
-	dialer *dialerapi.Dialer, network, address string,
-) (model.DNSResolver, error) {
-	// Implementation note: system need to be dealt with
-	// separately because it doesn't have any transport.
-	if network == "system" {
-		return &net.Resolver{
-			PreferGo: false,
-		}, nil
-	}
-	var transport model.DNSRoundTripper
-	if network == "doh" {
-		transport = dnsoverhttps.NewTransport(
-			newHTTPClientForDoH(dialer.Beginning, dialer.Handler), address,
-		)
-	} else if network == "dot" {
-		transport = dnsovertcp.NewTransport(
-			// We need a child dialer here to avoid an endless loop where the
-			// dialer will ask us to resolve, we'll tell the dialer to dial, it
-			// will ask us to resolve, ...
-			dnsovertcp.NewTLSDialerAdapter(
-				dialerapi.NewDialer(dialer.Beginning, dialer.Handler),
-			),
-			withPort(address, "853"),
-		)
-	} else if network == "tcp" {
-		transport = dnsovertcp.NewTransport(
-			// Same rationale as above: avoid possible endless loop
-			dialerapi.NewDialer(dialer.Beginning, dialer.Handler),
-			withPort(address, "53"),
-		)
-	} else if network == "udp" {
-		transport = dnsoverudp.NewTransport(
-			// Same rationale as above: avoid possible endless loop
-			dialerapi.NewDialer(dialer.Beginning, dialer.Handler),
-			withPort(address, "53"),
-		)
-	}
-	if transport == nil {
-		return nil, errors.New("dnsconf: unsupported network value")
-	}
-	return ooniresolver.New(dialer.Beginning, dialer.Handler, transport), nil
-}

internal/dnsconf/dnsconf_test.go

@@ -1,7 +1,6 @@
 package dnsconf_test
 
 import (
-	"context"
 	"testing"
 	"time"
 
@@ -10,102 +9,6 @@ import (
 	"github.com/ooni/netx/internal/dnsconf"
 )
 
-func testresolverquick(t *testing.T, network, address string) {
-	d := dialerapi.NewDialer(time.Now(), handlers.NoHandler)
-	resolver, err := dnsconf.NewResolver(d, network, address)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if resolver == nil {
-		t.Fatal("expected non-nil resolver here")
-	}
-	addrs, err := resolver.LookupHost(context.Background(), "dns.google.com")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if addrs == nil {
-		t.Fatal("expected non-nil addrs here")
-	}
-	var foundquad8 bool
-	for _, addr := range addrs {
-		if addr == "8.8.8.8" {
-			foundquad8 = true
-		}
-	}
-	if !foundquad8 {
-		t.Fatal("did not find 8.8.8.8 in ouput")
-	}
-}
-
-func TestIntegrationNewResolverUDPAddress(t *testing.T) {
-	testresolverquick(t, "udp", "8.8.8.8:53")
-}
-
-func TestIntegrationNewResolverUDPAddressNoPort(t *testing.T) {
-	testresolverquick(t, "udp", "8.8.8.8")
-}
-
-func TestIntegrationNewResolverUDPDomain(t *testing.T) {
-	testresolverquick(t, "udp", "dns.google.com:53")
-}
-
-func TestIntegrationNewResolverUDPDomainNoPort(t *testing.T) {
-	testresolverquick(t, "udp", "dns.google.com")
-}
-
-func TestIntegrationNewResolverSystem(t *testing.T) {
-	testresolverquick(t, "system", "")
-}
-
-func TestIntegrationNewResolverTCPAddress(t *testing.T) {
-	testresolverquick(t, "tcp", "8.8.8.8:53")
-}
-
-func TestIntegrationNewResolverTCPAddressNoPort(t *testing.T) {
-	testresolverquick(t, "tcp", "8.8.8.8")
-}
-
-func TestIntegrationNewResolverTCPDomain(t *testing.T) {
-	testresolverquick(t, "tcp", "dns.google.com:53")
-}
-
-func TestIntegrationNewResolverTCPDomainNoPort(t *testing.T) {
-	testresolverquick(t, "tcp", "dns.google.com")
-}
-
-func TestIntegrationNewResolverDoTAddress(t *testing.T) {
-	testresolverquick(t, "dot", "9.9.9.9:853")
-}
-
-func TestIntegrationNewResolverDoTAddressNoPort(t *testing.T) {
-	testresolverquick(t, "dot", "9.9.9.9")
-}
-
-func TestIntegrationNewResolverDoTDomain(t *testing.T) {
-	testresolverquick(t, "dot", "dns.quad9.net:853")
-}
-
-func TestIntegrationNewResolverDoTDomainNoPort(t *testing.T) {
-	testresolverquick(t, "dot", "dns.quad9.net")
-}
-
-func TestIntegrationNewResolverDoH(t *testing.T) {
-	testresolverquick(t, "doh", "https://cloudflare-dns.com/dns-query")
-}
-
-func TestIntegrationNewResolverInvalid(t *testing.T) {
-	d := dialerapi.NewDialer(time.Now(), handlers.NoHandler)
-	resolver, err := dnsconf.NewResolver(
-		d, "antani", "https://cloudflare-dns.com/dns-query",
-	)
-	if err == nil {
-		t.Fatal("expected an error here")
-	}
-	if resolver != nil {
-		t.Fatal("expected a nil resolver here")
-	}
-}
-
 func testconfigurednsquick(t *testing.T, network, address string) {
 	d := dialerapi.NewDialer(time.Now(), handlers.NoHandler)
 	err := dnsconf.ConfigureDNS(d, network, address)

internal/resolver/ooniresolver/ooniresolver_test.go

@@ -9,7 +9,7 @@ import (
 
 	"github.com/miekg/dns"
 	"github.com/ooni/netx/handlers"
-	"github.com/ooni/netx/internal/dnstransport/dnsovertcp"
+	"github.com/ooni/netx/internal/resolver/dnstransport/dnsovertcp"
 	"github.com/ooni/netx/model"
 )
 

internal/resolver/resolver.go

@@ -0,0 +1,90 @@
+// Package resolver contains code to create a resolver
+package resolver
+
+import (
+	"errors"
+	"net"
+	"net/http"
+	"strings"
+	"time"
+
+	"github.com/ooni/netx/internal/dialerapi"
+	"github.com/ooni/netx/internal/resolver/dnstransport/dnsoverhttps"
+	"github.com/ooni/netx/internal/resolver/dnstransport/dnsovertcp"
+	"github.com/ooni/netx/internal/resolver/dnstransport/dnsoverudp"
+	"github.com/ooni/netx/internal/httptransport"
+	"github.com/ooni/netx/internal/resolver/ooniresolver"
+	"github.com/ooni/netx/model"
+)
+
+func newHTTPClientForDoH(beginning time.Time, handler model.Handler) *http.Client {
+	dialer := dialerapi.NewDialer(beginning, handler)
+	transport := httptransport.NewTransport(dialer.Beginning, dialer.Handler)
+	// Logic to make sure we'll use the dialer in the new HTTP transport. We have
+	// an already well configured config that works for http2 (as explained in a
+	// comment there). Here we just use it because it's what we need.
+	dialer.TLSConfig = transport.TLSClientConfig
+	// Arrange the configuration such that we always use `dialer` for dialing.
+	transport.Dial = dialer.Dial
+	transport.DialContext = dialer.DialContext
+	transport.DialTLS = dialer.DialTLS
+	transport.MaxConnsPerHost = 1 // seems to be better for cloudflare DNS
+	return &http.Client{Transport: transport}
+}
+
+func withPort(address, port string) string {
+	// Handle the case where port was not specified. We have written in
+	// a bunch of places that we can just pass a domain in this case and
+	// so we need to gracefully ensure this is still possible.
+	_, _, err := net.SplitHostPort(address)
+	if err != nil && strings.Contains(err.Error(), "missing port in address") {
+		address = net.JoinHostPort(address, port)
+	}
+	return address
+}
+
+// New returns a new resolver using this Dialer as dialer for
+// creating new network connections used for resolving.
+func New(
+	beginning time.Time, handler model.Handler, network, address string,
+) (model.DNSResolver, error) {
+	// Implementation note: system need to be dealt with
+	// separately because it doesn't have any transport.
+	if network == "system" {
+		return &net.Resolver{
+			PreferGo: false,
+		}, nil
+	}
+	var transport model.DNSRoundTripper
+	if network == "doh" {
+		transport = dnsoverhttps.NewTransport(
+			newHTTPClientForDoH(beginning, handler), address,
+		)
+	} else if network == "dot" {
+		transport = dnsovertcp.NewTransport(
+			// We need a child dialer here to avoid an endless loop where the
+			// dialer will ask us to resolve, we'll tell the dialer to dial, it
+			// will ask us to resolve, ...
+			dnsovertcp.NewTLSDialerAdapter(
+				dialerapi.NewDialer(beginning, handler),
+			),
+			withPort(address, "853"),
+		)
+	} else if network == "tcp" {
+		transport = dnsovertcp.NewTransport(
+			// Same rationale as above: avoid possible endless loop
+			dialerapi.NewDialer(beginning, handler),
+			withPort(address, "53"),
+		)
+	} else if network == "udp" {
+		transport = dnsoverudp.NewTransport(
+			// Same rationale as above: avoid possible endless loop
+			dialerapi.NewDialer(beginning, handler),
+			withPort(address, "53"),
+		)
+	}
+	if transport == nil {
+		return nil, errors.New("resolver.New: unsupported network value")
+	}
+	return ooniresolver.New(beginning, handler, transport), nil
+}