Merge pull request #502 from wneessen/bugfix/fix-potential-nil-pointer

Refactored `SendWithSMTPClient` to improve error handling and added test cases

Author: wneessen

client.go

@@ -259,6 +259,9 @@ var (
 
 	// ErrDialContextFuncIsNil indicates that a required dial context function is not provided.
 	ErrDialContextFuncIsNil = errors.New("dial context function is nil")
+
+	// ErrClientIsNil indicates that a required smtp client is not provided.
+	ErrClientIsNil = errors.New("client is nil")
 )
 
 // NewClient creates a new Client instance with the provided host and optional configuration Option functions.
@@ -1252,16 +1255,19 @@ func (c *Client) Send(messages ...*Msg) (returnErr error) {
 //   - An error that represents the sending result, which may include multiple SendErrors if
 //     any occurred; otherwise, returns nil.
 func (c *Client) SendWithSMTPClient(client *smtp.Client, messages ...*Msg) (returnErr error) {
-	escSupport := false
-	if client != nil {
-		escSupport, _ = client.Extension("ENHANCEDSTATUSCODES")
+	if client == nil {
+		return &SendError{
+			Reason: ErrConnCheck, errlist: []error{ErrClientIsNil}, isTemp: isTempError(ErrClientIsNil),
+			errcode: errorCode(ErrClientIsNil), enhancedStatusCode: enhancedStatusCode(ErrClientIsNil, false),
+		}
 	}
+
+	escSupport, _ := client.Extension("ENHANCEDSTATUSCODES")
 	if err := c.checkConn(client); err != nil {
-		returnErr = &SendError{
+		return &SendError{
 			Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err),
 			errcode: errorCode(err), enhancedStatusCode: enhancedStatusCode(err, escSupport),
 		}
-		return returnErr
 	}
 
 	var errs []error

client_test.go

@@ -3755,6 +3755,98 @@ func TestClient_DialWithContextNewVersionsOnly(t *testing.T) {
 	})
 }
 
+func TestClient_SendWithSMTPClient(t *testing.T) {
+	message := testMessage(t)
+
+	t.Run("sending with normal client should succeed", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		PortAdder.Add(1)
+		serverPort := int(TestServerPortBase + PortAdder.Load())
+		featureSet := "250-AUTH PLAIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"
+		go func() {
+			if err := simpleSMTPServer(ctx, t, &serverProps{FeatureSet: featureSet, ListenPort: serverPort}); err != nil {
+				t.Errorf("failed to start test server: %s", err)
+				return
+			}
+		}()
+		time.Sleep(time.Millisecond * 30)
+
+		ctxDial, cancelDial := context.WithTimeout(ctx, time.Millisecond*500)
+		t.Cleanup(cancelDial)
+
+		client, err := NewClient(DefaultHost, WithPort(serverPort), WithTLSPolicy(NoTLS))
+		if err != nil {
+			t.Fatalf("failed to create new client: %s", err)
+		}
+		if err = client.DialWithContext(ctxDial); err != nil {
+			var netErr net.Error
+			if errors.As(err, &netErr) && netErr.Timeout() {
+				t.Skip("failed to connect to the test server due to timeout")
+			}
+			t.Fatalf("failed to connect to test server: %s", err)
+		}
+		t.Cleanup(func() {
+			if err := client.Close(); err != nil {
+				t.Errorf("failed to close client: %s", err)
+			}
+		})
+		if err = client.SendWithSMTPClient(client.smtpClient, message); err != nil {
+			t.Errorf("failed to deliver message via smtp client: %s", err)
+		}
+	})
+	t.Run("sending should return a send error on connection test", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		PortAdder.Add(1)
+		serverPort := int(TestServerPortBase + PortAdder.Load())
+		featureSet := "250-AUTH PLAIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"
+		go func() {
+			if err := simpleSMTPServer(ctx, t, &serverProps{
+				FeatureSet: featureSet,
+				ListenPort: serverPort,
+				FailOnNoop: true,
+			}); err != nil {
+				t.Errorf("failed to start test server: %s", err)
+				return
+			}
+		}()
+		time.Sleep(time.Millisecond * 30)
+
+		ctxDial, cancelDial := context.WithTimeout(ctx, time.Millisecond*500)
+		t.Cleanup(cancelDial)
+
+		client, err := NewClient(DefaultHost, WithPort(serverPort), WithTLSPolicy(NoTLS))
+		if err != nil {
+			t.Fatalf("failed to create new client: %s", err)
+		}
+		if err = client.DialWithContext(ctxDial); err != nil {
+			var netErr net.Error
+			if errors.As(err, &netErr) && netErr.Timeout() {
+				t.Skip("failed to connect to the test server due to timeout")
+			}
+			t.Fatalf("failed to connect to test server: %s", err)
+		}
+		t.Cleanup(func() {
+			if err := client.Close(); err != nil {
+				t.Errorf("failed to close client: %s", err)
+			}
+		})
+		if err = client.SendWithSMTPClient(client.smtpClient, message); err == nil {
+			t.Errorf("expected delivery to fail on connection check, but it didn't")
+		}
+	})
+	t.Run("sending with nil client should fail", func(t *testing.T) {
+		client, err := NewClient(DefaultHost)
+		if err != nil {
+			t.Fatalf("failed to create new client: %s", err)
+		}
+		if err = client.SendWithSMTPClient(nil, message); err == nil {
+			t.Errorf("expected delivery to fail with nil client, but it didn't")
+		}
+	})
+}
+
 func TestClient_checkConn(t *testing.T) {
 	t.Run("connection is alive", func(t *testing.T) {
 		ctx, cancel := context.WithCancel(context.Background())