From 72cfeacf3e0c2a77754d88b306907884016bdabf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 12:39:24 +0000 Subject: [PATCH] [patch] Keep a surrogate pair in one output chunk A read of the 4096-char buffer could stop between the two halves of a surrogate pair, so a raw OutputHandler callback got one chunk ending in a lone high surrogate and the next starting with a lone low surrogate. A trailing high surrogate is now held back and put in front of the next read, and flushed on its own at end of stream. Fixes #109 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5g4znzysmAf6XdCd2 --- .../AsyncProcessStreamReaderTests.cs | 71 +++++++++++++++++++ RunCommand/AsyncProcessStreamReader.cs | 45 ++++++++++-- 2 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 RunCommand.Test/AsyncProcessStreamReaderTests.cs diff --git a/RunCommand.Test/AsyncProcessStreamReaderTests.cs b/RunCommand.Test/AsyncProcessStreamReaderTests.cs new file mode 100644 index 0000000..933adc1 --- /dev/null +++ b/RunCommand.Test/AsyncProcessStreamReaderTests.cs @@ -0,0 +1,71 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.RunCommand.Test; + +[TestClass] +public class AsyncProcessStreamReaderTests +{ + private const string Emoji = "\U0001F600"; + private static readonly string[] ExpectedOneCharChunks = ["x", Emoji, Emoji, "y", Emoji]; + private static readonly string[] ExpectedPlainChunks = ["abcd", "ef"]; + + private static async Task> ReadChunksAsync(string text, int bufferSize) + { + List chunks = []; + using StringReader reader = new(text); + await AsyncProcessStreamReader.ReadChunks(reader, new char[bufferSize], chunks.Add).ConfigureAwait(false); + return chunks; + } + + private static void AssertNoChunkSplitsASurrogatePair(List chunks) + { + for (int i = 0; i < chunks.Count; i++) + { + Assert.IsFalse(char.IsHighSurrogate(chunks[i][^1]), $"Chunk {i} ends with a lone high surrogate."); + Assert.IsFalse(char.IsLowSurrogate(chunks[i][0]), $"Chunk {i} starts with a lone low surrogate."); + } + } + + [TestMethod] + public async Task ASurrogatePairOnABufferBoundaryIsDeliveredInOneChunk() + { + // The issue's reproduction: 4095 chars fill all but the last slot of the 4096-char buffer, so + // the read stops between the emoji's two halves. + string text = new string('a', 4095) + Emoji + new string('b', 5000); + + List chunks = await ReadChunksAsync(text, 4096).ConfigureAwait(false); + + AssertNoChunkSplitsASurrogatePair(chunks); + Assert.AreEqual(text, string.Concat(chunks)); + } + + [TestMethod] + public async Task EverySurrogatePairSurvivesAOneCharBuffer() + { + string text = $"x{Emoji}{Emoji}y{Emoji}"; + + List chunks = await ReadChunksAsync(text, 1).ConfigureAwait(false); + + AssertNoChunkSplitsASurrogatePair(chunks); + CollectionAssert.AreEqual(ExpectedOneCharChunks, chunks); + } + + [TestMethod] + public async Task AHighSurrogateAtEndOfStreamIsStillDelivered() + { + // Nothing follows it, so holding it back must not lose it. + string text = "ab\uD83D"; + + List chunks = await ReadChunksAsync(text, 4096).ConfigureAwait(false); + + Assert.AreEqual(text, string.Concat(chunks)); + } + + [TestMethod] + public async Task OutputWithoutSurrogatesIsDeliveredAsItIsRead() + { + List chunks = await ReadChunksAsync("abcdef", 4).ConfigureAwait(false); + + CollectionAssert.AreEqual(ExpectedPlainChunks, chunks); + } +} diff --git a/RunCommand/AsyncProcessStreamReader.cs b/RunCommand/AsyncProcessStreamReader.cs index 39fced5..a529429 100644 --- a/RunCommand/AsyncProcessStreamReader.cs +++ b/RunCommand/AsyncProcessStreamReader.cs @@ -149,13 +149,10 @@ private static void Abandon(params Task[] reads) /// The buffer each read fills. /// The handler each chunk is passed to. /// Whether the stream is standard output. - private async Task ReadToEnd(StreamReader streamReader, char[] buffer, Action? onData, bool isStandardOutput) - { - int charsRead; - while ((charsRead = await streamReader.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(false)) > 0) + private Task ReadToEnd(StreamReader streamReader, char[] buffer, Action? onData, bool isStandardOutput) => + ReadChunks(streamReader, buffer, chunk => { - string data = new(buffer, 0, charsRead); - data = StripLeadingByteOrderMark(data, isStandardOutput); + string data = StripLeadingByteOrderMark(chunk, isStandardOutput); if (data.Length > 0 && onData is not null) { @@ -164,6 +161,42 @@ private async Task ReadToEnd(StreamReader streamReader, char[] buffer, Action + /// Reads until end of stream, handing each read to + /// without ever splitting a surrogate pair between two chunks. + /// + /// + /// A read stops wherever the buffer fills, which can be between the two halves of a surrogate + /// pair. Passed on as it was, one chunk ended with a lone high surrogate and the next began with + /// a lone low surrogate, so a handler that encoded or serialized each chunk on its own wrote two + /// replacement characters instead of the one character the command printed. A trailing high + /// surrogate is therefore held back and put in front of the next read, and handed over on its own + /// only when the stream ends without the low half it was waiting for. + /// + /// The reader to drain. + /// The buffer each read fills. + /// The callback each non-empty chunk is passed to. + internal static async Task ReadChunks(TextReader reader, char[] buffer, Action onChunk) + { + string heldBack = string.Empty; + int charsRead; + while ((charsRead = await reader.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(false)) > 0) + { + int length = char.IsHighSurrogate(buffer[charsRead - 1]) ? charsRead - 1 : charsRead; + string data = heldBack + new string(buffer, 0, length); + heldBack = length == charsRead ? string.Empty : buffer[charsRead - 1].ToString(); + + if (data.Length > 0) + { + onChunk(data); + } + } + + if (heldBack.Length > 0) + { + onChunk(heldBack); } }