dnsoverhttps.go: decouple from netx/httpx (#27)

internal/dnsconf/dnsconf.go

@@ -5,6 +5,8 @@ import (
 	"context"
 	"errors"
 	"net"
+	"net/http"
+	"time"
 
 	"github.com/ooni/netx/dnsx"
 	"github.com/ooni/netx/internal/connx"
@@ -13,6 +15,8 @@ import (
 	"github.com/ooni/netx/internal/dnstransport/dnsovertcp"
 	"github.com/ooni/netx/internal/dnstransport/dnsoverudp"
 	"github.com/ooni/netx/internal/godns"
+	"github.com/ooni/netx/internal/httptransport"
+	"github.com/ooni/netx/model"
 )
 
 // ConfigureDNS implements netx.Dialer.ConfigureDNS.
@@ -24,6 +28,21 @@ func ConfigureDNS(dialer *dialerapi.Dialer, network, address string) error {
 	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}
+}
+
 // NewResolver returns a new resolver using this Dialer as dialer for
 // creating new network connections used for resolving.
 func NewResolver(
@@ -55,7 +74,7 @@ func NewResolver(
 	var transport dnsx.RoundTripper
 	if network == "doh" {
 		transport = dnsoverhttps.NewTransport(
-			dialer.Beginning, dialer.Handler, address,
+			newHTTPClientForDoH(dialer.Beginning, dialer.Handler), address,
 		)
 	} else if network == "dot" {
 		host, port, err := net.SplitHostPort(address)

internal/dnstransport/dnsoverhttps/dnsoverhttps.go

@@ -6,62 +6,40 @@ import (
 	"errors"
 	"io/ioutil"
 	"net/http"
-	"time"
-
-	"github.com/ooni/netx/internal/dialerapi"
-	"github.com/ooni/netx/internal/httptransport"
-	"github.com/ooni/netx/model"
 )
 
 // Transport is a DNS over HTTPS dnsx.RoundTripper.
 //
 // As a known bug, this implementation does not cache the domain
 // name in the URL for reuse, but this should be easy to fix.
 type Transport struct {
-	// Client is the HTTP client to use.
-	Client *http.Client
-
-	// ClientDo allows to override the Client.Do behaviour. This is
-	// initialized in NewTransport to call Client.Do.
-	ClientDo func(req *http.Request) (*http.Response, error)
-
-	// URL is the DoH server URL.
-	URL string
+	clientDo func(req *http.Request) (*http.Response, error)
+	url      string
 }
 
 // NewTransport creates a new Transport
-func NewTransport(beginning time.Time, handler model.Handler, URL string) *Transport {
-	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
-	dialer.TLSConfig = transport.TLSClientConfig
-	transport.Dial = dialer.Dial
-	transport.DialContext = dialer.DialContext
-	transport.DialTLS = dialer.DialTLS
-	transport.MaxConnsPerHost = 1 // seems to be better for cloudflare DNS
-	client := &http.Client{Transport: transport}
+func NewTransport(client *http.Client, URL string) *Transport {
 	return &Transport{
-		Client:   client,
-		ClientDo: client.Do,
-		URL:      URL,
+		clientDo: client.Do,
+		url:      URL,
 	}
 }
 
 // RoundTrip sends a request and receives a response.
 func (t *Transport) RoundTrip(query []byte) (reply []byte, err error) {
-	req, err := http.NewRequest("POST", t.URL, bytes.NewReader(query))
+	req, err := http.NewRequest("POST", t.url, bytes.NewReader(query))
 	if err != nil {
 		return nil, err
 	}
 	req.Header.Set("content-type", "application/dns-message")
 	var resp *http.Response
-	resp, err = t.ClientDo(req)
+	resp, err = t.clientDo(req)
 	if err != nil {
 		return
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode != 200 {
-		// TODO(ooni): we should map the status code to a
+		// TODO(bassosimone): we should map the status code to a
 		// proper Error in the DNS context.
 		err = errors.New("doh: server returned error")
 		return

internal/dnstransport/dnsoverhttps/dnsoverhttps_test.go

@@ -1,22 +1,18 @@
-package dnsoverhttps_test
+package dnsoverhttps
 
 import (
 	"errors"
 	"io/ioutil"
 	"net/http"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/miekg/dns"
-	"github.com/ooni/netx/handlers"
-	"github.com/ooni/netx/internal/dnstransport/dnsoverhttps"
 )
 
 func TestIntegrationSuccess(t *testing.T) {
-	transport := dnsoverhttps.NewTransport(
-		time.Now(), handlers.NoHandler,
-		"https://cloudflare-dns.com/dns-query",
+	transport := NewTransport(
+		http.DefaultClient, "https://cloudflare-dns.com/dns-query",
 	)
 	err := threeRounds(transport)
 	if err != nil {
@@ -25,9 +21,8 @@ func TestIntegrationSuccess(t *testing.T) {
 }
 
 func TestIntegrationNewRequestFailure(t *testing.T) {
-	transport := dnsoverhttps.NewTransport(
-		time.Now(), handlers.NoHandler,
-		"\t", // invalid URL
+	transport := NewTransport(
+		http.DefaultClient, "\t", // invalid URL
 	)
 	err := threeRounds(transport)
 	if err == nil {
@@ -36,11 +31,10 @@ func TestIntegrationNewRequestFailure(t *testing.T) {
 }
 
 func TestIntegrationClientDoFailure(t *testing.T) {
-	transport := dnsoverhttps.NewTransport(
-		time.Now(), handlers.NoHandler,
-		"https://cloudflare-dns.com/dns-query",
+	transport := NewTransport(
+		http.DefaultClient, "https://cloudflare-dns.com/dns-query",
 	)
-	transport.ClientDo = func(*http.Request) (*http.Response, error) {
+	transport.clientDo = func(*http.Request) (*http.Response, error) {
 		return nil, errors.New("mocked error")
 	}
 	err := threeRounds(transport)
@@ -50,11 +44,10 @@ func TestIntegrationClientDoFailure(t *testing.T) {
 }
 
 func TestIntegrationHTTPFailure(t *testing.T) {
-	transport := dnsoverhttps.NewTransport(
-		time.Now(), handlers.NoHandler,
-		"https://cloudflare-dns.com/dns-query",
+	transport := NewTransport(
+		http.DefaultClient, "https://cloudflare-dns.com/dns-query",
 	)
-	transport.ClientDo = func(*http.Request) (*http.Response, error) {
+	transport.clientDo = func(*http.Request) (*http.Response, error) {
 		return &http.Response{
 			StatusCode: 500,
 			Body:       ioutil.NopCloser(strings.NewReader("")),
@@ -67,11 +60,10 @@ func TestIntegrationHTTPFailure(t *testing.T) {
 }
 
 func TestIntegrationMissingHeader(t *testing.T) {
-	transport := dnsoverhttps.NewTransport(
-		time.Now(), handlers.NoHandler,
-		"https://cloudflare-dns.com/dns-query",
+	transport := NewTransport(
+		http.DefaultClient, "https://cloudflare-dns.com/dns-query",
 	)
-	transport.ClientDo = func(*http.Request) (*http.Response, error) {
+	transport.clientDo = func(*http.Request) (*http.Response, error) {
 		return &http.Response{
 			StatusCode: 200,
 			Body:       ioutil.NopCloser(strings.NewReader("")),
@@ -83,7 +75,7 @@ func TestIntegrationMissingHeader(t *testing.T) {
 	}
 }
 
-func threeRounds(transport *dnsoverhttps.Transport) error {
+func threeRounds(transport *Transport) error {
 	err := roundTrip(transport, "ooni.io.")
 	if err != nil {
 		return err
@@ -99,7 +91,7 @@ func threeRounds(transport *dnsoverhttps.Transport) error {
 	return nil
 }
 
-func roundTrip(transport *dnsoverhttps.Transport, domain string) error {
+func roundTrip(transport *Transport, domain string) error {
 	query := new(dns.Msg)
 	query.SetQuestion(domain, dns.TypeA)
 	data, err := query.Pack()

internal/godns/godns_test.go

@@ -2,6 +2,7 @@ package godns_test
 
 import (
 	"context"
+	"net/http"
 	"testing"
 	"time"
 
@@ -13,8 +14,7 @@ import (
 func TestIntegrationSuccess(t *testing.T) {
 	start := time.Now()
 	transport := dnsoverhttps.NewTransport(
-		start, handlers.NoHandler,
-		"https://cloudflare-dns.com/dns-query",
+		http.DefaultClient, "https://cloudflare-dns.com/dns-query",
 	)
 	client := godns.NewClient(start, handlers.NoHandler, transport)
 	addrs, err := client.LookupHost(context.Background(), "ooni.io")
@@ -29,8 +29,7 @@ func TestIntegrationSuccess(t *testing.T) {
 func TestIntegrationReadWithTimeout(t *testing.T) {
 	start := time.Now()
 	transport := dnsoverhttps.NewTransport(
-		start, handlers.NoHandler,
-		"https://cloudflare-dns.com/dns-query",
+		http.DefaultClient, "https://cloudflare-dns.com/dns-query",
 	)
 	conn := godns.NewPseudoConn(start, handlers.NoHandler, transport)
 	err := conn.SetDeadline(time.Now()) // very short deadline