Skip to content

Commit

Permalink
Set Console.InputEncoding to avoid having a preamble written in .NET …
Browse files Browse the repository at this point in the history 
…Framework
  • Loading branch information
@jeffkl
jeffkl committed Sep 29, 2023
1 parent 9d4545b commit 71b9b54
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 140 deletions.
91 changes: 9 additions & 82 deletions src/NuGet.Core/NuGet.Build.Tasks.Console/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ private static bool TryParseArguments(string[] args, Func<Stream> getStream, Tex
else
{
#endif
using var reader = new BinaryReader(getStream());
using var reader = new BinaryReader(getStream(), Encoding.UTF8, leaveOpen: true);

if (!TryDeserializeGlobalProperties(errorWriter, reader, out globalProperties))
{
Expand Down Expand Up @@ -233,109 +233,36 @@ private static bool LogError(TextWriter errorWriter, string format, params objec
/// Attempts to deserialize global properties from the standard input stream.
/// </summary>
/// <remarks>
/// The stream backed by the specified <see cref="BinaryReader" /> may contain a preamble/byte order mark. 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 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 integer in the stream and the preamble is skipped.
///
/// </remarks>
/// <param name="errorWriter">A <see cref="TextWriter" /> to write errors to if one occurs.</param>
/// <param name="reader">The <see cref="BinaryReader" /> to use when deserializing the arguments.</param>
/// <param name="globalProperties">Receives a <see cref="Dictionary{TKey, TValue}" /> representing the global properties.</param>
/// <returns><see langword="true" /> if the arguments were successfully deserialized, otherwise <see langword="false" />.</returns>
internal static bool TryDeserializeGlobalProperties(TextWriter errorWriter, BinaryReader reader, out Dictionary<string, string> globalProperties)
{
globalProperties = null;

byte[] buffer = new byte[4];
int count = 0;

try
{
// Attempt to read the first 4 bytes into the buffer which contain a variable length preamble or an integer representing the number of items in the dictionary
if (reader.Read(buffer, 0, buffer.Length) != buffer.Length)
{
// An error occurred parsing command-line arguments in static graph-based restore as end of the standard input stream was unexpectedly encountered. Please file an issue at https://github.com/NuGet/Home
return LogError(errorWriter, Strings.Error_StaticGraphRestoreArgumentsParsingFailedEndOfStream);
}
// Read the first integer from the stream which is the number of global properties
count = reader.ReadInt32();
}
catch (Exception e)
{
// An error occurred parsing command-line arguments in static graph-based restore as an exception occurred reading the standard input stream, {0}. Please file an issue at https://github.com/NuGet/Home
return LogError(errorWriter, Strings.Error_StaticGraphRestoreArgumentsParsingFailedExceptionReadingStream, e.Message, e.ToString());
}

using (MemoryStream memoryStream = new MemoryStream(buffer, writable: false))
using (StreamReader streamReader = new StreamReader(memoryStream, Encoding.Default, detectEncodingFromByteOrderMarks: true, bufferSize: buffer.Length))
{
// The call to Read() here will detect the preamble in the buffer by populating streamReader.CurrentEncoding
int result = streamReader.Read();

byte[] preamble = streamReader.CurrentEncoding.GetPreamble();

if (preamble.Length != 0)
{
// Copies the bytes from beyond the preamble to the beginning of the buffer
Array.Copy(buffer, preamble.Length, buffer, 0, buffer.Length - preamble.Length);

try
{
// Reads in next bytes from the BinaryReader to fill the rest of the buffer so that it contains the integer representing the number of items in the dictionary
if (reader.Read(buffer, buffer.Length - preamble.Length, preamble.Length) != preamble.Length)
{
// An error occurred parsing command-line arguments in static graph-based restore as end of the standard input stream was unexpectedly encountered. Please file an issue at https://github.com/NuGet/Home
return LogError(errorWriter, Strings.Error_StaticGraphRestoreArgumentsParsingFailedEndOfStream);
}
}
catch (Exception e)
{
return LogError(errorWriter, Strings.Error_StaticGraphRestoreArgumentsParsingFailedExceptionReadingStream, e.Message, e.ToString());
}
}
}

// Convert the bytes in the buffer to an integer
int length = BitConverter.ToInt32(buffer, 0);

// If the integer is negative or greater than or equal to int.MaxValue, then the integer is invalid. This should never happen unless the bytes in the stream contain completely unexpected values
if (length < 0 || length >= int.MaxValue)
if (count < 0 || count >= int.MaxValue)
{
// An error occurred parsing command-line arguments in static graph-based restore as the first integer read, {0}, was is greater than the allowable value. Please file an issue at https://github.com/NuGet/Home
return LogError(errorWriter, Strings.Error_StaticGraphRestoreArgumentsParsingFailedUnexpectedIntegerValue, length);
return LogError(errorWriter, Strings.Error_StaticGraphRestoreArgumentsParsingFailedUnexpectedIntegerValue, count);
}

globalProperties = new Dictionary<string, string>(length, StringComparer.OrdinalIgnoreCase);
globalProperties = new Dictionary<string, string>(capacity: count, StringComparer.OrdinalIgnoreCase);

for (int i = 0; i < length; i++)
for (int i = 0; i < count; i++)
{
try
{
Expand Down
31 changes: 24 additions & 7 deletions src/NuGet.Core/NuGet.Build.Tasks/StaticGraphRestoreTaskBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ public override bool Execute()
RedirectStandardError = true,
RedirectStandardInput = true,
RedirectStandardOutput = true,
#if !NETFRAMEWORK
StandardInputEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false),
#endif
UseShellExecute = false,
WorkingDirectory = Environment.CurrentDirectory,
};
Expand All @@ -144,17 +147,31 @@ public override bool Execute()
{
Log.LogMessageFromResources(MessageImportance.Low, nameof(Strings.Log_RunningStaticGraphRestoreCommand), process.StartInfo.FileName, process.StartInfo.Arguments);

process.Start();
Encoding previousConsoleInputEncoding = Console.InputEncoding;

// Set the input encoding to UTF8 without a byte order mark, the spawned process will use this encoding on .NET Framework
Console.InputEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false);

if (SerializeGlobalProperties)
try
{
using var writer = new BinaryWriter(process.StandardInput.BaseStream, Encoding.UTF8, leaveOpen: true);
process.Start();

WriteGlobalProperties(writer, globalProperties);
}
process.BeginOutputReadLine();
process.BeginErrorReadLine();

if (SerializeGlobalProperties)
{
using var writer = new BinaryWriter(process.StandardInput.BaseStream, Encoding.UTF8, leaveOpen: true);

process.BeginOutputReadLine();
process.BeginErrorReadLine();
WriteGlobalProperties(writer, globalProperties);
}

process.StandardInput.Close();
}
finally
{
Console.InputEncoding = previousConsoleInputEncoding;
}

semaphore.Wait(_cancellationTokenSource.Token);

Expand Down
58 changes: 7 additions & 51 deletions test/NuGet.Core.Tests/NuGet.Build.Tasks.Console.Test/ProgramTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,11 @@ namespace NuGet.Build.Tasks.Console.Test
{
public class ProgramTests
{
public static IEnumerable<object[]> GetEncodingsToTest()
{
yield return new object[] { System.Console.InputEncoding };

yield return new object[] { Encoding.ASCII };
yield return new object[] { Encoding.Default };

foreach (bool byteOrderMark in new bool[] { true, false })
{
yield return new object[] { new UTF8Encoding(encoderShouldEmitUTF8Identifier: byteOrderMark) };

foreach (bool bigEndian in new bool[] { true, false })
{
yield return new object[] { new UTF32Encoding(bigEndian, byteOrderMark) };

yield return new object[] { new UnicodeEncoding(bigEndian, byteOrderMark) };
}
}
}

/// <summary>
/// Verifies that <see cref="Program.TryDeserializeGlobalProperties(TextWriter, BinaryReader, out Dictionary{string, string})" /> correctly handles if a stream contains a leading preamble.
/// Verifies that <see cref="Program.TryDeserializeGlobalProperties(TextWriter, BinaryReader, out Dictionary{string, string})" /> correctly deserializes from a stream.
/// </summary>
/// <param name="encoding">The <see cref="Encoding" /> to use as a preamble to begin a stream with.</param>
[Theory]
[MemberData(nameof(GetEncodingsToTest))]
public void TryDeserializeGlobalProperties_WhenEncodingUsed_PropertiesAreDeserialized(Encoding encoding)
[Fact]
public void TryDeserializeGlobalProperties_WhenEncodingUsed_PropertiesAreDeserialized()
{
var expectedGlobalProperties = new Dictionary<string, string>
{
Expand All @@ -50,8 +28,6 @@ public void TryDeserializeGlobalProperties_WhenEncodingUsed_PropertiesAreDeseria
using var stream = new MemoryStream();
using var writer = new BinaryWriter(stream);

writer.Write(encoding.GetPreamble());

StaticGraphRestoreTaskBase.WriteGlobalProperties(writer, expectedGlobalProperties);

var errors = new StringBuilder();
Expand All @@ -74,8 +50,6 @@ public void TryDeserializeGlobalProperties_WhenEncodingUsed_PropertiesAreDeseria
[Theory]
[InlineData(new byte[] { 0x00, 0x00, 0x00, 0xFF })] // 4 byte integer that is negative
[InlineData(new byte[] { 0xFF, 0xFF, 0xFF, 0x7F })] // 4 byte integer that is too big
[InlineData(new byte[] { 0xFF, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF })] // Valid UTF-32LE BOM with a 4 byte integer that is negative
[InlineData(new byte[] { 0xFF, 0xFE, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0x7F })] // Valid UTF-32LE BOM with a 4 byte integer after that is too big
public void TryDeserializeGlobalProperties_WhenInvalidDictionaryLength_ReturnsFalse(byte[] bytes)
{
using var stream = GetStreamWithBytes(bytes);
Expand All @@ -85,33 +59,15 @@ public void TryDeserializeGlobalProperties_WhenInvalidDictionaryLength_ReturnsFa
VerifyTryDeserializeGlobalPropertiesError(stream, Strings.Error_StaticGraphRestoreArgumentsParsingFailedUnexpectedIntegerValue, expectedLength);
}

/// <summary>
/// Verifies that <see cref="Program.TryDeserializeGlobalProperties(TextWriter, BinaryReader, out Dictionary{string, string})" /> returns <see langword="false" /> and logs an error if the stream contains bytes that are unexpected.
/// </summary>
/// <param name="bytes">An array of bytes to use as the stream.</param>
[Theory]
[InlineData(new byte[] { 0x01, 0x02 })] // Only two bytes
[InlineData(new byte[] { 0xFF, 0xFE, 0x00, 0x00 })] // Valid UTF-32LE BOM but nothing after that
[InlineData(new byte[] { 0xFF, 0xFE, 0x00, 0x00, 0x01, 0x00 })] // Valid UTF-32LE BOM but not a 4 byte integer after that
public void TryDeserializeGlobalProperties_WhenInvalidFirstBytes_ReturnsFalse(byte[] bytes)
{
using var stream = GetStreamWithBytes(bytes);

VerifyTryDeserializeGlobalPropertiesError(stream, Strings.Error_StaticGraphRestoreArgumentsParsingFailedEndOfStream);
}

/// <summary>
/// Verifies that <see cref="Program.TryDeserializeGlobalProperties(TextWriter, BinaryReader, out Dictionary{string, string})" /> returns <see langword="false" /> and logs an error if reading the stream causes an exception to be thrown.
/// </summary>
/// <param name="throwOnReadCount">Indicates for which call to Stream.Read() should throw, the first, second, or third.</param>
/// <param name="bytes">An array of bytes to use as the stream.</param>
[Theory]
[InlineData(1, new byte[] { 0x01, 0x00, 0x00, 0x00 })] // No preamble/BOM, throw on first Read()
[InlineData(2, new byte[] { 0x01, 0x00, 0x00, 0x00 })] // No preamble/BOM, throw on second Read()
[InlineData(1, new byte[] { 0xEF, 0xBB, 0xBF, 0x01, 0x00, 0x00, 0x00 })] // UTF8 preamble/BOM, throw on first Read()
[InlineData(2, new byte[] { 0xEF, 0xBB, 0xBF, 0x01, 0x00, 0x00, 0x00 })] // UTF8 preamble/BOM, throw on second Read()
[InlineData(3, new byte[] { 0xEF, 0xBB, 0xBF, 0x01, 0x00, 0x00, 0x00 })] // UTF8 preamble/BOM, throw on third Read()
public void TryDeserializeGlobalProperties_WhenStreamReadThrows_ReturnsFalse(int throwOnReadCount, byte[] bytes)
[InlineData(1)] // Throw on first Read()
[InlineData(2)] // Throw on second Read()
public void TryDeserializeGlobalProperties_WhenStreamReadThrows_ReturnsFalse(int throwOnReadCount)
{
var expectedGlobalProperties = new Dictionary<string, string>
{
Expand All @@ -121,7 +77,7 @@ public void TryDeserializeGlobalProperties_WhenStreamReadThrows_ReturnsFalse(int

Exception exception = new InvalidOperationException();

using var stream = new StreamThatThrowsAnException(bytes, throwOnReadCount, exception);
using var stream = new StreamThatThrowsAnException(BitConverter.GetBytes(1), throwOnReadCount, exception);

VerifyTryDeserializeGlobalPropertiesErrorStartsWith(stream, Strings.Error_StaticGraphRestoreArgumentsParsingFailedExceptionReadingStream, exception.Message, string.Empty);
}
Expand Down

0 comments on commit 71b9b54

Please sign in to comment.