diff --git a/internal/engine/experiment/ndt7/dial.go b/internal/engine/experiment/ndt7/dial.go index 18d22d5..3829da6 100644 --- a/internal/engine/experiment/ndt7/dial.go +++ b/internal/engine/experiment/ndt7/dial.go @@ -36,7 +36,6 @@ func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (* var reso resolver.Resolver = resolver.SystemResolver{} reso = resolver.LoggingResolver{Resolver: reso, Logger: mgr.logger} var dlr dialer.Dialer = dialer.Default - dlr = dialer.TimeoutDialer{Dialer: dlr} dlr = dialer.ErrorWrapperDialer{Dialer: dlr} dlr = dialer.LoggingDialer{Dialer: dlr, Logger: mgr.logger} dlr = dialer.DNSDialer{Dialer: dlr, Resolver: reso} diff --git a/internal/engine/legacy/netx/dialer.go b/internal/engine/legacy/netx/dialer.go index 508a655..ffc23f5 100644 --- a/internal/engine/legacy/netx/dialer.go +++ b/internal/engine/legacy/netx/dialer.go @@ -60,19 +60,16 @@ func maybeWithMeasurementRoot( // - DNSDialer (topmost) // - EmitterDialer // - ErrorWrapperDialer -// - TimeoutDialer // - ByteCountingDialer -// - net.Dialer +// - dialer.Default // // If you have others needs, manually build the chain you need. func newDNSDialer(resolver dialer.Resolver) dialer.DNSDialer { return dialer.DNSDialer{ Dialer: dialer.EmitterDialer{ Dialer: dialer.ErrorWrapperDialer{ - Dialer: dialer.TimeoutDialer{ - Dialer: dialer.ByteCounterDialer{ - Dialer: new(net.Dialer), - }, + Dialer: dialer.ByteCounterDialer{ + Dialer: dialer.Default, }, }, }, diff --git a/internal/engine/netx/dialer/system.go b/internal/engine/netx/dialer/system.go index cb8f0ad..ea22148 100644 --- a/internal/engine/netx/dialer/system.go +++ b/internal/engine/netx/dialer/system.go @@ -6,8 +6,8 @@ import ( "time" ) -// defaultNetDialer is the net.Dialer we use by default. -var defaultNetDialer = &net.Dialer{ +// underlyingDialer is the underlying net.Dialer. +var underlyingDialer = &net.Dialer{ Timeout: 15 * time.Second, KeepAlive: 15 * time.Second, } @@ -17,7 +17,7 @@ type SystemDialer struct{} // DialContext implements Dialer.DialContext func (d SystemDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - return defaultNetDialer.DialContext(ctx, network, address) + return underlyingDialer.DialContext(ctx, network, address) } // Default is the dialer we use by default. diff --git a/internal/engine/netx/dialer/system_test.go b/internal/engine/netx/dialer/system_test.go index 25f6e7e..08bbbcf 100644 --- a/internal/engine/netx/dialer/system_test.go +++ b/internal/engine/netx/dialer/system_test.go @@ -3,11 +3,12 @@ package dialer import ( "strings" "testing" + "time" "github.com/ooni/psiphon/oopsi/golang.org/x/net/context" ) -func TestSystemDialer(t *testing.T) { +func TestSystemDialerWorks(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // fail immediately conn, err := Default.DialContext(ctx, "tcp", "8.8.8.8:853") @@ -18,3 +19,10 @@ func TestSystemDialer(t *testing.T) { t.Fatal("expected nil conn here") } } + +func TestUnderlyingDialerHasTimeout(t *testing.T) { + expected := 15 * time.Second + if underlyingDialer.Timeout != expected { + t.Fatal("unexpected timeout value") + } +} diff --git a/internal/engine/netx/dialer/timeout.go b/internal/engine/netx/dialer/timeout.go deleted file mode 100644 index 36b24a2..0000000 --- a/internal/engine/netx/dialer/timeout.go +++ /dev/null @@ -1,24 +0,0 @@ -package dialer - -import ( - "context" - "net" - "time" -) - -// TimeoutDialer is a Dialer that enforces a timeout -type TimeoutDialer struct { - Dialer - ConnectTimeout time.Duration // default: 30 seconds -} - -// DialContext implements Dialer.DialContext -func (d TimeoutDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - timeout := 30 * time.Second - if d.ConnectTimeout != 0 { - timeout = d.ConnectTimeout - } - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - return d.Dialer.DialContext(ctx, network, address) -} diff --git a/internal/engine/netx/dialer/timeout_test.go b/internal/engine/netx/dialer/timeout_test.go deleted file mode 100644 index 62bb6bb..0000000 --- a/internal/engine/netx/dialer/timeout_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package dialer_test - -import ( - "context" - "errors" - "io" - "net" - "testing" - "time" - - "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" -) - -type SlowDialer struct{} - -func (SlowDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-time.After(30 * time.Second): - return nil, io.EOF - } -} - -func TestTimeoutDialer(t *testing.T) { - d := dialer.TimeoutDialer{Dialer: SlowDialer{}, ConnectTimeout: time.Second} - conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") - if !errors.Is(err, context.DeadlineExceeded) { - t.Fatal("not the error we expected") - } - if conn != nil { - t.Fatal("expected nil conn here") - } -} diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 40fc4a6..8951da7 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -147,7 +147,6 @@ func NewDialer(config Config) Dialer { config.FullResolver = NewResolver(config) } var d Dialer = dialer.Default - d = dialer.TimeoutDialer{Dialer: d} d = dialer.ErrorWrapperDialer{Dialer: d} if config.Logger != nil { d = dialer.LoggingDialer{Dialer: d, Logger: config.Logger} diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index 84fda72..6504277 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -241,11 +241,7 @@ func TestNewDialerVanilla(t *testing.T) { if !ok { t.Fatal("not the dialer we expected") } - td, ok := ewd.Dialer.(dialer.TimeoutDialer) - if !ok { - t.Fatal("not the dialer we expected") - } - if _, ok := td.Dialer.(dialer.SystemDialer); !ok { + if _, ok := ewd.Dialer.(dialer.SystemDialer); !ok { t.Fatal("not the dialer we expected") } } @@ -281,11 +277,7 @@ func TestNewDialerWithResolver(t *testing.T) { if !ok { t.Fatal("not the dialer we expected") } - td, ok := ewd.Dialer.(dialer.TimeoutDialer) - if !ok { - t.Fatal("not the dialer we expected") - } - if _, ok := td.Dialer.(dialer.SystemDialer); !ok { + if _, ok := ewd.Dialer.(dialer.SystemDialer); !ok { t.Fatal("not the dialer we expected") } } @@ -330,11 +322,7 @@ func TestNewDialerWithLogger(t *testing.T) { if !ok { t.Fatal("not the dialer we expected") } - td, ok := ewd.Dialer.(dialer.TimeoutDialer) - if !ok { - t.Fatal("not the dialer we expected") - } - if _, ok := td.Dialer.(dialer.SystemDialer); !ok { + if _, ok := ewd.Dialer.(dialer.SystemDialer); !ok { t.Fatal("not the dialer we expected") } } @@ -380,11 +368,7 @@ func TestNewDialerWithDialSaver(t *testing.T) { if !ok { t.Fatal("not the dialer we expected") } - td, ok := ewd.Dialer.(dialer.TimeoutDialer) - if !ok { - t.Fatal("not the dialer we expected") - } - if _, ok := td.Dialer.(dialer.SystemDialer); !ok { + if _, ok := ewd.Dialer.(dialer.SystemDialer); !ok { t.Fatal("not the dialer we expected") } } @@ -430,11 +414,7 @@ func TestNewDialerWithReadWriteSaver(t *testing.T) { if !ok { t.Fatal("not the dialer we expected") } - td, ok := ewd.Dialer.(dialer.TimeoutDialer) - if !ok { - t.Fatal("not the dialer we expected") - } - if _, ok := td.Dialer.(dialer.SystemDialer); !ok { + if _, ok := ewd.Dialer.(dialer.SystemDialer); !ok { t.Fatal("not the dialer we expected") } } @@ -476,11 +456,7 @@ func TestNewDialerWithContextByteCounting(t *testing.T) { if !ok { t.Fatal("not the dialer we expected") } - td, ok := ewd.Dialer.(dialer.TimeoutDialer) - if !ok { - t.Fatal("not the dialer we expected") - } - if _, ok := td.Dialer.(dialer.SystemDialer); !ok { + if _, ok := ewd.Dialer.(dialer.SystemDialer); !ok { t.Fatal("not the dialer we expected") } }