Fixed queueing logic for pipelining SMTP and POP3 commands

Fixes issue #1568
jstedfast · May 14, 2023 · b2ad91f · b2ad91f
1 parent fc86dd0
commit b2ad91f
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 2 deletions.
diff --git a/MailKit/Net/Pop3/Pop3Stream.cs b/MailKit/Net/Pop3/Pop3Stream.cs
@@ -700,8 +700,19 @@ unsafe bool TryQueueCommand (Encoder encoder, string command, ref int index)
 				char* chars = cmd + index;
 
 				var needed = encoder.GetByteCount (chars, charCount, true);
-				if (needed > outputLeft)
+
+				if (needed > output.Length) {
+					// If the command we are trying to queue is larger than the output buffer and we
+					// already have some commands queued in the output buffer, then flush the queue
+					// before queuing this command.
+					if (outputIndex > 0)
+						return false;
+				} else if (needed > outputLeft && index == 0) {
+					// If we are trying to queue a new command (index == 0) and we need more space than
+					// what remains in the output buffer, then flush the output buffer before queueing
+					// the new command. Some servers do not handle receiving partial commands well.
 					return false;
+				}
 
 				fixed (byte* outbuf = output) {
 					byte* outptr = outbuf + outputIndex;

diff --git a/MailKit/Net/Smtp/SmtpStream.cs b/MailKit/Net/Smtp/SmtpStream.cs
@@ -597,8 +597,19 @@ unsafe bool TryQueueCommand (Encoder encoder, string command, ref int index)
 				char* chars = cmd + index;
 
 				var needed = encoder.GetByteCount (chars, charCount, true);
-				if (needed > outputLeft)
+
+				if (needed > output.Length) {
+					// If the command we are trying to queue is larger than the output buffer and we
+					// already have some commands queued in the output buffer, then flush the queue
+					// before queuing this command.
+					if (outputIndex > 0)
+						return false;
+				} else if (needed > outputLeft && index == 0) {
+					// If we are trying to queue a new command (index == 0) and we need more space than
+					// what remains in the output buffer, then flush the output buffer before queueing
+					// the new command. Some servers do not handle receiving partial commands well.
 					return false;
+				}
 
 				fixed (byte* outbuf = output) {
 					byte* outptr = outbuf + outputIndex;

diff --git a/UnitTests/Net/Pop3/Pop3StreamTests.cs b/UnitTests/Net/Pop3/Pop3StreamTests.cs
@@ -245,5 +245,38 @@ public async Task TestWriteAsync ()
 				memory.SetLength (0);
 			}
 		}
+
+		[Test]
+		public void TestQueueReallyLongCommand ()
+		{
+			using var stream = new Pop3Stream (new DummyNetworkStream (), new NullProtocolLogger ());
+			var memory = (MemoryStream) stream.Stream;
+			var command = "AUTH GSSAPI YIIkMgYGK" + new string ('X', 4096) + "\r\n";
+
+			stream.QueueCommand (Encoding.UTF8, command, default);
+			stream.Flush ();
+
+			var actual = Encoding.ASCII.GetString (memory.GetBuffer (), 0, (int) memory.Length);
+
+			Assert.AreEqual (command, actual);
+		}
+
+		[Test]
+		public void TestQueueReallyLongCommandAfterShortCommand ()
+		{
+			using var stream = new Pop3Stream (new DummyNetworkStream (), new NullProtocolLogger ());
+			var memory = (MemoryStream) stream.Stream;
+
+			var shortCommand = "CAPA\r\n";
+			var longCommand = "AUTH GSSAPI YIIkMgYGK" + new string ('X', 4096) + "\r\n";
+
+			stream.QueueCommand (Encoding.UTF8, shortCommand, default);
+			stream.QueueCommand (Encoding.UTF8, longCommand, default);
+			stream.Flush ();
+
+			var actual = Encoding.ASCII.GetString (memory.GetBuffer (), 0, (int) memory.Length);
+
+			Assert.AreEqual (shortCommand + longCommand, actual);
+		}
 	}
 }
diff --git a/UnitTests/Net/Smtp/SmtpStreamTests.cs b/UnitTests/Net/Smtp/SmtpStreamTests.cs
@@ -289,5 +289,38 @@ public async Task TestWriteAsync ()
 				memory.SetLength (0);
 			}
 		}
+
+		[Test]
+		public void TestQueueReallyLongCommand ()
+		{
+			using var stream = new SmtpStream (new DummyNetworkStream (), new NullProtocolLogger ());
+			var memory = (MemoryStream) stream.Stream;
+			var command = "AUTH GSSAPI YIIkMgYGK" + new string ('X', 4096) + "\r\n";
+
+			stream.QueueCommand (command, default);
+			stream.Flush ();
+
+			var actual = Encoding.ASCII.GetString (memory.GetBuffer (), 0, (int) memory.Length);
+
+			Assert.AreEqual (command, actual);
+		}
+
+		[Test]
+		public void TestQueueReallyLongCommandAfterShortCommand ()
+		{
+			using var stream = new SmtpStream (new DummyNetworkStream (), new NullProtocolLogger ());
+			var memory = (MemoryStream) stream.Stream;
+
+			var shortCommand = "EHLO [192.168.1.1]\r\n";
+			var longCommand = "AUTH GSSAPI YIIkMgYGK" + new string ('X', 4096) + "\r\n";
+
+			stream.QueueCommand (shortCommand, default);
+			stream.QueueCommand (longCommand, default);
+			stream.Flush ();
+
+			var actual = Encoding.ASCII.GetString (memory.GetBuffer (), 0, (int) memory.Length);
+
+			Assert.AreEqual (shortCommand + longCommand, actual);
+		}
 	}
 }