diff --git a/internal/engine/netx/dialer/proxy.go b/internal/engine/netx/dialer/proxy.go index 3cd68c0..3d07320 100644 --- a/internal/engine/netx/dialer/proxy.go +++ b/internal/engine/netx/dialer/proxy.go @@ -17,6 +17,9 @@ type ProxyDialer struct { ProxyURL *url.URL } +// ErrProxyUnsupportedScheme indicates we don't support a protocol scheme. +var ErrProxyUnsupportedScheme = errors.New("proxy: unsupported scheme") + // DialContext implements Dialer.DialContext func (d ProxyDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { url := d.ProxyURL @@ -24,38 +27,18 @@ func (d ProxyDialer) DialContext(ctx context.Context, network, address string) ( return d.Dialer.DialContext(ctx, network, address) } if url.Scheme != "socks5" { - return nil, errors.New("Scheme is not socks5") + return nil, ErrProxyUnsupportedScheme } // the code at proxy/socks5.go never fails; see https://git.io/JfJ4g child, _ := proxy.SOCKS5( - network, url.Host, nil, proxyDialerWrapper{Dialer: d.Dialer}) + network, url.Host, nil, proxyDialerWrapper{d.Dialer}) return d.dial(ctx, child, network, address) } func (d ProxyDialer) dial( ctx context.Context, child proxy.Dialer, network, address string) (net.Conn, error) { - connch := make(chan net.Conn) - errch := make(chan error, 1) - go func() { - conn, err := child.Dial(network, address) - if err != nil { - errch <- err - return - } - select { - case connch <- conn: - default: - conn.Close() - } - }() - select { - case <-ctx.Done(): - return nil, ctx.Err() - case err := <-errch: - return nil, err - case conn := <-connch: - return conn, nil - } + cd := child.(proxy.ContextDialer) // will work + return cd.DialContext(ctx, network, address) } // proxyDialerWrapper is required because SOCKS5 expects a Dialer.Dial type but internally @@ -68,5 +51,5 @@ type proxyDialerWrapper struct { } func (d proxyDialerWrapper) Dial(network, address string) (net.Conn, error) { - return d.DialContext(context.Background(), network, address) + panic(errors.New("proxyDialerWrapper.Dial should not be called directly")) } diff --git a/internal/engine/netx/dialer/proxy_internal_test.go b/internal/engine/netx/dialer/proxy_internal_test.go deleted file mode 100644 index 063eef2..0000000 --- a/internal/engine/netx/dialer/proxy_internal_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package dialer - -import ( - "context" - "net" - - "golang.org/x/net/proxy" -) - -type ProxyDialerWrapper = proxyDialerWrapper - -func (d ProxyDialer) DialContextWithDialer( - ctx context.Context, child proxy.Dialer, network, address string) (net.Conn, error) { - return d.dial(ctx, child, network, address) -} diff --git a/internal/engine/netx/dialer/proxy_test.go b/internal/engine/netx/dialer/proxy_test.go index 600eda9..7172ee9 100644 --- a/internal/engine/netx/dialer/proxy_test.go +++ b/internal/engine/netx/dialer/proxy_test.go @@ -1,4 +1,4 @@ -package dialer_test +package dialer import ( "context" @@ -7,15 +7,13 @@ import ( "net" "net/url" "testing" - "time" - "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/mockablex" ) func TestProxyDialerDialContextNoProxyURL(t *testing.T) { expected := errors.New("mocked error") - d := dialer.ProxyDialer{ + d := ProxyDialer{ Dialer: mockablex.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { return nil, expected @@ -32,11 +30,11 @@ func TestProxyDialerDialContextNoProxyURL(t *testing.T) { } func TestProxyDialerDialContextInvalidScheme(t *testing.T) { - d := dialer.ProxyDialer{ + d := ProxyDialer{ ProxyURL: &url.URL{Scheme: "antani"}, } conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") - if err.Error() != "Scheme is not socks5" { + if !errors.Is(err, ErrProxyUnsupportedScheme) { t.Fatal("not the error we expected") } if conn != nil { @@ -45,13 +43,17 @@ func TestProxyDialerDialContextInvalidScheme(t *testing.T) { } func TestProxyDialerDialContextWithEOF(t *testing.T) { - d := dialer.ProxyDialer{ + const expect = "10.0.0.1:9050" + d := ProxyDialer{ Dialer: mockablex.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { + if address != expect { + return nil, errors.New("unexpected address") + } return nil, io.EOF }, }, - ProxyURL: &url.URL{Scheme: "socks5"}, + ProxyURL: &url.URL{Scheme: "socks5", Host: expect}, } conn, err := d.DialContext(context.Background(), "tcp", "www.google.com:443") if !errors.Is(err, io.EOF) { @@ -62,105 +64,18 @@ func TestProxyDialerDialContextWithEOF(t *testing.T) { } } -func TestProxyDialerDialContextWithContextCanceled(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() // immediately fail - d := dialer.ProxyDialer{ - Dialer: mockablex.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return nil, io.EOF - }, - }, - ProxyURL: &url.URL{Scheme: "socks5"}, - } - conn, err := d.DialContext(ctx, "tcp", "www.google.com:443") - if !errors.Is(err, context.Canceled) { - t.Fatal("not the error we expected") - } - if conn != nil { - t.Fatal("conn is not nil") - } -} - -func TestProxyDialerDialContextWithDialerSuccess(t *testing.T) { - d := dialer.ProxyDialer{ - Dialer: mockablex.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return &mockablex.Conn{ - MockRead: func(b []byte) (int, error) { - return 0, io.EOF - }, - MockWrite: func(b []byte) (int, error) { - return 0, io.EOF - }, - MockClose: func() error { - return io.EOF - }, - }, nil - }, - }, - ProxyURL: &url.URL{Scheme: "socks5"}, - } - conn, err := d.DialContextWithDialer( - context.Background(), dialer.ProxyDialerWrapper{ - Dialer: d.Dialer, - }, "tcp", "www.google.com:443") - if err != nil { - t.Fatal(err) - } - conn.Close() -} - -func TestProxyDialerDialContextWithDialerCanceledContext(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - // Stop immediately. The FakeDialer sleeps for some microseconds so - // it is much more likely we immediately exit with done context. The - // arm where we receive the conn is much less likely. - cancel() - d := dialer.ProxyDialer{ - Dialer: mockablex.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - time.Sleep(10 * time.Microsecond) - return &mockablex.Conn{ - MockRead: func(b []byte) (int, error) { - return 0, io.EOF - }, - MockWrite: func(b []byte) (int, error) { - return 0, io.EOF - }, - MockClose: func() error { - return io.EOF - }, - }, nil - }, - }, - ProxyURL: &url.URL{Scheme: "socks5"}, - } - conn, err := d.DialContextWithDialer( - ctx, dialer.ProxyDialerWrapper{ - Dialer: d.Dialer, - }, "tcp", "www.google.com:443") - if !errors.Is(err, context.Canceled) { - t.Fatal("not the error we expected") - } - if conn != nil { - t.Fatal("expected nil conn here") - } -} - -func TestProxyDialerWrapper(t *testing.T) { - d := dialer.ProxyDialerWrapper{ - Dialer: mockablex.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return nil, io.EOF - }, - }, - } - conn, err := d.Dial("tcp", "www.google.com:443") - if !errors.Is(err, io.EOF) { - t.Fatal("not the error we expected") - } - if conn != nil { - t.Fatal("conn is not nil") +func TestProxyDialWrapperPanics(t *testing.T) { + d := &proxyDialerWrapper{} + err := func() (rv error) { + defer func() { + if r := recover(); r != nil { + rv = r.(error) + } + }() + d.Dial("tcp", "10.0.0.1:1234") + return + }() + if err.Error() != "proxyDialerWrapper.Dial should not be called directly" { + t.Fatal("unexpected result", err) } }