diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 65038eb9ba1..f71ffb76472 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -456,20 +456,33 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView) { // TODO: MSFT 21006766 // This is not great but I need it demoable. Fix by making a buffer stream writer. - if (wch >= 0xD800 && wch <= 0xDFFF) + // + // If wch is a surrogate character we need to read 2 code units + // from the stringView to form a single code point. + const auto isSurrogate = wch >= 0xD800 && wch <= 0xDFFF; + const auto view = stringView.substr(i, isSurrogate ? 2 : 1); + const OutputCellIterator it{ view, _buffer->GetCurrentAttributes() }; + const auto end = _buffer->Write(it); + const auto cellDistance = end.GetCellDistance(it); + const auto inputDistance = end.GetInputDistance(it); + + if (inputDistance > 0) { - const OutputCellIterator it{ stringView.substr(i, 2), _buffer->GetCurrentAttributes() }; - const auto end = _buffer->Write(it); - const auto cellDistance = end.GetCellDistance(it); - i += cellDistance - 1; + // If "wch" was a surrogate character, we just consumed 2 code units above. + // -> Increment "i" by 1 in that case and thus by 2 in total in this iteration. proposedCursorPosition.X += gsl::narrow(cellDistance); + i += inputDistance - 1; } else { - const OutputCellIterator it{ stringView.substr(i, 1), _buffer->GetCurrentAttributes() }; - const auto end = _buffer->Write(it); - const auto cellDistance = end.GetCellDistance(it); - proposedCursorPosition.X += gsl::narrow(cellDistance); + // If _WriteBuffer() is called with a consecutive string longer than the viewport/buffer width + // the call to _buffer->Write() will refuse to write anything on the current line. + // GetInputDistance() thus returns 0, which would in turn cause i to be + // decremented by 1 below and force the outer loop to loop forever. + // This if() basically behaves as if "\r\n" had been encountered above and retries the write. + // With well behaving shells during normal operation this safeguard should normally not be encountered. + proposedCursorPosition.X = 0; + proposedCursorPosition.Y++; } } diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp index a4c08504b71..45a64e0f1c5 100644 --- a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp @@ -12,6 +12,9 @@ using namespace winrt::Microsoft::Terminal::Settings; using namespace Microsoft::Terminal::Core; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + namespace TerminalCoreUnitTests { #define WCS(x) WCSHELPER(x) @@ -37,5 +40,74 @@ namespace TerminalCoreUnitTests VERIFY_IS_FALSE(term.SetColorTableEntry(256, 100)); VERIFY_IS_FALSE(term.SetColorTableEntry(512, 100)); } + + // Terminal::_WriteBuffer used to enter infinite loops under certain conditions. + // This test ensures that Terminal::_WriteBuffer doesn't get stuck when + // PrintString() is called with more code units than the buffer width. + TEST_METHOD(PrintStringOfSurrogatePairs) + { + DummyRenderTarget renderTarget; + Terminal term; + term.Create({ 100, 100 }, 3, renderTarget); + + std::wstring text; + text.reserve(600); + + for (size_t i = 0; i < 100; ++i) + { + text.append(L"𐐌𐐜𐐬"); + } + + struct Baton + { + HANDLE done; + std::wstring text; + Terminal* pTerm; + } baton{ + CreateEventW(nullptr, TRUE, FALSE, L"done signal"), + text, + &term, + }; + + Log::Comment(L"Launching thread to write data."); + const auto thread = CreateThread( + nullptr, + 0, + [](LPVOID data) -> DWORD { + const Baton& baton = *reinterpret_cast(data); + Log::Comment(L"Writing data."); + baton.pTerm->PrintString(baton.text); + Log::Comment(L"Setting event."); + SetEvent(baton.done); + return 0; + }, + (LPVOID)&baton, + 0, + nullptr); + + Log::Comment(L"Waiting for the write."); + switch (WaitForSingleObject(baton.done, 2000)) + { + case WAIT_OBJECT_0: + Log::Comment(L"Didn't get stuck. Success."); + break; + case WAIT_TIMEOUT: + Log::Comment(L"Wait timed out. It got stuck."); + Log::Result(WEX::Logging::TestResults::Failed); + break; + case WAIT_FAILED: + Log::Comment(L"Wait failed for some reason. We didn't expect this."); + Log::Result(WEX::Logging::TestResults::Failed); + break; + default: + Log::Comment(L"Wait return code that no one expected. Fail."); + Log::Result(WEX::Logging::TestResults::Failed); + break; + } + + TerminateThread(thread, 0); + CloseHandle(baton.done); + return; + } }; }