From 863899469e4158e3e7fc1d85a2b4d1276d88b38a Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 1 Jul 2021 18:00:09 +0200 Subject: [PATCH] refactor: move ErrorWrapperTLSHandshaker to errorsx (#418) Part of https://github.com/ooni/probe/issues/1505 --- internal/engine/legacy/netx/dialer.go | 3 +- internal/engine/netx/netx.go | 3 +- internal/engine/netx/netx_test.go | 13 +++---- internal/engine/netx/tlsdialer/tls.go | 19 ---------- internal/engine/netx/tlsdialer/tls_test.go | 23 ------------ internal/errorsx/tls.go | 31 ++++++++++++++++ internal/errorsx/tls_test.go | 42 ++++++++++++++++++++++ 7 files changed, 84 insertions(+), 50 deletions(-) create mode 100644 internal/errorsx/tls.go create mode 100644 internal/errorsx/tls_test.go diff --git a/internal/engine/legacy/netx/dialer.go b/internal/engine/legacy/netx/dialer.go index dffa231..48276de 100644 --- a/internal/engine/legacy/netx/dialer.go +++ b/internal/engine/legacy/netx/dialer.go @@ -14,6 +14,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" + "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -107,7 +108,7 @@ func newTLSDialer(d dialer.Dialer, config *tls.Config) *netxlite.TLSDialer { Config: config, Dialer: d, TLSHandshaker: tlsdialer.EmitterTLSHandshaker{ - TLSHandshaker: tlsdialer.ErrorWrapperTLSHandshaker{ + TLSHandshaker: &errorsx.ErrorWrapperTLSHandshaker{ TLSHandshaker: &netxlite.TLSHandshakerConfigurable{}, }, }, diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 321c531..cc66532 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -38,6 +38,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -184,7 +185,7 @@ func NewTLSDialer(config Config) TLSDialer { config.Dialer = NewDialer(config) } var h tlsHandshaker = &netxlite.TLSHandshakerConfigurable{} - h = tlsdialer.ErrorWrapperTLSHandshaker{TLSHandshaker: h} + h = &errorsx.ErrorWrapperTLSHandshaker{TLSHandshaker: h} if config.Logger != nil { h = &netxlite.TLSHandshakerLogger{Logger: config.Logger, TLSHandshaker: h} } diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index beba3c6..a15ed8e 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -14,6 +14,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" + "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -230,7 +231,7 @@ func TestNewTLSDialerVanilla(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(tlsdialer.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -259,7 +260,7 @@ func TestNewTLSDialerWithConfig(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(tlsdialer.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -298,7 +299,7 @@ func TestNewTLSDialerWithLogging(t *testing.T) { if lth.Logger != log.Log { t.Fatal("not the Logger we expected") } - ewth, ok := lth.TLSHandshaker.(tlsdialer.ErrorWrapperTLSHandshaker) + ewth, ok := lth.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -338,7 +339,7 @@ func TestNewTLSDialerWithSaver(t *testing.T) { if sth.Saver != saver { t.Fatal("not the Logger we expected") } - ewth, ok := sth.TLSHandshaker.(tlsdialer.ErrorWrapperTLSHandshaker) + ewth, ok := sth.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -371,7 +372,7 @@ func TestNewTLSDialerWithNoTLSVerifyAndConfig(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(tlsdialer.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -406,7 +407,7 @@ func TestNewTLSDialerWithNoTLSVerifyAndNoConfig(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(tlsdialer.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } diff --git a/internal/engine/netx/tlsdialer/tls.go b/internal/engine/netx/tlsdialer/tls.go index 8a5384f..a1de437 100644 --- a/internal/engine/netx/tlsdialer/tls.go +++ b/internal/engine/netx/tlsdialer/tls.go @@ -8,7 +8,6 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" - "github.com/ooni/probe-cli/v3/internal/errorsx" ) // UnderlyingDialer is the underlying dialer type. @@ -22,24 +21,6 @@ type TLSHandshaker interface { net.Conn, tls.ConnectionState, error) } -// ErrorWrapperTLSHandshaker wraps the returned error to be an OONI error -type ErrorWrapperTLSHandshaker struct { - TLSHandshaker -} - -// Handshake implements Handshaker.Handshake -func (h ErrorWrapperTLSHandshaker) Handshake( - ctx context.Context, conn net.Conn, config *tls.Config, -) (net.Conn, tls.ConnectionState, error) { - tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) - err = errorsx.SafeErrWrapperBuilder{ - Classifier: errorsx.ClassifyTLSFailure, - Error: err, - Operation: errorsx.TLSHandshakeOperation, - }.MaybeBuild() - return tlsconn, state, err -} - // EmitterTLSHandshaker emits events using the MeasurementRoot type EmitterTLSHandshaker struct { TLSHandshaker diff --git a/internal/engine/netx/tlsdialer/tls_test.go b/internal/engine/netx/tlsdialer/tls_test.go index e2a2011..40257f9 100644 --- a/internal/engine/netx/tlsdialer/tls_test.go +++ b/internal/engine/netx/tlsdialer/tls_test.go @@ -11,7 +11,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/handlers" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" - "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -38,28 +37,6 @@ func (c *SetDeadlineConn) SetDeadline(t time.Time) error { return nil } -func TestErrorWrapperTLSHandshakerFailure(t *testing.T) { - h := tlsdialer.ErrorWrapperTLSHandshaker{TLSHandshaker: tlsdialer.EOFTLSHandshaker{}} - conn, _, err := h.Handshake( - context.Background(), tlsdialer.EOFConn{}, new(tls.Config)) - if !errors.Is(err, io.EOF) { - t.Fatal("not the error that we expected") - } - if conn != nil { - t.Fatal("expected nil con here") - } - var errWrapper *errorsx.ErrWrapper - if !errors.As(err, &errWrapper) { - t.Fatal("cannot cast to ErrWrapper") - } - if errWrapper.Failure != errorsx.FailureEOFError { - t.Fatal("unexpected Failure") - } - if errWrapper.Operation != errorsx.TLSHandshakeOperation { - t.Fatal("unexpected Operation") - } -} - func TestEmitterTLSHandshakerFailure(t *testing.T) { saver := &handlers.SavingHandler{} ctx := modelx.WithMeasurementRoot(context.Background(), &modelx.MeasurementRoot{ diff --git a/internal/errorsx/tls.go b/internal/errorsx/tls.go new file mode 100644 index 0000000..46fbdf5 --- /dev/null +++ b/internal/errorsx/tls.go @@ -0,0 +1,31 @@ +package errorsx + +import ( + "context" + "crypto/tls" + "net" +) + +// TLSHandshaker is the generic TLS handshaker +type TLSHandshaker interface { + Handshake(ctx context.Context, conn net.Conn, config *tls.Config) ( + net.Conn, tls.ConnectionState, error) +} + +// ErrorWrapperTLSHandshaker wraps the returned error to be an OONI error +type ErrorWrapperTLSHandshaker struct { + TLSHandshaker +} + +// Handshake implements TLSHandshaker.Handshake +func (h *ErrorWrapperTLSHandshaker) Handshake( + ctx context.Context, conn net.Conn, config *tls.Config, +) (net.Conn, tls.ConnectionState, error) { + tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) + err = SafeErrWrapperBuilder{ + Classifier: ClassifyTLSFailure, + Error: err, + Operation: TLSHandshakeOperation, + }.MaybeBuild() + return tlsconn, state, err +} diff --git a/internal/errorsx/tls_test.go b/internal/errorsx/tls_test.go new file mode 100644 index 0000000..60bd7b3 --- /dev/null +++ b/internal/errorsx/tls_test.go @@ -0,0 +1,42 @@ +package errorsx + +import ( + "context" + "crypto/tls" + "errors" + "io" + "net" + "testing" + + "github.com/ooni/probe-cli/v3/internal/netxmocks" +) + +func TestErrorWrapperTLSHandshakerFailure(t *testing.T) { + th := ErrorWrapperTLSHandshaker{TLSHandshaker: &netxmocks.TLSHandshaker{ + MockHandshake: func(ctx context.Context, conn net.Conn, config *tls.Config) (net.Conn, tls.ConnectionState, error) { + return nil, tls.ConnectionState{}, io.EOF + }, + }} + conn, _, err := th.Handshake( + context.Background(), &netxmocks.Conn{ + MockRead: func(b []byte) (int, error) { + return 0, io.EOF + }, + }, new(tls.Config)) + if !errors.Is(err, io.EOF) { + t.Fatal("not the error that we expected") + } + if conn != nil { + t.Fatal("expected nil con here") + } + var errWrapper *ErrWrapper + if !errors.As(err, &errWrapper) { + t.Fatal("cannot cast to ErrWrapper") + } + if errWrapper.Failure != FailureEOFError { + t.Fatal("unexpected Failure") + } + if errWrapper.Operation != TLSHandshakeOperation { + t.Fatal("unexpected Operation") + } +}