Merge branch 'feature/file_chunk_starts_with_BOM' into develop

Resolves #64

Author: Jericho

Source/HttpMultipartParser.UnitTests/ParserScenarios/FileChunkStartsWithBOM.cs

@@ -0,0 +1,89 @@
+using System.Collections.Generic;
+using System.IO;
+using System.Text;
+using Xunit;
+
+namespace HttpMultipartParser.UnitTests.ParserScenarios
+{
+	// This test attempts to reproduce the bug discussed here:
+	// https://github.com/Http-Multipart-Data-Parser/Http-Multipart-Data-Parser/issues/64
+	public class FileChunkStartsWithBOM
+	{
+		private static readonly int _binaryBufferSize = 100;
+		private static readonly byte[] _utf8BOMBinary = new byte[] { 0xef, 0xbb, 0xbf };
+		private static readonly string _utf8BOMString = Encoding.UTF8.GetString(_utf8BOMBinary);
+
+		private static readonly string _prefix =
+	@"--boundary
+Content-Type: application/octet-stream
+
+";
+		// This padding ensures that the BOM is positioned at the begining of the second chunk
+		private static readonly string _padding = new string('+', _binaryBufferSize - _prefix.Length);
+		private static readonly string _fileContent = $"{_padding}{_utf8BOMString}Hello world";
+
+		private static readonly string _testDataFormat =
+			@"{0}{1}
+--boundary--";
+		private static readonly string _testData = TestUtil.TrimAllLines(string.Format(_testDataFormat, _prefix, _fileContent));
+
+		/// <summary>
+		///     Test case for files with additional parameter.
+		/// </summary>
+		private static readonly TestData _testCase = new TestData(
+			_testData,
+			new List<ParameterPart> { },
+			new List<FilePart> {
+				new FilePart(null, null, TestUtil.StringToStreamNoBom(_fileContent), null, "application/octet-stream", "form-data")
+			}
+		);
+
+		/// <summary>
+		///     Initializes the test data before each run, this primarily
+		///     consists of resetting data stream positions.
+		/// </summary>
+		public FileChunkStartsWithBOM()
+		{
+			foreach (var filePart in _testCase.ExpectedFileData)
+			{
+				filePart.Data.Position = 0;
+			}
+		}
+
+		[Fact]
+		public void CanHandleBOM()
+		{
+			var encoding = Encoding.UTF8;
+
+			using (Stream stream = TestUtil.StringToStream(_testCase.Request, encoding))
+			{
+				var parser = MultipartFormDataParser.Parse(stream, encoding, _binaryBufferSize);
+
+				using (var file = parser.Files[0].Data)
+				{
+					file.Position = 0;
+
+					// Read the padding and assert we get the expected value
+					var paddingBuffer = new byte[_padding.Length];
+					var readCount = file.Read(paddingBuffer, 0, paddingBuffer.Length);
+
+					Assert.Equal(_padding, encoding.GetString(paddingBuffer));
+
+					// Read the BOM and assert we get the expected value
+					var bomBuffer = new byte[_utf8BOMBinary.Length];
+					readCount = file.Read(bomBuffer, 0, bomBuffer.Length);
+
+					// If this assertion fails, it means that we have reproduced the problem described in GH-64
+					// If it succeeds, it means that the bug has been fixed.
+					Assert.Equal(_utf8BOMString, encoding.GetString(bomBuffer));
+
+					// Read the rest of the content and assert we get the expected value
+					var restOfContentBuffer = new byte[_fileContent.Length - _padding.Length - _utf8BOMString.Length];
+					readCount = file.Read(restOfContentBuffer, 0, restOfContentBuffer.Length);
+
+					Assert.Equal("Hello world", encoding.GetString(restOfContentBuffer));
+				}
+			}
+		}
+	}
+}

Source/HttpMultipartParser.UnitTests/RebufferableBinaryReaderUnitTests.cs

@@ -67,6 +67,48 @@ public void CanReadMixedAsciiAndUTFCharactersOverBuffers()
 			Assert.Equal('g', reader.Read());
 		}
 
+		[Theory]
+		[InlineData(new byte[] { 0xef, 0xbb, 0xbf }, "utf-8")] // UTF-8
+		[InlineData(new byte[] { 0xff, 0xfe }, "utf-16")] // UTF-16
+		[InlineData(new byte[] { 0xfe, 0xff }, "utf-16BE")] // UTF-16 Big Endian
+		[InlineData(new byte[] { 0xfe, 0xff }, "utf-32")] // UTF-32
+		[InlineData(new byte[] { 0x00, 0x00, 0xfe, 0xff }, "utf-32BE")] // UTF-32 Big Endian
+		public void CanReadBOM(byte[] bom, string encodingName)
+		{
+			var encoding = Encoding.GetEncoding(encodingName);
+
+			var prefixString = "Foo Bar";
+			var prefixBinary = encoding.GetBytes(prefixString);
+
+			var sufixString = "Hello world";
+			var sufixBinary = encoding.GetBytes(sufixString);
+
+			var stream = new MemoryStream();
+			var binaryWriter = new BinaryWriter(stream);
+			binaryWriter.Write(prefixBinary);
+			binaryWriter.Write(bom);
+			binaryWriter.Write(sufixBinary);
+			binaryWriter.Flush();
+			stream.Position = 0;
+
+			var reader = new RebufferableBinaryReader(stream, encoding);
+
+			var buffer = new byte[prefixBinary.Length];
+			reader.Read(buffer, 0, buffer.Length);
+			Assert.Equal(prefixString, encoding.GetString(buffer));
+
+			buffer = new byte[bom.Length];
+			reader.Read(buffer, 0, buffer.Length);
+			for (int i = 0; i < buffer.Length; i++)
+			{
+				Assert.Equal(bom[i], buffer[i]);
+			}
+
+			buffer = new byte[sufixBinary.Length];
+			reader.Read(buffer, 0, buffer.Length);
+			Assert.Equal(sufixString, encoding.GetString(buffer));
+		}
+
 		#endregion
 
 		#region ReadAsync() Tests

Source/HttpMultipartParser/RebufferableBinaryReader.cs

@@ -61,6 +61,11 @@ public class RebufferableBinaryReader
 		/// </summary>
 		private readonly BinaryStreamStack streamStack;
 
+		/// <summary>
+		/// Counts the number of chunks read from the underlying stream.
+		/// </summary>
+		private int processedChunkCounter;
+
 		#endregion
 
 		#region Constructors and Destructors
@@ -73,7 +78,7 @@ public class RebufferableBinaryReader
 		///     The input stream to read from.
 		/// </param>
 		public RebufferableBinaryReader(Stream input)
-			: this(input, new UTF8Encoding(false))
+			: this(input, Encoding.UTF8)
 		{
 		}
 
@@ -95,6 +100,7 @@ public RebufferableBinaryReader(Stream input, Encoding encoding, int bufferSize
 			streamStack = new BinaryStreamStack(encoding);
 			this.encoding = encoding;
 			this.bufferSize = bufferSize;
+			processedChunkCounter = 0;
 		}
 
 		#endregion
@@ -529,15 +535,30 @@ private async Task<int> StreamDataAsync(CancellationToken cancellationToken = de
 		/// </param>
 		private void PushToStack(byte[] buffer, int amountRead)
 		{
-			// We need to check if our stream is using our encodings
-			// BOM, if it is we need to jump it.
-			int bomOffset = GetBomOffset(buffer);
+			/*
+				The logic in this method until August 2025 would eliminate the BOM (also called the encoding preamble)
+				if it was present at the begining of each and every buffer read from the stream.
 
-			// Sometimes we'll get a buffer that's smaller then we expect, chop it down
-			// for the reader:
-			if (amountRead - bomOffset > 0)
+				However, we only need to remove the BOM if present at the very begining of the stream.
+				In other words: only remove the BOM from the first chunk.
+			 */
+
+			if (amountRead > 0)
 			{
-				streamStack.Push(buffer, bomOffset, amountRead - bomOffset);
+				if (processedChunkCounter == 0)
+				{
+					int bomOffset = GetBomOffset(buffer);
+					if (amountRead - bomOffset > 0)
+					{
+						streamStack.Push(buffer, bomOffset, amountRead - bomOffset);
+					}
+				}
+				else
+				{
+					streamStack.Push(buffer, 0, amountRead);
+				}
+
+				processedChunkCounter++;
 			}
 		}