Skip to content

Commit

Permalink
refactor(netxlite): add Transport suffix to DNS transports (#731)
Browse files Browse the repository at this point in the history 
This diff has been extracted from bassosimone/websteps-illustrated@c2f7cca

See ooni/probe#2096
  • Loading branch information
@bassosimone
bassosimone authored May 14, 2022
1 parent 6c388d2 commit f5b801a
Show file tree
Hide file tree
Showing 16 changed files with 94 additions and 94 deletions.
8 changes: 4 additions & 4 deletions internal/engine/experiment/urlgetter/configurer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestConfigurerNewConfigurationResolverDNSOverHTTPSPowerdns(t *testing.T) {
if !ok {
t.Fatal("not the DNS transport we expected")
}
dohtxp, ok := stxp.DNSTransport.(*netxlite.DNSOverHTTPS)
dohtxp, ok := stxp.DNSTransport.(*netxlite.DNSOverHTTPSTransport)
if !ok {
t.Fatal("not the DNS transport we expected")
}
Expand Down Expand Up @@ -200,7 +200,7 @@ func TestConfigurerNewConfigurationResolverDNSOverHTTPSGoogle(t *testing.T) {
if !ok {
t.Fatal("not the DNS transport we expected")
}
dohtxp, ok := stxp.DNSTransport.(*netxlite.DNSOverHTTPS)
dohtxp, ok := stxp.DNSTransport.(*netxlite.DNSOverHTTPSTransport)
if !ok {
t.Fatal("not the DNS transport we expected")
}
Expand Down Expand Up @@ -276,7 +276,7 @@ func TestConfigurerNewConfigurationResolverDNSOverHTTPSCloudflare(t *testing.T)
if !ok {
t.Fatal("not the DNS transport we expected")
}
dohtxp, ok := stxp.DNSTransport.(*netxlite.DNSOverHTTPS)
dohtxp, ok := stxp.DNSTransport.(*netxlite.DNSOverHTTPSTransport)
if !ok {
t.Fatal("not the DNS transport we expected")
}
Expand Down Expand Up @@ -352,7 +352,7 @@ func TestConfigurerNewConfigurationResolverUDP(t *testing.T) {
if !ok {
t.Fatal("not the DNS transport we expected")
}
udptxp, ok := stxp.DNSTransport.(*netxlite.DNSOverUDP)
udptxp, ok := stxp.DNSTransport.(*netxlite.DNSOverUDPTransport)
if !ok {
t.Fatal("not the DNS transport we expected")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/experiment/webstepsx/th.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,5 +368,5 @@ const thResolverURL = "https://dns.google/dns-query"
// Here we're using github.com/apex/log as the logger, which
// is fine because this is backend only code.
var thResolver = netxlite.WrapResolver(log.Log, netxlite.NewSerialResolver(
netxlite.NewDNSOverHTTPS(http.DefaultClient, thResolverURL),
netxlite.NewDNSOverHTTPSTransport(http.DefaultClient, thResolverURL),
))
6 changes: 3 additions & 3 deletions internal/engine/netx/netx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
case "https":
config.TLSConfig.NextProtos = []string{"h2", "http/1.1"}
httpClient := &http.Client{Transport: NewHTTPTransport(config)}
var txp model.DNSTransport = netxlite.NewDNSOverHTTPSWithHostOverride(
var txp model.DNSTransport = netxlite.NewDNSOverHTTPSTransportWithHostOverride(
httpClient, URL, hostOverride)
if config.ResolveSaver != nil {
txp = resolver.SaverDNSTransport{
Expand All @@ -301,7 +301,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
if err != nil {
return nil, err
}
var txp model.DNSTransport = netxlite.NewDNSOverUDP(
var txp model.DNSTransport = netxlite.NewDNSOverUDPTransport(
dialer, endpoint)
if config.ResolveSaver != nil {
txp = resolver.SaverDNSTransport{
Expand Down Expand Up @@ -332,7 +332,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
if err != nil {
return nil, err
}
var txp model.DNSTransport = netxlite.NewDNSOverTCP(
var txp model.DNSTransport = netxlite.NewDNSOverTCPTransport(
dialer.DialContext, endpoint)
if config.ResolveSaver != nil {
txp = resolver.SaverDNSTransport{
Expand Down
20 changes: 10 additions & 10 deletions internal/engine/netx/netx_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ func TestNewDNSClientPowerdnsDoH(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := r.Transport().(*netxlite.DNSOverHTTPS); !ok {
if _, ok := r.Transport().(*netxlite.DNSOverHTTPSTransport); !ok {
t.Fatal("not the transport we expected")
}
dnsclient.CloseIdleConnections()
Expand All @@ -602,7 +602,7 @@ func TestNewDNSClientGoogleDoH(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := r.Transport().(*netxlite.DNSOverHTTPS); !ok {
if _, ok := r.Transport().(*netxlite.DNSOverHTTPSTransport); !ok {
t.Fatal("not the transport we expected")
}
dnsclient.CloseIdleConnections()
Expand All @@ -618,7 +618,7 @@ func TestNewDNSClientCloudflareDoH(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := r.Transport().(*netxlite.DNSOverHTTPS); !ok {
if _, ok := r.Transport().(*netxlite.DNSOverHTTPSTransport); !ok {
t.Fatal("not the transport we expected")
}
dnsclient.CloseIdleConnections()
Expand All @@ -639,7 +639,7 @@ func TestNewDNSClientCloudflareDoHSaver(t *testing.T) {
if !ok {
t.Fatal("not the transport we expected")
}
if _, ok := txp.DNSTransport.(*netxlite.DNSOverHTTPS); !ok {
if _, ok := txp.DNSTransport.(*netxlite.DNSOverHTTPSTransport); !ok {
t.Fatal("not the transport we expected")
}
dnsclient.CloseIdleConnections()
Expand All @@ -655,7 +655,7 @@ func TestNewDNSClientUDP(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
if _, ok := r.Transport().(*netxlite.DNSOverUDP); !ok {
if _, ok := r.Transport().(*netxlite.DNSOverUDPTransport); !ok {
t.Fatal("not the transport we expected")
}
dnsclient.CloseIdleConnections()
Expand All @@ -676,7 +676,7 @@ func TestNewDNSClientUDPDNSSaver(t *testing.T) {
if !ok {
t.Fatal("not the transport we expected")
}
if _, ok := txp.DNSTransport.(*netxlite.DNSOverUDP); !ok {
if _, ok := txp.DNSTransport.(*netxlite.DNSOverUDPTransport); !ok {
t.Fatal("not the transport we expected")
}
dnsclient.CloseIdleConnections()
Expand All @@ -692,7 +692,7 @@ func TestNewDNSClientTCP(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
txp, ok := r.Transport().(*netxlite.DNSOverTCP)
txp, ok := r.Transport().(*netxlite.DNSOverTCPTransport)
if !ok {
t.Fatal("not the transport we expected")
}
Expand All @@ -717,7 +717,7 @@ func TestNewDNSClientTCPDNSSaver(t *testing.T) {
if !ok {
t.Fatal("not the transport we expected")
}
dotcp, ok := txp.DNSTransport.(*netxlite.DNSOverTCP)
dotcp, ok := txp.DNSTransport.(*netxlite.DNSOverTCPTransport)
if !ok {
t.Fatal("not the transport we expected")
}
Expand All @@ -737,7 +737,7 @@ func TestNewDNSClientDoT(t *testing.T) {
if !ok {
t.Fatal("not the resolver we expected")
}
txp, ok := r.Transport().(*netxlite.DNSOverTCP)
txp, ok := r.Transport().(*netxlite.DNSOverTCPTransport)
if !ok {
t.Fatal("not the transport we expected")
}
Expand All @@ -762,7 +762,7 @@ func TestNewDNSClientDoTDNSSaver(t *testing.T) {
if !ok {
t.Fatal("not the transport we expected")
}
dotls, ok := txp.DNSTransport.(*netxlite.DNSOverTCP)
dotls, ok := txp.DNSTransport.(*netxlite.DNSOverTCPTransport)
if !ok {
t.Fatal("not the transport we expected")
}
Expand Down
10 changes: 5 additions & 5 deletions internal/engine/netx/resolver/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,28 +71,28 @@ func TestNewResolverSystem(t *testing.T) {

func TestNewResolverUDPAddress(t *testing.T) {
reso := netxlite.NewSerialResolver(
netxlite.NewDNSOverUDP(netxlite.DefaultDialer, "8.8.8.8:53"))
netxlite.NewDNSOverUDPTransport(netxlite.DefaultDialer, "8.8.8.8:53"))
testresolverquick(t, reso)
testresolverquickidna(t, reso)
}

func TestNewResolverUDPDomain(t *testing.T) {
reso := netxlite.NewSerialResolver(
netxlite.NewDNSOverUDP(netxlite.DefaultDialer, "dns.google.com:53"))
netxlite.NewDNSOverUDPTransport(netxlite.DefaultDialer, "dns.google.com:53"))
testresolverquick(t, reso)
testresolverquickidna(t, reso)
}

func TestNewResolverTCPAddress(t *testing.T) {
reso := netxlite.NewSerialResolver(
netxlite.NewDNSOverTCP(new(net.Dialer).DialContext, "8.8.8.8:53"))
netxlite.NewDNSOverTCPTransport(new(net.Dialer).DialContext, "8.8.8.8:53"))
testresolverquick(t, reso)
testresolverquickidna(t, reso)
}

func TestNewResolverTCPDomain(t *testing.T) {
reso := netxlite.NewSerialResolver(
netxlite.NewDNSOverTCP(new(net.Dialer).DialContext, "dns.google.com:53"))
netxlite.NewDNSOverTCPTransport(new(net.Dialer).DialContext, "dns.google.com:53"))
testresolverquick(t, reso)
testresolverquickidna(t, reso)
}
Expand All @@ -113,7 +113,7 @@ func TestNewResolverDoTDomain(t *testing.T) {

func TestNewResolverDoH(t *testing.T) {
reso := netxlite.NewSerialResolver(
netxlite.NewDNSOverHTTPS(http.DefaultClient, "https://cloudflare-dns.com/dns-query"))
netxlite.NewDNSOverHTTPSTransport(http.DefaultClient, "https://cloudflare-dns.com/dns-query"))
testresolverquick(t, reso)
testresolverquickidna(t, reso)
}
2 changes: 1 addition & 1 deletion internal/measurex/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (mx *Measurer) NewResolverSystem(db WritableDB, logger model.Logger) model.
func (mx *Measurer) NewResolverUDP(db WritableDB, logger model.Logger, address string) model.Resolver {
return mx.WrapResolver(db, netxlite.WrapResolver(
logger, netxlite.NewSerialResolver(
mx.WrapDNSXRoundTripper(db, netxlite.NewDNSOverUDP(
mx.WrapDNSXRoundTripper(db, netxlite.NewDNSOverUDPTransport(
mx.NewDialerWithSystemResolver(db, logger),
address,
)))),
Expand Down
2 changes: 1 addition & 1 deletion internal/netxlite/classify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ const (
)

// These errors are returned by custom DNSTransport instances (e.g.,
// DNSOverHTTPS and DNSOverUDP). Their suffix matches the equivalent
// DNSOverHTTPSTransport and DNSOverUDPTransport). Their suffix matches the equivalent
// unexported errors used by the Go standard library.
var (
ErrOODNSNoSuchHost = fmt.Errorf("ooniresolver: %s", DNSNoSuchHostSuffix)
Expand Down
30 changes: 15 additions & 15 deletions internal/netxlite/dnsoverhttps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
"github.com/ooni/probe-cli/v3/internal/model"
)

// DNSOverHTTPS is a DNS-over-HTTPS DNSTransport.
type DNSOverHTTPS struct {
// DNSOverHTTPSTransport is a DNS-over-HTTPS DNSTransport.
type DNSOverHTTPSTransport struct {
// Client is the MANDATORY http client to use.
Client model.HTTPClient

Expand All @@ -24,26 +24,26 @@ type DNSOverHTTPS struct {
HostOverride string
}

// NewDNSOverHTTPS creates a new DNSOverHTTPS instance.
// NewDNSOverHTTPSTransport creates a new DNSOverHTTPSTransport instance.
//
// Arguments:
//
// - client in http.Client-like type (e.g., http.DefaultClient);
//
// - URL is the DoH resolver URL (e.g., https://1.1.1.1/dns-query).
func NewDNSOverHTTPS(client model.HTTPClient, URL string) *DNSOverHTTPS {
return NewDNSOverHTTPSWithHostOverride(client, URL, "")
func NewDNSOverHTTPSTransport(client model.HTTPClient, URL string) *DNSOverHTTPSTransport {
return NewDNSOverHTTPSTransportWithHostOverride(client, URL, "")
}

// NewDNSOverHTTPSWithHostOverride creates a new DNSOverHTTPS
// NewDNSOverHTTPSTransportWithHostOverride creates a new DNSOverHTTPSTransport
// with the given Host header override.
func NewDNSOverHTTPSWithHostOverride(
client model.HTTPClient, URL, hostOverride string) *DNSOverHTTPS {
return &DNSOverHTTPS{Client: client, URL: URL, HostOverride: hostOverride}
func NewDNSOverHTTPSTransportWithHostOverride(
client model.HTTPClient, URL, hostOverride string) *DNSOverHTTPSTransport {
return &DNSOverHTTPSTransport{Client: client, URL: URL, HostOverride: hostOverride}
}

// RoundTrip sends a query and receives a reply.
func (t *DNSOverHTTPS) RoundTrip(ctx context.Context, query []byte) ([]byte, error) {
func (t *DNSOverHTTPSTransport) RoundTrip(ctx context.Context, query []byte) ([]byte, error) {
ctx, cancel := context.WithTimeout(ctx, 45*time.Second)
defer cancel()
req, err := http.NewRequest("POST", t.URL, bytes.NewReader(query))
Expand Down Expand Up @@ -71,23 +71,23 @@ func (t *DNSOverHTTPS) RoundTrip(ctx context.Context, query []byte) ([]byte, err
}

// RequiresPadding returns true for DoH according to RFC8467.
func (t *DNSOverHTTPS) RequiresPadding() bool {
func (t *DNSOverHTTPSTransport) RequiresPadding() bool {
return true
}

// Network returns the transport network, i.e., "doh".
func (t *DNSOverHTTPS) Network() string {
func (t *DNSOverHTTPSTransport) Network() string {
return "doh"
}

// Address returns the URL we're using for the DoH server.
func (t *DNSOverHTTPS) Address() string {
func (t *DNSOverHTTPSTransport) Address() string {
return t.URL
}

// CloseIdleConnections closes idle connections, if any.
func (t *DNSOverHTTPS) CloseIdleConnections() {
func (t *DNSOverHTTPSTransport) CloseIdleConnections() {
t.Client.CloseIdleConnections()
}

var _ model.DNSTransport = &DNSOverHTTPS{}
var _ model.DNSTransport = &DNSOverHTTPSTransport{}
20 changes: 10 additions & 10 deletions internal/netxlite/dnsoverhttps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import (
"github.com/ooni/probe-cli/v3/internal/model/mocks"
)

func TestDNSOverHTTPS(t *testing.T) {
func TestDNSOverHTTPSTransport(t *testing.T) {
t.Run("RoundTrip", func(t *testing.T) {
t.Run("NewRequestFailure", func(t *testing.T) {
const invalidURL = "\t"
txp := NewDNSOverHTTPS(http.DefaultClient, invalidURL)
txp := NewDNSOverHTTPSTransport(http.DefaultClient, invalidURL)
data, err := txp.RoundTrip(context.Background(), nil)
if err == nil || !strings.HasSuffix(err.Error(), "invalid control character in URL") {
t.Fatal("expected an error here")
Expand All @@ -29,7 +29,7 @@ func TestDNSOverHTTPS(t *testing.T) {

t.Run("client.Do failure", func(t *testing.T) {
expected := errors.New("mocked error")
txp := &DNSOverHTTPS{
txp := &DNSOverHTTPSTransport{
Client: &mocks.HTTPClient{
MockDo: func(*http.Request) (*http.Response, error) {
return nil, expected
Expand All @@ -47,7 +47,7 @@ func TestDNSOverHTTPS(t *testing.T) {
})

t.Run("server returns 500", func(t *testing.T) {
txp := &DNSOverHTTPS{
txp := &DNSOverHTTPSTransport{
Client: &mocks.HTTPClient{
MockDo: func(*http.Request) (*http.Response, error) {
return &http.Response{
Expand All @@ -68,7 +68,7 @@ func TestDNSOverHTTPS(t *testing.T) {
})

t.Run("missing content type", func(t *testing.T) {
txp := &DNSOverHTTPS{
txp := &DNSOverHTTPSTransport{
Client: &mocks.HTTPClient{
MockDo: func(*http.Request) (*http.Response, error) {
return &http.Response{
Expand All @@ -90,7 +90,7 @@ func TestDNSOverHTTPS(t *testing.T) {

t.Run("success", func(t *testing.T) {
body := []byte("AAA")
txp := &DNSOverHTTPS{
txp := &DNSOverHTTPSTransport{
Client: &mocks.HTTPClient{
MockDo: func(*http.Request) (*http.Response, error) {
return &http.Response{
Expand All @@ -116,7 +116,7 @@ func TestDNSOverHTTPS(t *testing.T) {
t.Run("sets the correct user-agent", func(t *testing.T) {
expected := errors.New("mocked error")
var correct bool
txp := &DNSOverHTTPS{
txp := &DNSOverHTTPSTransport{
Client: &mocks.HTTPClient{
MockDo: func(req *http.Request) (*http.Response, error) {
correct = req.Header.Get("User-Agent") == httpheader.UserAgent()
Expand All @@ -141,7 +141,7 @@ func TestDNSOverHTTPS(t *testing.T) {
var correct bool
expected := errors.New("mocked error")
hostOverride := "test.com"
txp := &DNSOverHTTPS{
txp := &DNSOverHTTPSTransport{
Client: &mocks.HTTPClient{
MockDo: func(req *http.Request) (*http.Response, error) {
correct = req.Host == hostOverride
Expand All @@ -167,7 +167,7 @@ func TestDNSOverHTTPS(t *testing.T) {

t.Run("other functions behave correctly", func(t *testing.T) {
const queryURL = "https://cloudflare-dns.com/dns-query"
txp := NewDNSOverHTTPS(http.DefaultClient, queryURL)
txp := NewDNSOverHTTPSTransport(http.DefaultClient, queryURL)
if txp.Network() != "doh" {
t.Fatal("invalid network")
}
Expand All @@ -181,7 +181,7 @@ func TestDNSOverHTTPS(t *testing.T) {

t.Run("CloseIdleConnections", func(t *testing.T) {
var called bool
doh := &DNSOverHTTPS{
doh := &DNSOverHTTPSTransport{
Client: &mocks.HTTPClient{
MockCloseIdleConnections: func() {
called = true
Expand Down
Loading

0 comments on commit f5b801a

Please sign in to comment.