Merge branch 'feature/boundary_ends_with_double_dash' into develop

Resolves #123

Author: Jericho

Source/HttpMultipartParser.UnitTests/ParserScenarios/BoundaryEndsWithDoubleDash.cs

@@ -0,0 +1,71 @@
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace HttpMultipartParser.UnitTests.ParserScenarios
+{
+	public class BoundaryEndsWithDoubleDash
+	{
+		// The boundary in this scenario ends with '--'. This is an unusual scenario but perfectly legitimate.
+		// For details, see: https://github.com/Http-Multipart-Data-Parser/Http-Multipart-Data-Parser/issues/123
+		private static readonly string _testData = TestUtil.TrimAllLines(
+			@"--boundary_text--
+			Content-type: text/plain; charset=UTF-8
+			Content-Disposition: form-data; name=""file1""; filename=""file1.txt""
+
+			file content here
+			--boundary_text--
+			Content-type: text/plain; charset=UTF-8
+			Content-Disposition: form-data; name=""file2""; filename=""file2.txt""
+
+			file content here 2
+			--boundary_text----"
+		);
+
+		private static readonly TestData _testCase = new TestData(
+			_testData,
+			Enumerable.Empty<ParameterPart>().ToList(),
+			new List<FilePart>() {
+				new FilePart("file1", "file1.txt", TestUtil.StringToStreamNoBom("file content here"), (new[] { new KeyValuePair<string, string>("charset", "UTF-8") }).ToDictionary(kvp => kvp.Key, kvp => kvp.Value)),
+				new FilePart("file2", "file2.txt", TestUtil.StringToStreamNoBom("file content here 2"), (new[] { new KeyValuePair<string, string>("charset", "UTF-8") }).ToDictionary(kvp => kvp.Key, kvp => kvp.Value)),
+			}
+		);
+
+		public BoundaryEndsWithDoubleDash()
+		{
+			foreach (var filePart in _testCase.ExpectedFileData)
+			{
+				filePart.Data.Position = 0;
+			}
+		}
+
+		/// <summary>
+		///     Tests for correct detection of the boundary in the input stream.
+		/// </summary>
+		[Fact]
+		public void CanAutoDetectBoundary()
+		{
+			using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
+			{
+				var parser = MultipartFormDataParser.Parse(stream);
+				Assert.True(_testCase.Validate(parser));
+			}
+		}
+
+		/// <summary>
+		///     Tests for correct detection of the boundary in the input stream.
+		/// </summary>
+		[Fact]
+		public async Task CanAutoDetectBoundaryAsync()
+		{
+			using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
+			{
+				var parser = await MultipartFormDataParser.ParseAsync(stream, Encoding.UTF8);
+				Assert.True(_testCase.Validate(parser));
+			}
+		}
+	}
+}

Source/HttpMultipartParser/StreamingBinaryMultipartFormDataParser.cs

@@ -277,9 +277,12 @@ private static string DetectBoundary(RebufferableBinaryReader reader)
 			// Remove the two dashes
 			string boundary = line.Substring(2);
 
-			// If the string ends with '--' it means that we found the "end" boundary and we
-			// need to trim the two dashes to get the actual boundary
-			if (boundary.EndsWith("--"))
+			// If the string ends with '--' and it's not followed by content, we can safely assume that we
+			// found the "end" boundary. In this scenario, we must trim the two dashes to get the actual boundary.
+			// Otherwise, the boundary must be accepted as-is. The reason we check for "additional content" is to
+			// resolve the problem explained in GH-123.
+			var moreContentAvailable = MoreContentAvailable(reader);
+			if (boundary.EndsWith("--") && !moreContentAvailable)
 			{
 				boundary = boundary.Substring(0, boundary.Length - 2);
 				reader.Buffer($"--{boundary}--\n");
@@ -333,9 +336,12 @@ private static async Task<string> DetectBoundaryAsync(RebufferableBinaryReader r
 			// Remove the two dashes
 			string boundary = line.Substring(2);
 
-			// If the string ends with '--' it means that we found the "end" boundary and we
-			// need to trim the two dashes to get the actual boundary.
-			if (boundary.EndsWith("--"))
+			// If the string ends with '--' and it's not followed by content, we can safely assume that we
+			// found the "end" boundary. In this scenario, we must trim the two dashes to get the actual boundary.
+			// Otherwise, the boundary must be accepted as-is. The reason we check for "additional content" is to
+			// resolve the problem explained in GH-123.
+			var moreContentAvailable = await MoreContentAvailableAsync(reader).ConfigureAwait(false);
+			if (boundary.EndsWith("--") && !moreContentAvailable)
 			{
 				boundary = boundary.Substring(0, boundary.Length - 2);
 				reader.Buffer($"--{boundary}--\n");
@@ -348,6 +354,47 @@ private static async Task<string> DetectBoundaryAsync(RebufferableBinaryReader r
 			return boundary;
 		}
 
+		/// <summary>
+		///     Determine if there is more content.
+		/// </summary>
+		/// <param name="reader">
+		///     The binary reader to parse.
+		/// </param>
+		/// <returns>
+		///     A boolean indicating whether more content is available in the binary reader.
+		/// </returns>
+		private static bool MoreContentAvailable(RebufferableBinaryReader reader)
+		{
+			var line = reader.ReadLine();
+
+			if (line == null) return false;
+			else reader.Buffer($"{line}\n");
+
+			return true;
+		}
+
+		/// <summary>
+		///     Determine if there is more content.
+		/// </summary>
+		/// <param name="reader">
+		///     The binary reader to parse.
+		/// </param>
+		/// <param name="cancellationToken">
+		///     The cancellation token.
+		/// </param>
+		/// <returns>
+		///     A boolean indicating whether more content is available in the binary reader.
+		/// </returns>
+		private static async Task<bool> MoreContentAvailableAsync(RebufferableBinaryReader reader, CancellationToken cancellationToken = default)
+		{
+			var line = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
+
+			if (line == null) return false;
+			else reader.Buffer($"{line}\n");
+
+			return true;
+		}
+
 		/// <summary>
 		/// Use a few assumptions to determine if a section contains a file.
 		/// </summary>