diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index 3a202a5..1fa6117 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -3,6 +3,7 @@ namespace ktsu.RunCommand.Test; using System.Diagnostics; +using System.Globalization; using System.Runtime.CompilerServices; using System.Text; using System.Runtime.InteropServices; @@ -1185,6 +1186,57 @@ public void AUtf8ByteOrderMarkIsStrippedFromTheStartOfOutput() Assert.AreEqual("hello", RunOverPath(path, StrictUtf8)); } + /// + /// Returns the encoding a byte-order-mark test names, along with that encoding's own BOM. + /// + /// + /// Named rather than passed because only takes constants. The + /// cases split on whether the encoding has a preamble: skips a + /// matching preamble itself whatever detectEncodingFromByteOrderMarks says, so one with a + /// preamble must not be stripped a second time by the library. + /// + private static (Encoding Encoding, byte[] Bom) GetByteOrderMarkCase(string name) => name switch + { + "Encoding.UTF8" => (Encoding.UTF8, [0xEF, 0xBB, 0xBF]), + "UTF8Encoding(false)" => (new UTF8Encoding(false), [0xEF, 0xBB, 0xBF]), + "Encoding.Unicode" => (Encoding.Unicode, [0xFF, 0xFE]), + "UnicodeEncoding(false, false)" => (new UnicodeEncoding(bigEndian: false, byteOrderMark: false), [0xFF, 0xFE]), + _ => throw new ArgumentOutOfRangeException(nameof(name), name, "Unknown encoding case."), + }; + + [TestMethod] + [DataRow("Encoding.UTF8")] + [DataRow("UTF8Encoding(false)")] + [DataRow("Encoding.Unicode")] + [DataRow("UnicodeEncoding(false, false)")] + public void OnlyTheByteOrderMarkIsStrippedWhenARealU_FEFFFollowsIt(string encodingName) + { + // A BOM, then U+FEFF that is part of the output, then "A". The BOM goes and the content + // stays, whichever of two equivalent encodings the caller passed. With a preamble the reader + // dropped the BOM and the library then dropped the real U+FEFF after it. + (Encoding encoding, byte[] bom) = GetByteOrderMarkCase(encodingName); + byte[] bytes = [.. bom, .. encoding.GetBytes("\uFEFFA")]; + string path = WriteBytesForTest(bytes, $"{nameof(OnlyTheByteOrderMarkIsStrippedWhenARealU_FEFFFollowsIt)}.{new string([.. encodingName.Where(char.IsLetterOrDigit)])}"); + + string output = RunOverPath(path, encoding); + + Assert.AreEqual("FEFF 0041", string.Join(' ', output.Select(c => ((int)c).ToString("X4", CultureInfo.InvariantCulture)))); + } + + [TestMethod] + [DataRow("Encoding.UTF8")] + [DataRow("UTF8Encoding(false)")] + [DataRow("Encoding.Unicode")] + [DataRow("UnicodeEncoding(false, false)")] + public void ALoneByteOrderMarkIsStrippedWhicheverEncodingReadsIt(string encodingName) + { + (Encoding encoding, byte[] bom) = GetByteOrderMarkCase(encodingName); + byte[] bytes = [.. bom, .. encoding.GetBytes("hello")]; + string path = WriteBytesForTest(bytes, $"{nameof(ALoneByteOrderMarkIsStrippedWhicheverEncodingReadsIt)}.{new string([.. encodingName.Where(char.IsLetterOrDigit)])}"); + + Assert.AreEqual("hello", RunOverPath(path, encoding)); + } + [TestMethod] public void ADecodeFailureIsNotDiscardedWhenTheProcessKeepsRunning() { diff --git a/RunCommand/AsyncProcessStreamReader.cs b/RunCommand/AsyncProcessStreamReader.cs index a529429..48ff508 100644 --- a/RunCommand/AsyncProcessStreamReader.cs +++ b/RunCommand/AsyncProcessStreamReader.cs @@ -32,6 +32,12 @@ internal sealed class AsyncProcessStreamReader(Process process, OutputHandler ou private readonly StreamReader errorStream = new(process.StandardError.BaseStream, outputHandler.Encoding, detectEncodingFromByteOrderMarks: false); + // StreamReader skips the encoding's own preamble at the start of a stream whatever + // detectEncodingFromByteOrderMarks says, so for an encoding that has one (Encoding.UTF8, the + // default, and Encoding.Unicode among them) the BOM is already gone by the time a chunk arrives. + // Stripping again there removed a U+FEFF that was real output. + private readonly bool readerSkipsByteOrderMark = outputHandler.Encoding.GetPreamble().Length > 0; + public void Dispose() { outputStream.Dispose(); @@ -204,11 +210,13 @@ internal static async Task ReadChunks(TextReader reader, char[] buffer, Action /// - /// Turning off the detection above also turned off the stripping that came with it, and a - /// leading U+FEFF in captured output is a change no caller asked for. Every encoding decodes its - /// own BOM to U+FEFF, so dropping that one character covers each of them without guessing at the - /// encoding. A BOM belonging to a different encoding no longer decodes to U+FEFF, which is - /// exactly the case that should surface as mis-decoded bytes rather than be silently honoured. + /// Turning off the detection above also turned off the stripping that came with it for an + /// encoding with no preamble, and a leading U+FEFF in captured output is a change no caller asked + /// for. Every encoding decodes its own BOM to U+FEFF, so dropping that one character covers each + /// of them without guessing at the encoding. A BOM belonging to a different encoding no longer + /// decodes to U+FEFF, which is exactly the case that should surface as mis-decoded bytes rather + /// than be silently honoured. An encoding with a preamble has had its BOM skipped by the reader + /// already, so a U+FEFF at the front of its first chunk is content and is kept. /// /// The chunk just read. /// Whether the chunk came from standard output. @@ -226,7 +234,7 @@ private string StripLeadingByteOrderMark(string data, bool isStandardOutput) errorHasEmitted = true; } - if (hasEmitted || data[0] != ByteOrderMark) + if (hasEmitted || readerSkipsByteOrderMark || data[0] != ByteOrderMark) { return data; }