From 2e747a70792cfbe6fac8fee7a7b7fb0cb4416721 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 11 Oct 2019 15:45:02 +0200 Subject: [PATCH 1/6] httpx.go: fix documentation comments This is purely yak shaving. I encountered these incorrect comments while doing other work and decided to fix them. --- httpx/httpx.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/httpx/httpx.go b/httpx/httpx.go index eff8bb0..d0f332b 100644 --- a/httpx/httpx.go +++ b/httpx/httpx.go @@ -1,6 +1,6 @@ // Package httpx contains OONI's net/http extensions. It defines the Client and // the Transport replacements that we should use in OONI. They emit measurements -// collected at network and HTTP level on a specific channel. +// collected at network and HTTP level using a specific handler. package httpx import ( @@ -20,8 +20,7 @@ type Transport struct { } // NewTransport creates a new Transport. The beginning argument is -// the time to use as zero for computing the elapsed time. The ch -// channel is where we'll emit Measurements. +// the time to use as zero for computing the elapsed time. func NewTransport(beginning time.Time, handler model.Handler) *Transport { t := new(Transport) t.dialer = dialerapi.NewDialer(beginning, handler) From 0d39e20c08af2f94e4eda247418d1daa252e8615 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 11 Oct 2019 15:45:47 +0200 Subject: [PATCH 2/6] Write constructor for dialerbase.Dialer Again yak shaving. This just makes the code more tidy. It was bothering me a little bit that we needed to know what special values to set in order to instantiate a correct dialerbase.Dialer. --- internal/dialerapi/dialerapi.go | 8 +++----- internal/dialerbase/dialerbase.go | 9 +++++++++ internal/dialerbase/dialerbase_test.go | 19 ++++++++++--------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/internal/dialerapi/dialerapi.go b/internal/dialerapi/dialerapi.go index a662587..503cd78 100644 --- a/internal/dialerapi/dialerapi.go +++ b/internal/dialerapi/dialerapi.go @@ -49,11 +49,9 @@ type Dialer struct { // NewDialer creates a new Dialer. func NewDialer(beginning time.Time, handler model.Handler) (d *Dialer) { d = &Dialer{ - Dialer: dialerbase.Dialer{ - Beginning: beginning, - Dialer: net.Dialer{}, - Handler: handler, - }, + Dialer: dialerbase.NewDialer( + beginning, handler, + ), Handler: handler, TLSConfig: &tls.Config{}, StartTLSHandshakeHook: func(net.Conn) {}, diff --git a/internal/dialerbase/dialerbase.go b/internal/dialerbase/dialerbase.go index 496137f..d6e0348 100644 --- a/internal/dialerbase/dialerbase.go +++ b/internal/dialerbase/dialerbase.go @@ -20,6 +20,15 @@ type Dialer struct { Handler model.Handler } +// NewDialer creates a new base dialer +func NewDialer(beginning time.Time, handler model.Handler) Dialer { + return Dialer{ + Dialer: net.Dialer{}, + Beginning: beginning, + Handler: handler, + } +} + // DialHostPort is like net.DialContext but requires a separate host // and port and returns a measurable net.Conn-like struct. func (d *Dialer) DialHostPort( diff --git a/internal/dialerbase/dialerbase_test.go b/internal/dialerbase/dialerbase_test.go index 9e552e3..8c21dc6 100644 --- a/internal/dialerbase/dialerbase_test.go +++ b/internal/dialerbase/dialerbase_test.go @@ -3,15 +3,16 @@ package dialerbase_test import ( "context" "testing" + "time" "github.com/ooni/netx/handlers" "github.com/ooni/netx/internal/dialerbase" ) func TestIntegrationSuccess(t *testing.T) { - dialer := dialerbase.Dialer{ - Handler: handlers.NoHandler, - } + dialer := dialerbase.NewDialer( + time.Now(), handlers.NoHandler, + ) conn, err := dialer.DialHostPort( context.Background(), "tcp", "8.8.8.8", "53", 17, ) @@ -22,9 +23,9 @@ func TestIntegrationSuccess(t *testing.T) { } func TestIntegrationErrorDomain(t *testing.T) { - dialer := dialerbase.Dialer{ - Handler: handlers.NoHandler, - } + dialer := dialerbase.NewDialer( + time.Now(), handlers.NoHandler, + ) conn, err := dialer.DialHostPort( context.Background(), "tcp", "dns.google.com", "53", 17, ) @@ -37,9 +38,9 @@ func TestIntegrationErrorDomain(t *testing.T) { } func TestIntegrationErrorNoConnect(t *testing.T) { - dialer := dialerbase.Dialer{ - Handler: handlers.NoHandler, - } + dialer := dialerbase.NewDialer( + time.Now(), handlers.NoHandler, + ) ctx, cancel := context.WithTimeout(context.Background(), 1) defer cancel() conn, err := dialer.DialHostPort( From d69ff41dd73c777fa54654540f491c94aeb19b82 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 11 Oct 2019 16:08:54 +0200 Subject: [PATCH 3/6] dialerapi: compose rather than embedding dialerbase This is again yak shaving. With composition I am hoping to further decouple dialerbase and dialerapi. --- internal/dialerapi/dialerapi.go | 12 +++++++----- internal/dialerbase/dialerbase.go | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/dialerapi/dialerapi.go b/internal/dialerapi/dialerapi.go index 503cd78..e25a08f 100644 --- a/internal/dialerapi/dialerapi.go +++ b/internal/dialerapi/dialerapi.go @@ -37,31 +37,33 @@ type DialHostPortFunc func( // Dialer defines the dialer API. We implement the most basic form // of DNS, but more advanced resolutions are possible. type Dialer struct { - dialerbase.Dialer + Beginning time.Time DialHostPort DialHostPortFunc Handler model.Handler LookupHost LookupHostFunc StartTLSHandshakeHook func(net.Conn) TLSConfig *tls.Config TLSHandshakeTimeout time.Duration + dialer *dialerbase.Dialer } // NewDialer creates a new Dialer. func NewDialer(beginning time.Time, handler model.Handler) (d *Dialer) { d = &Dialer{ - Dialer: dialerbase.NewDialer( - beginning, handler, - ), + Beginning: beginning, Handler: handler, TLSConfig: &tls.Config{}, StartTLSHandshakeHook: func(net.Conn) {}, + dialer: dialerbase.NewDialer( + beginning, handler, + ), } // This is equivalent to ConfigureDNS("system", "...") r := &net.Resolver{ PreferGo: false, } d.LookupHost = r.LookupHost - d.DialHostPort = d.Dialer.DialHostPort + d.DialHostPort = d.dialer.DialHostPort return } diff --git a/internal/dialerbase/dialerbase.go b/internal/dialerbase/dialerbase.go index d6e0348..b0ecd25 100644 --- a/internal/dialerbase/dialerbase.go +++ b/internal/dialerbase/dialerbase.go @@ -21,8 +21,8 @@ type Dialer struct { } // NewDialer creates a new base dialer -func NewDialer(beginning time.Time, handler model.Handler) Dialer { - return Dialer{ +func NewDialer(beginning time.Time, handler model.Handler) *Dialer { + return &Dialer{ Dialer: net.Dialer{}, Beginning: beginning, Handler: handler, From 50f21775a1b5753ca23d2c3b61fd4baba76f1e84 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 15 Oct 2019 13:50:25 +0200 Subject: [PATCH 4/6] netx.go: fix ConfigureDNS documentation --- netx.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/netx.go b/netx.go index a730533..4537cc0 100644 --- a/netx.go +++ b/netx.go @@ -60,12 +60,12 @@ func NewDialer(handler model.Handler) *Dialer { // // For example: // -// d.SetResolver("system", "") -// d.SetResolver("godns", "") -// d.SetResolver("udp", "8.8.8.8:53") -// d.SetResolver("tcp", "8.8.8.8:53") -// d.SetResolver("dot", "dns.quad9.net") -// d.SetResolver("doh", "https://cloudflare-dns.com/dns-query") +// d.ConfigureDNS("system", "") +// d.ConfigureDNS("godns", "") +// d.ConfigureDNS("udp", "8.8.8.8:53") +// d.ConfigureDNS("tcp", "8.8.8.8:53") +// d.ConfigureDNS("dot", "dns.quad9.net") +// d.ConfigureDNS("doh", "https://cloudflare-dns.com/dns-query") // // ConfigureDNS is currently only executed when Go chooses to // use the pure Go implementation of the DNS. This means that it From 780af4ddcd8582b5631d52e94b18f37dd34d5d2e Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 15 Oct 2019 13:52:34 +0200 Subject: [PATCH 5/6] netx.go: fix spelling issue in a comment --- netx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netx.go b/netx.go index 4537cc0..768bf40 100644 --- a/netx.go +++ b/netx.go @@ -108,7 +108,7 @@ func (d *Dialer) NewResolver(network, address string) (dnsx.Client, error) { } // SetCABundle configures the dialer to use a specific CA bundle. This -// function is not goroutine safe. Make sure you call it befor starting +// function is not goroutine safe. Make sure you call it before starting // to use this specific dialer. func (d *Dialer) SetCABundle(path string) error { return d.dialer.SetCABundle(path) From 806c2c930a647e2cef2abe22c87cce2cf51b63ba Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 15 Oct 2019 14:11:19 +0200 Subject: [PATCH 6/6] dnsx/dnsx.go: fix grammar --- dnsx/dnsx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dnsx/dnsx.go b/dnsx/dnsx.go index addba96..f23a88b 100644 --- a/dnsx/dnsx.go +++ b/dnsx/dnsx.go @@ -25,7 +25,7 @@ type Client interface { LookupNS(ctx context.Context, name string) ([]*net.NS, error) } -// RoundTripper represent an abstract DNS transport. +// RoundTripper represents an abstract DNS transport. type RoundTripper interface { // RoundTrip sends a DNS query and receives the reply. RoundTrip(query []byte) (reply []byte, err error)