Properly handle alternate console stream encodings in StaticGraphRestoreArguments

src/NuGet.Core/NuGet.Build.Tasks.Console/Program.cs

@@ -112,7 +112,7 @@ private static bool TryGetArguments(out StaticGraphRestoreArguments staticGraphR
             {
                 using Stream stream = System.Console.OpenStandardInput();
 
-                staticGraphRestoreArguments = StaticGraphRestoreArguments.Read(stream);
+                staticGraphRestoreArguments = StaticGraphRestoreArguments.Read(stream, System.Console.InputEncoding);
 
                 return staticGraphRestoreArguments != null;
             }

src/NuGet.Core/NuGet.Build.Tasks/StaticGraphRestoreArguments.cs

@@ -13,7 +13,6 @@ namespace NuGet.Build.Tasks
     /// <summary>
     /// Represents arguments to the out-of-proc static graph-based restore which can be written to disk by <see cref="RestoreTaskEx" /> and then read by NuGet.Build.Tasks.Console.
     /// </summary>
-    [Serializable]
     public sealed class StaticGraphRestoreArguments
     {
         /// <summary>
@@ -32,18 +31,22 @@ public sealed class StaticGraphRestoreArguments
         /// </summary>
         /// <param name="stream">A <see cref="Stream" /> to read arguments from.</param>
         /// <returns>A <see cref="StaticGraphRestoreArguments" /> object read from the specified stream.</returns>
-        public static StaticGraphRestoreArguments Read(Stream stream)
+        public static StaticGraphRestoreArguments Read(Stream stream, Encoding encoding)
         {
-            using var reader = new BinaryReader(stream, Encoding.Default, leaveOpen: true);
+            using var reader = new BinaryReader(stream, encoding, leaveOpen: true);
+
+            int count = SkipPreamble(reader, encoding);
 
             return new StaticGraphRestoreArguments
             {
-                GlobalProperties = ReadDictionary(count: reader.ReadInt32()),
-                Options = ReadDictionary(count: reader.ReadInt32())
+                GlobalProperties = ReadDictionary(count),
+                Options = ReadDictionary()
             };
 
-            Dictionary<string, string> ReadDictionary(int count)
+            Dictionary<string, string> ReadDictionary(int count = -1)
             {
+                count = count == -1 ? reader.ReadInt32() : count;
+
                 var dictionary = new Dictionary<string, string>(capacity: count, StringComparer.OrdinalIgnoreCase);
 
                 for (int i = 0; i < count; i++)
@@ -61,9 +64,9 @@ Dictionary<string, string> ReadDictionary(int count)
         /// Writes the current arguments to the specified stream.
         /// </summary>
         /// <param name="stream">A <see cref="Stream" /> to write the arguments to.</param>
-        public void Write(Stream stream)
+        public void Write(StreamWriter streamWriter)
         {
-            using BinaryWriter writer = new BinaryWriter(stream, Encoding.Default, leaveOpen: true);
+            using BinaryWriter writer = new BinaryWriter(streamWriter.BaseStream, streamWriter.Encoding, leaveOpen: true);
 
             WriteDictionary(GlobalProperties);
             WriteDictionary(Options);
@@ -79,5 +82,93 @@ void WriteDictionary(Dictionary<string, string> dictionary)
                 }
             }
         }
+
+        /// <summary>Skips the preamble in the specified <see cref="BinaryReader" />if any and returns the value of the first integer in the stream.</summary>
+        /// <param name="reader">The <see cref="BinaryReader "/> that contains the preamble to skip.</param>
+        /// <param name="encoding">The <see cref="Encoding" /> of the content.</param>
+        /// <returns>The first integer in the stream after the preamble if one was found, otherwise -1.</returns>
+        /// <remarks>
+        /// Preambles are variable length from 2 to 4 bytes.  The first 4 bytes are either:
+        ///   -Variable length preamble and any remaining bytes of the actual content
+        ///   -No preamble and just the integer representing the number of items in the first dictionary
+        ///
+        /// Since the preamble could be 3 bytes, that means that the last byte in the buffer will be the first byte of the next segment. So this code
+        /// determines how long the preamble is, replaces the remaining bytes at the beginning of the buffer, and copies the next set of bytes.  This
+        /// effectively "skips" the preamble by eventually having the buffer contain the first 4 bytes of the content that should actually be read.
+        ///
+        /// Example stream:
+        /// 
+        /// |   3 byte preamble  |      4 byte integer       |
+        /// |------|------|------|------|------|------|------|
+        /// | 0xFF | 0XEF | 0x00 | 0x07 | 0x00 | 0x00 | 0x00 |
+        ///
+        /// The first 4 bytes are read into the buffer (notice one of the bytes is actually not the preamble):
+        /// 
+        /// |       4 byte buffer       |
+        /// |------|------|------|------|
+        /// | 0xFF | 0XEF | 0x00 | 0x07 |
+        ///
+        /// If the first three bytes match the preamble (0xFF, 0xEF, 0x00), then the array is shifted so that last item becomes the first:
+        /// 
+        /// |       4 byte buffer       |
+        /// |------|------|------|------|
+        /// | 0x07 | 0XEF | 0x00 | 0x07 |
+        ///
+        /// Since only 1 byte was moved up in the buffer, then the next 3 bytes from the stream are read:
+        ///
+        /// |       4 byte buffer       |
+        /// |------|------|------|------|
+        /// | 0x07 | 0X00 | 0x00 | 0x00 |
+        ///
+        /// Now the buffer contains the first integer in the stream and the preamble is skipped.
+        /// 
+        /// </remarks>
+        private static int SkipPreamble(BinaryReader reader, Encoding encoding)
+        {
+            // Get the preamble from the current encoding which should only be a maximum of 4 bytes
+            byte[] preamble = encoding.GetPreamble();
+
+            if (preamble.Length == 0)
+            {
+                // Return -1 to the caller if there is no preamble for the encoding meaning that the stream is at the beginning of the expected content
+                return -1;
+            }
+
+            // Create a buffer for the preamble which should be a maximum of 4 bytes
+            byte[] buffer = new byte[4];
+
+            // Read the first 4 bytes of the stream which should be either a preamble of 2 to 4 bytes or a 4 byte integer, if less than 4 bytes were read
+            // then the buffer contains nothing.
+            if (reader.Read(buffer, 0, buffer.Length) != buffer.Length)
+            {
+                return -1;
+            }
+
+            int matchingPreambleLength = 0;
+
+            // Loop through the buffer and verify each byte of the preamble.
+            for (int i = 0; i < preamble.Length; i++)
+            {
+                if (buffer[i] != preamble[i])
+                {
+                    break;
+                }
+
+                matchingPreambleLength++;
+            }
+
+            // If the first set of bytes were the preamble then it needs to be skipped
+            if (matchingPreambleLength == preamble.Length)
+            {
+                // Copy the bytes after the preamble to the start of the buffer
+                Array.Copy(buffer, matchingPreambleLength, buffer, 0, buffer.Length - matchingPreambleLength);
+
+                // Read in the next bytes from the stream into the buffer so it contains the first 4 bytes after the preamble
+                reader.Read(buffer, buffer.Length - matchingPreambleLength, matchingPreambleLength);
+            }
+
+            // Convert the buffer to an integer
+            return BitConverter.ToInt32(buffer, 0);
+        }
     }
 }

src/NuGet.Core/NuGet.Build.Tasks/StaticGraphRestoreTaskBase.cs

@@ -5,7 +5,6 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
-using System.Linq;
 
 #if !IS_CORECLR
 
@@ -120,7 +119,7 @@ public override bool Execute()
 
                         process.Start();
 
-                        WriteArguments(process.StandardInput.BaseStream);
+                        WriteArguments(process.StandardInput);
 
                         process.BeginOutputReadLine();
 
@@ -258,15 +257,15 @@ protected virtual Dictionary<string, string> GetOptions()
             };
         }
 
-        internal void WriteArguments(Stream stream)
+        internal void WriteArguments(StreamWriter streamWriter)
         {
             var arguments = new StaticGraphRestoreArguments
             {
                 GlobalProperties = GetGlobalProperties(),
                 Options = GetOptions(),
             };
 
-            arguments.Write(stream);
+            arguments.Write(streamWriter);
         }
     }
 }

test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/GenerateRestoreGraphFileTaskTests.cs

@@ -52,7 +52,9 @@ public void GetCommandLineArguments_WhenOptionsSpecified_CorrectValuesReturned()
                 {
                     using var stream = new MemoryStream();
 
-                    task.WriteArguments(stream);
+                    using var streamWriter = new StreamWriter(stream, Encoding.UTF8);
+
+                    task.WriteArguments(streamWriter);
 
                     string actualArguments = task.GetCommandLineArguments(msbuildBinPath);
 
@@ -71,7 +73,7 @@ public void GetCommandLineArguments_WhenOptionsSpecified_CorrectValuesReturned()
 
                     stream.Position = 0;
 
-                    var arguments = StaticGraphRestoreArguments.Read(stream);
+                    var arguments = StaticGraphRestoreArguments.Read(stream, streamWriter.Encoding);
 
                     arguments.Options.Should().BeEquivalentTo(new Dictionary<string, string>()
                     {

test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/RestoreTaskExTests.cs

@@ -58,9 +58,10 @@ public void GetCommandLineArguments_WhenOptionsSpecified_CorrectValuesReturned()
                     MSBuildStartupDirectory = testDirectory,
                 })
                 {
-                    using MemoryStream stream = new MemoryStream();
+                    using var stream = new MemoryStream();
+                    using var streamWriter = new StreamWriter(stream, Encoding.UTF8);
 
-                    task.WriteArguments(stream);
+                    task.WriteArguments(streamWriter);
 
                     string actualArguments = task.GetCommandLineArguments(msbuildBinPath);
 
@@ -79,7 +80,7 @@ public void GetCommandLineArguments_WhenOptionsSpecified_CorrectValuesReturned()
 
                     stream.Position = 0;
 
-                    StaticGraphRestoreArguments arguments = StaticGraphRestoreArguments.Read(stream);
+                    StaticGraphRestoreArguments arguments = StaticGraphRestoreArguments.Read(stream, streamWriter.Encoding);
 
                     arguments.Options.Should().BeEquivalentTo(new Dictionary<string, string>()
                     {

test/NuGet.Core.Tests/NuGet.Build.Tasks.Test/StaticGraphRestoreArgumentsTests.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.IO;
 using System.Text;
 using FluentAssertions;
@@ -59,15 +60,72 @@ public void Read_WhenGLobalPropertiesContainComplexCharacters_CanBeRead()
             };
 
             using var stream = new MemoryStream();
+            using var streamWriter = new StreamWriter(stream, Encoding.UTF8);
 
-            expected.Write(stream);
+            expected.Write(streamWriter);
 
             stream.Position = 0;
 
-            StaticGraphRestoreArguments actual = StaticGraphRestoreArguments.Read(stream);
+            StaticGraphRestoreArguments actual = StaticGraphRestoreArguments.Read(stream, streamWriter.Encoding);
 
             actual.GlobalProperties.Should().BeEquivalentTo(expected.GlobalProperties);
             actual.Options.Should().BeEquivalentTo(expected.Options);
         }
+
+        [Theory]
+        [MemberData(nameof(GetEncodingsToTest))]
+        public void Read_WhenStreamContainsByteOrderMark_CanBeRead(Encoding encoding, bool byteOrderMark)
+        {
+            var expected = new StaticGraphRestoreArguments
+            {
+                GlobalProperties = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                {
+                    ["Property1"] = "Value1",
+                    ["Property2"] = "Value2",
+                },
+                Options = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                {
+                    ["Option1"] = bool.TrueString,
+                    ["Option2"] = bool.FalseString,
+                },
+            };
+
+            using var stream = new MemoryStream();
+            using var streamWriter = new StreamWriter(stream, encoding);
+
+            byte[] preamble = encoding.GetPreamble();
+
+            if (byteOrderMark)
+            {
+                if (preamble.Length == 0)
+                {
+                    throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture, "The preamble for the encoding {0} is not defined but the test requested a byte order mark be written.", encoding.EncodingName));
+                }
+
+                stream.Write(preamble, 0, preamble.Length);
+            }
+
+            expected.Write(streamWriter);
+
+            stream.Position = 0;
+
+            StaticGraphRestoreArguments actual = StaticGraphRestoreArguments.Read(stream, encoding);
+
+            actual.GlobalProperties.Should().BeEquivalentTo(expected.GlobalProperties);
+            actual.Options.Should().BeEquivalentTo(expected.Options);
+        }
+
+        public static IEnumerable<object[]> GetEncodingsToTest()
+        {
+            foreach (bool byteOrderMark in new bool[] { true, false })
+            {
+                yield return new object[] { new UTF8Encoding( encoderShouldEmitUTF8Identifier: true), byteOrderMark };
+
+                foreach (bool bigEndian in new bool[] { true, false })
+                {
+                    yield return new object[] { new UTF32Encoding(bigEndian, byteOrderMark), byteOrderMark };
+                }
+            }
+        }
     }
 }