Swallow SMTP RSET exceptions. These obscure other Send/SendAsync exce…

…ptions. Partial fix for issue #1748
jstedfast · May 2, 2024 · 7aa56a4 · 7aa56a4
1 parent 8adec39
commit 7aa56a4
Show file tree

Hide file tree

Showing 4 changed files with 248 additions and 6 deletions.
diff --git a/MailKit/Net/Smtp/AsyncSmtpClient.cs b/MailKit/Net/Smtp/AsyncSmtpClient.cs
@@ -926,7 +926,14 @@ async Task<string> MessageDataAsync (FormatOptions options, MimeMessage message,
 
 		async Task ResetAsync (CancellationToken cancellationToken)
 		{
-			var response = await SendCommandInternalAsync ("RSET\r\n", cancellationToken).ConfigureAwait (false);
+			SmtpResponse response;
+
+			try {
+				response = await SendCommandInternalAsync ("RSET\r\n", cancellationToken).ConfigureAwait (false);
+			} catch {
+				// Swallow RSET exceptions so that we do not obscure the exception that caused the need for the RSET command in the first place.
+				return;
+			}
 
 			if (response.StatusCode != SmtpStatusCode.Ok)
 				Disconnect (uri.Host, uri.Port, GetSecureSocketOptions (uri), false);
@@ -979,11 +986,9 @@ async Task<string> SendAsync (FormatOptions options, MimeMessage message, Mailbo
 				var dataResponse = await Stream.SendCommandAsync ("DATA\r\n", cancellationToken).ConfigureAwait (false);
 
 				ParseDataResponse (dataResponse);
-				dataResponse = null;
 
 				return await MessageDataAsync (format, message, size, cancellationToken, progress).ConfigureAwait (false);
 			} catch (ServiceNotAuthenticatedException) {
-				// do not disconnect
 				await ResetAsync (cancellationToken).ConfigureAwait (false);
 				throw;
 			} catch (SmtpCommandException) {

diff --git a/MailKit/Net/Smtp/SmtpClient.cs b/MailKit/Net/Smtp/SmtpClient.cs
@@ -2222,7 +2222,14 @@ string MessageData (FormatOptions options, MimeMessage message, long size, Cance
 
 		void Reset (CancellationToken cancellationToken)
 		{
-			var response = SendCommandInternal ("RSET\r\n", cancellationToken);
+			SmtpResponse response;
+
+			try {
+				response = SendCommandInternal ("RSET\r\n", cancellationToken);
+			} catch {
+				// Swallow RSET exceptions so that we do not obscure the exception that caused the need for the RSET command in the first place.
+				return;
+			}
 
 			if (response.StatusCode != SmtpStatusCode.Ok)
 				Disconnect (uri.Host, uri.Port, GetSecureSocketOptions (uri), false);
@@ -2373,11 +2380,9 @@ string Send (FormatOptions options, MimeMessage message, MailboxAddress sender,
 				var dataResponse = Stream.SendCommand ("DATA\r\n", cancellationToken);
 
 				ParseDataResponse (dataResponse);
-				dataResponse = null;
 
 				return MessageData (format, message, size, cancellationToken, progress);
 			} catch (ServiceNotAuthenticatedException) {
-				// do not disconnect
 				Reset (cancellationToken);
 				throw;
 			} catch (SmtpCommandException) {

diff --git a/UnitTests/Net/Smtp/SmtpClientTests.cs b/UnitTests/Net/Smtp/SmtpClientTests.cs
@@ -3865,6 +3865,234 @@ public async Task TestMailFromMailboxUnavailableAsync ()
 			}
 		}
 
+		static List<SmtpReplayCommand> CreateMailFromAuthRequiredRsetDisconnectCommands ()
+		{
+			return new List<SmtpReplayCommand> {
+				new SmtpReplayCommand ("", "comcast-greeting.txt"),
+				new SmtpReplayCommand ($"EHLO {SmtpClient.DefaultLocalDomain}\r\n", "comcast-ehlo.txt"),
+				new SmtpReplayCommand ("AUTH PLAIN AHVzZXJuYW1lAHBhc3N3b3Jk\r\n", "comcast-auth-plain.txt"),
+				new SmtpReplayCommand ("MAIL FROM:<sender@example.com>\r\n", "auth-required.txt", SmtpReplayState.UnexpectedDisconnect),
+				new SmtpReplayCommand ("RSET\r\n", string.Empty)
+			};
+		}
+
+		[Test]
+		public void TestMailFromAuthRequiredRsetDisconnect ()
+		{
+			var commands = CreateMailFromAuthRequiredRsetDisconnectCommands ();
+
+			using (var client = new SmtpClient ()) {
+				try {
+					client.Connect (new SmtpReplayStream (commands, false), "localhost", 25, SecureSocketOptions.None);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Connect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.True, "Client failed to connect.");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Authentication), Is.True, "Failed to detect AUTH extension");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("LOGIN"), "Failed to detect the LOGIN auth mechanism");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("PLAIN"), "Failed to detect the PLAIN auth mechanism");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EightBitMime), Is.True, "Failed to detect 8BITMIME extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EnhancedStatusCodes), Is.True, "Failed to detect ENHANCEDSTATUSCODES extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Size), Is.True, "Failed to detect SIZE extension");
+				Assert.That (client.MaxSize, Is.EqualTo (36700160), "Failed to parse SIZE correctly");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.StartTLS), Is.True, "Failed to detect STARTTLS extension");
+
+				try {
+					client.Authenticate ("username", "password");
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Authenticate: {ex}");
+				}
+
+				try {
+					using (var message = CreateSimpleMessage ())
+						client.Send (message);
+					Assert.Fail ("Expected an ServiceNotAuthenticatedException");
+				} catch (ServiceNotAuthenticatedException) {
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect this exception in Send: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Expected the client to be disconnected");
+
+				try {
+					client.Disconnect (true);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Disconnect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Failed to disconnect");
+			}
+		}
+
+		[Test]
+		public async Task TestMailFromAuthRequiredRsetDisconnectAsync ()
+		{
+			var commands = CreateMailFromAuthRequiredRsetDisconnectCommands ();
+
+			using (var client = new SmtpClient ()) {
+				try {
+					await client.ConnectAsync (new SmtpReplayStream (commands, true), "localhost", 25, SecureSocketOptions.None);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Connect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.True, "Client failed to connect.");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Authentication), Is.True, "Failed to detect AUTH extension");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("LOGIN"), "Failed to detect the LOGIN auth mechanism");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("PLAIN"), "Failed to detect the PLAIN auth mechanism");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EightBitMime), Is.True, "Failed to detect 8BITMIME extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EnhancedStatusCodes), Is.True, "Failed to detect ENHANCEDSTATUSCODES extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Size), Is.True, "Failed to detect SIZE extension");
+				Assert.That (client.MaxSize, Is.EqualTo (36700160), "Failed to parse SIZE correctly");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.StartTLS), Is.True, "Failed to detect STARTTLS extension");
+
+				try {
+					await client.AuthenticateAsync ("username", "password");
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Authenticate: {ex}");
+				}
+
+				try {
+					using (var message = CreateSimpleMessage ())
+						await client.SendAsync (message);
+					Assert.Fail ("Expected an ServiceNotAuthenticatedException");
+				} catch (ServiceNotAuthenticatedException) {
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect this exception in Send: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Expected the client to be disconnected");
+
+				try {
+					await client.DisconnectAsync (true);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Disconnect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Failed to disconnect");
+			}
+		}
+
+		static List<SmtpReplayCommand> CreateMailFromUnavailableRsetDisconnectCommands ()
+		{
+			return new List<SmtpReplayCommand> {
+				new SmtpReplayCommand ("", "comcast-greeting.txt"),
+				new SmtpReplayCommand ($"EHLO {SmtpClient.DefaultLocalDomain}\r\n", "comcast-ehlo.txt"),
+				new SmtpReplayCommand ("AUTH PLAIN AHVzZXJuYW1lAHBhc3N3b3Jk\r\n", "comcast-auth-plain.txt"),
+				new SmtpReplayCommand ("MAIL FROM:<sender@example.com>\r\n", "mailbox-unavailable.txt", SmtpReplayState.UnexpectedDisconnect),
+				new SmtpReplayCommand ("RSET\r\n", string.Empty)
+			};
+		}
+
+		[Test]
+		public void TestMailFromUnavailableRsetDisconnect ()
+		{
+			var commands = CreateMailFromUnavailableRsetDisconnectCommands ();
+
+			using (var client = new SmtpClient ()) {
+				try {
+					client.Connect (new SmtpReplayStream (commands, false), "localhost", 25, SecureSocketOptions.None);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Connect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.True, "Client failed to connect.");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Authentication), Is.True, "Failed to detect AUTH extension");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("LOGIN"), "Failed to detect the LOGIN auth mechanism");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("PLAIN"), "Failed to detect the PLAIN auth mechanism");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EightBitMime), Is.True, "Failed to detect 8BITMIME extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EnhancedStatusCodes), Is.True, "Failed to detect ENHANCEDSTATUSCODES extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Size), Is.True, "Failed to detect SIZE extension");
+				Assert.That (client.MaxSize, Is.EqualTo (36700160), "Failed to parse SIZE correctly");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.StartTLS), Is.True, "Failed to detect STARTTLS extension");
+
+				try {
+					client.Authenticate ("username", "password");
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Authenticate: {ex}");
+				}
+
+				try {
+					using (var message = CreateSimpleMessage ())
+						client.Send (message);
+					Assert.Fail ("Expected an SmtpCommandException");
+				} catch (SmtpCommandException sex) {
+					Assert.That (sex.ErrorCode, Is.EqualTo (SmtpErrorCode.SenderNotAccepted), "Unexpected SmtpErrorCode");
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect this exception in Send: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Expected the client to be disconnected");
+
+				try {
+					client.Disconnect (true);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Disconnect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Failed to disconnect");
+			}
+		}
+
+		[Test]
+		public async Task TestMailFromUnavailableRsetDisconnectAsync ()
+		{
+			var commands = CreateMailFromUnavailableRsetDisconnectCommands ();
+
+			using (var client = new SmtpClient ()) {
+				try {
+					await client.ConnectAsync (new SmtpReplayStream (commands, true), "localhost", 25, SecureSocketOptions.None);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Connect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.True, "Client failed to connect.");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Authentication), Is.True, "Failed to detect AUTH extension");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("LOGIN"), "Failed to detect the LOGIN auth mechanism");
+				Assert.That (client.AuthenticationMechanisms, Does.Contain ("PLAIN"), "Failed to detect the PLAIN auth mechanism");
+
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EightBitMime), Is.True, "Failed to detect 8BITMIME extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.EnhancedStatusCodes), Is.True, "Failed to detect ENHANCEDSTATUSCODES extension");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.Size), Is.True, "Failed to detect SIZE extension");
+				Assert.That (client.MaxSize, Is.EqualTo (36700160), "Failed to parse SIZE correctly");
+				Assert.That (client.Capabilities.HasFlag (SmtpCapabilities.StartTLS), Is.True, "Failed to detect STARTTLS extension");
+
+				try {
+					await client.AuthenticateAsync ("username", "password");
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Authenticate: {ex}");
+				}
+
+				try {
+					using (var message = CreateSimpleMessage ())
+						await client.SendAsync (message);
+					Assert.Fail ("Expected an SmtpCommandException");
+				} catch (SmtpCommandException sex) {
+					Assert.That (sex.ErrorCode, Is.EqualTo (SmtpErrorCode.SenderNotAccepted), "Unexpected SmtpErrorCode");
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect this exception in Send: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Expected the client to be disconnected");
+
+				try {
+					await client.DisconnectAsync (true);
+				} catch (Exception ex) {
+					Assert.Fail ($"Did not expect an exception in Disconnect: {ex}");
+				}
+
+				Assert.That (client.IsConnected, Is.False, "Failed to disconnect");
+			}
+		}
+
 		static List<SmtpReplayCommand> CreateRcptToMailboxUnavailableCommands ()
 		{
 			return new List<SmtpReplayCommand> {

diff --git a/UnitTests/Net/Smtp/SmtpReplayStream.cs b/UnitTests/Net/Smtp/SmtpReplayStream.cs
@@ -66,6 +66,7 @@ enum SmtpReplayState {
 		SendResponse,
 		WaitForCommand,
 		WaitForEndOfData,
+		UnexpectedDisconnect
 	}
 
 	class SmtpReplayStream : Stream
@@ -143,6 +144,9 @@ public override int Read (byte[] buffer, int offset, int count)
 				Assert.That (isAsync, Is.False, "Trying to ReadAsync in a non-async unit test.");
 			}
 
+			if (state == SmtpReplayState.UnexpectedDisconnect)
+				return 0;
+
 			Assert.That (state, Is.EqualTo (SmtpReplayState.SendResponse), "Trying to read when no command given.");
 			Assert.That (stream, Is.Not.Null, "Trying to read when no data available.");