refactor(netxlite/errorsx): change all tests to be unit tests (#483)

Later we will try to write comprehensive integration tests for
the whole netxlite package. We want just unit tests here.

internal/netxlite/errorsx/classify.go

@@ -124,19 +124,6 @@ const (
 	quicTLSUnrecognizedName = 112
 )
 
-// quicIsCertificateError tells us whether a specific TLS alert error
-// we received is actually an error depending on the certificate.
-//
-// The set of checks we implement here is a set of heuristics based
-// on our understanding of the TLS spec and may need tweaks.
-func quicIsCertificateError(alert uint8) bool {
-	return (alert == quicTLSAlertBadCertificate ||
-		alert == quicTLSAlertUnsupportedCertificate ||
-		alert == quicTLSAlertCertificateExpired ||
-		alert == quicTLSAlertCertificateRevoked ||
-		alert == quicTLSAlertCertificateUnknown)
-}
-
 // ClassifyQUICHandshakeError maps an error occurred during the QUIC
 // handshake to an OONI failure string.
 //
@@ -196,6 +183,29 @@ func ClassifyQUICHandshakeError(err error) string {
 	return ClassifyGenericError(err)
 }
 
+// quicIsCertificateError tells us whether a specific TLS alert error
+// we received is actually an error depending on the certificate.
+//
+// The set of checks we implement here is a set of heuristics based
+// on our understanding of the TLS spec and may need tweaks.
+func quicIsCertificateError(alert uint8) bool {
+	// List out each case separately so we know we test them
+	switch alert {
+	case quicTLSAlertBadCertificate:
+		return true
+	case quicTLSAlertUnsupportedCertificate:
+		return true
+	case quicTLSAlertCertificateExpired:
+		return true
+	case quicTLSAlertCertificateRevoked:
+		return true
+	case quicTLSAlertCertificateUnknown:
+		return true
+	default:
+		return false
+	}
+}
+
 // ErrDNSBogon indicates that we found a bogon address. Code that
 // filters for DNS bogons MUST use this error.
 var ErrDNSBogon = errors.New("dns: detected bogon address")

internal/netxlite/errorsx/classify_test.go

@@ -2,12 +2,9 @@ package errorsx
 
 import (
 	"context"
-	"crypto/tls"
 	"crypto/x509"
 	"errors"
 	"io"
-	"net"
-	"syscall"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -16,6 +13,9 @@ import (
 )
 
 func TestClassifyGenericError(t *testing.T) {
+	// Please, keep this list sorted in the same order
+	// in which checks appear on the code
+
 	t.Run("for input being already an ErrWrapper", func(t *testing.T) {
 		err := &ErrWrapper{Failure: FailureEOFError}
 		if ClassifyGenericError(err) != FailureEOFError {
@@ -23,20 +23,18 @@ func TestClassifyGenericError(t *testing.T) {
 		}
 	})
 
-	t.Run("for already wrapped error", func(t *testing.T) {
+	t.Run("for a system call error", func(t *testing.T) {
-		err := io.EOF
+		if ClassifyGenericError(EWOULDBLOCK) != FailureOperationWouldBlock {
-		if ClassifyGenericError(err) != FailureEOFError {
+			t.Fatal("unexpected results")
-			t.Fatal("unexpected result")
 		}
 	})
 
-	t.Run("for context.Canceled", func(t *testing.T) {
+	// Now we enter into classifyWithStringSuffix. We test it here
-		if ClassifyGenericError(context.Canceled) != FailureInterrupted {
+	// since we want to test the ClassifyGenericError in is
-			t.Fatal("unexpected result")
+	// entirety here and the classifyWithStringSuffix function
-		}
+	// is just an implementation detail.
-	})
 
-	t.Run("for operation was canceled error", func(t *testing.T) {
+	t.Run("for operation was canceled", func(t *testing.T) {
 		if ClassifyGenericError(errors.New("operation was canceled")) != FailureInterrupted {
 			t.Fatal("unexpected result")
 		}
@@ -44,45 +42,12 @@ func TestClassifyGenericError(t *testing.T) {
 
 	t.Run("for EOF", func(t *testing.T) {
 		if ClassifyGenericError(io.EOF) != FailureEOFError {
-			t.Fatal("unexpected results")
+			t.Fatal("unexpected result")
-		}
-	})
-
-	t.Run("for canceled", func(t *testing.T) {
-		if ClassifyGenericError(syscall.ECANCELED) != FailureOperationCanceled {
-			t.Fatal("unexpected results")
-		}
-	})
-
-	t.Run("for connection_refused", func(t *testing.T) {
-		if ClassifyGenericError(syscall.ECONNREFUSED) != FailureConnectionRefused {
-			t.Fatal("unexpected results")
-		}
-	})
-
-	t.Run("for connection_reset", func(t *testing.T) {
-		if ClassifyGenericError(syscall.ECONNRESET) != FailureConnectionReset {
-			t.Fatal("unexpected results")
-		}
-	})
-
-	t.Run("for host_unreachable", func(t *testing.T) {
-		if ClassifyGenericError(syscall.EHOSTUNREACH) != FailureHostUnreachable {
-			t.Fatal("unexpected results")
-		}
-	})
-
-	t.Run("for system timeout", func(t *testing.T) {
-		if ClassifyGenericError(syscall.ETIMEDOUT) != FailureTimedOut {
-			t.Fatal("unexpected results")
 		}
 	})
 
 	t.Run("for context deadline exceeded", func(t *testing.T) {
-		ctx, cancel := context.WithTimeout(context.Background(), 1)
+		if ClassifyGenericError(context.DeadlineExceeded) != FailureGenericTimeoutError {
-		defer cancel()
-		<-ctx.Done()
-		if ClassifyGenericError(ctx.Err()) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
@@ -93,22 +58,13 @@ func TestClassifyGenericError(t *testing.T) {
 		}
 	})
 
-	t.Run("for i/o error", func(t *testing.T) {
+	t.Run("for i/o timeout", func(t *testing.T) {
-		ctx, cancel := context.WithTimeout(context.Background(), 1)
+		if ClassifyGenericError(errors.New("i/o timeout")) != FailureGenericTimeoutError {
-		defer cancel() // fail immediately
-		conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", "www.google.com:80")
-		if err == nil {
-			t.Fatal("expected an error here")
-		}
-		if conn != nil {
-			t.Fatal("expected nil connection here")
-		}
-		if ClassifyGenericError(err) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for TLS handshake timeout error", func(t *testing.T) {
+	t.Run("for TLS handshake timeout", func(t *testing.T) {
 		err := errors.New("net/http: TLS handshake timeout")
 		if ClassifyGenericError(err) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
@@ -116,14 +72,21 @@ func TestClassifyGenericError(t *testing.T) {
 	})
 
 	t.Run("for no such host", func(t *testing.T) {
-		if ClassifyGenericError(&net.DNSError{
+		if ClassifyGenericError(errors.New("no such host")) != FailureDNSNXDOMAINError {
-			Err: "no such host",
-		}) != FailureDNSNXDOMAINError {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for errors including IPv4 address", func(t *testing.T) {
+	// Now we're back in ClassifyGenericError
+
+	t.Run("for context.Canceled", func(t *testing.T) {
+		if ClassifyGenericError(context.Canceled) != FailureInterrupted {
+			t.Fatal("unexpected result")
+		}
+	})
+
+	t.Run("for unknown errors", func(t *testing.T) {
+		t.Run("with an IPv4 address", func(t *testing.T) {
 			input := errors.New("read tcp 10.0.2.15:56948->93.184.216.34:443: use of closed network connection")
 			expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection"
 			out := ClassifyGenericError(input)
@@ -132,7 +95,7 @@ func TestClassifyGenericError(t *testing.T) {
 			}
 		})
 
-	t.Run("for errors including IPv6 address", func(t *testing.T) {
+		t.Run("with an IPv6 address", func(t *testing.T) {
 			input := errors.New("read tcp [::1]:56948->[::1]:443: use of closed network connection")
 			expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection"
 			out := ClassifyGenericError(input)
@@ -140,29 +103,13 @@ func TestClassifyGenericError(t *testing.T) {
 				t.Fatal(cmp.Diff(expected, out))
 			}
 		})
-
-	t.Run("for i/o error", func(t *testing.T) {
-		ctx, cancel := context.WithTimeout(context.Background(), 1)
-		defer cancel() // fail immediately
-		udpAddr := &net.UDPAddr{IP: net.ParseIP("216.58.212.164"), Port: 80, Zone: ""}
-		udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4zero, Port: 0})
-		if err != nil {
-			t.Fatal(err)
-		}
-		sess, err := quic.DialEarlyContext(ctx, udpConn, udpAddr, "google.com:80", &tls.Config{}, &quic.Config{})
-		if err == nil {
-			t.Fatal("expected an error here")
-		}
-		if sess != nil {
-			t.Fatal("expected nil session here")
-		}
-		if ClassifyGenericError(err) != FailureGenericTimeoutError {
-			t.Fatal("unexpected results")
-		}
 	})
 }
 
 func TestClassifyQUICHandshakeError(t *testing.T) {
+	// Please, keep this list sorted in the same order
+	// in which checks appear on the code
+
 	t.Run("for input being already an ErrWrapper", func(t *testing.T) {
 		err := &ErrWrapper{Failure: FailureEOFError}
 		if ClassifyQUICHandshakeError(err) != FailureEOFError {
@@ -170,58 +117,93 @@ func TestClassifyQUICHandshakeError(t *testing.T) {
 		}
 	})
 
-	t.Run("for connection_reset", func(t *testing.T) {
-		if ClassifyQUICHandshakeError(&quic.StatelessResetError{}) != FailureConnectionReset {
-			t.Fatal("unexpected results")
-		}
-	})
-
 	t.Run("for incompatible quic version", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.VersionNegotiationError{}) != FailureQUICIncompatibleVersion {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for quic connection refused", func(t *testing.T) {
+	t.Run("for stateless reset", func(t *testing.T) {
-		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: quic.ConnectionRefused}) != FailureConnectionRefused {
+		if ClassifyQUICHandshakeError(&quic.StatelessResetError{}) != FailureConnectionReset {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for quic handshake timeout", func(t *testing.T) {
+	t.Run("for handshake timeout", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.HandshakeTimeoutError{}) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for QUIC idle connection timeout", func(t *testing.T) {
+	t.Run("for idle timeout", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.IdleTimeoutError{}) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for QUIC CRYPTO Handshake", func(t *testing.T) {
+	t.Run("for connection refused", func(t *testing.T) {
-		var err quic.TransportErrorCode = quicTLSAlertHandshakeFailure
+		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: quic.ConnectionRefused}) != FailureConnectionRefused {
-		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for QUIC CRYPTO Invalid Certificate", func(t *testing.T) {
+	t.Run("for bad certificate", func(t *testing.T) {
 		var err quic.TransportErrorCode = quicTLSAlertBadCertificate
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for QUIC CRYPTO Unknown CA", func(t *testing.T) {
+	t.Run("for unsupported certificate", func(t *testing.T) {
+		var err quic.TransportErrorCode = quicTLSAlertUnsupportedCertificate
+		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate {
+			t.Fatal("unexpected results")
+		}
+	})
+
+	t.Run("for certificate expired", func(t *testing.T) {
+		var err quic.TransportErrorCode = quicTLSAlertCertificateExpired
+		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate {
+			t.Fatal("unexpected results")
+		}
+	})
+
+	t.Run("for certificate revoked", func(t *testing.T) {
+		var err quic.TransportErrorCode = quicTLSAlertCertificateRevoked
+		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate {
+			t.Fatal("unexpected results")
+		}
+	})
+
+	t.Run("for certificate unknown", func(t *testing.T) {
+		var err quic.TransportErrorCode = quicTLSAlertCertificateUnknown
+		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate {
+			t.Fatal("unexpected results")
+		}
+	})
+
+	t.Run("for decrypt error", func(t *testing.T) {
+		var err quic.TransportErrorCode = quicTLSAlertDecryptError
+		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake {
+			t.Fatal("unexpected results")
+		}
+	})
+
+	t.Run("for handshake failure", func(t *testing.T) {
+		var err quic.TransportErrorCode = quicTLSAlertHandshakeFailure
+		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake {
+			t.Fatal("unexpected results")
+		}
+	})
+
+	t.Run("for unknown CA", func(t *testing.T) {
 		var err quic.TransportErrorCode = quicTLSAlertUnknownCA
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLUnknownAuthority {
 			t.Fatal("unexpected results")
 		}
 	})
 
-	t.Run("for QUIC CRYPTO Bad Hostname", func(t *testing.T) {
+	t.Run("for unrecognized hostname", func(t *testing.T) {
 		var err quic.TransportErrorCode = quicTLSUnrecognizedName
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidHostname {
 			t.Fatal("unexpected results")
@@ -236,6 +218,9 @@ func TestClassifyQUICHandshakeError(t *testing.T) {
 }
 
 func TestClassifyResolverError(t *testing.T) {
+	// Please, keep this list sorted in the same order
+	// in which checks appear on the code
+
 	t.Run("for input being already an ErrWrapper", func(t *testing.T) {
 		err := &ErrWrapper{Failure: FailureEOFError}
 		if ClassifyResolverError(err) != FailureEOFError {
@@ -257,6 +242,9 @@ func TestClassifyResolverError(t *testing.T) {
 }
 
 func TestClassifyTLSHandshakeError(t *testing.T) {
+	// Please, keep this list sorted in the same order
+	// in which checks appear on the code
+
 	t.Run("for input being already an ErrWrapper", func(t *testing.T) {
 		err := &ErrWrapper{Failure: FailureEOFError}
 		if ClassifyTLSHandshakeError(err) != FailureEOFError {