diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index 0a1779c6a64..72e2f846e40 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -17,6 +17,7 @@ StateMachine::StateMachine(std::unique_ptr engine) : _intermediates{}, _parameters{}, _oscString{}, + _cachedSequence{ std::nullopt }, _processingIndividually(false) { _ActionClear(); @@ -533,6 +534,7 @@ void StateMachine::_ActionSs3Dispatch(const wchar_t wch) void StateMachine::_EnterGround() noexcept { _state = VTStates::Ground; + _cachedSequence.reset(); // entering ground means we've completed the pending sequence _trace.TraceStateChange(L"Ground"); } @@ -1226,11 +1228,27 @@ void StateMachine::ProcessCharacter(const wchar_t wch) // - true if the engine successfully handled the string. bool StateMachine::FlushToTerminal() { - // _pwchCurr is incremented after a call to ProcessCharacter to indicate - // that pwchCurr was processed. - // However, if we're here, then the processing of pwchChar triggered the - // engine to request the entire sequence get passed through, including pwchCurr. - return _engine->ActionPassThroughString(_run); + bool success{ true }; + + if (success && _cachedSequence.has_value()) + { + // Flush the partial sequence to the terminal before we flush the rest of it. + // We always want to clear the sequence, even if we failed, so we don't accumulate bad state + // and dump it out elsewhere later. + success = _engine->ActionPassThroughString(*_cachedSequence); + _cachedSequence.reset(); + } + + if (success) + { + // _pwchCurr is incremented after a call to ProcessCharacter to indicate + // that pwchCurr was processed. + // However, if we're here, then the processing of pwchChar triggered the + // engine to request the entire sequence get passed through, including pwchCurr. + success = _engine->ActionPassThroughString(_run); + } + + return success; } // Routine Description: @@ -1365,6 +1383,13 @@ void StateMachine::ProcessString(const std::wstring_view string) // after dispatching the characters _EnterGround(); } + else + { + // If the engine doesn't require flushing at the end of the string, we + // want to cache the partial sequence in case we have to flush the whole + // thing to the terminal later. + _cachedSequence = _cachedSequence.value_or(std::wstring{}) + std::wstring{ _run }; + } } } diff --git a/src/terminal/parser/stateMachine.hpp b/src/terminal/parser/stateMachine.hpp index e2b521f2c80..ef81387d185 100644 --- a/src/terminal/parser/stateMachine.hpp +++ b/src/terminal/parser/stateMachine.hpp @@ -115,6 +115,8 @@ namespace Microsoft::Console::VirtualTerminal std::wstring _oscString; size_t _oscParameter; + std::optional _cachedSequence; + // This is tracked per state machine instance so that separate calls to Process* // can start and finish a sequence. bool _processingIndividually; diff --git a/src/terminal/parser/ut_parser/StateMachineTest.cpp b/src/terminal/parser/ut_parser/StateMachineTest.cpp index 85fbb872e8b..77f5395f386 100644 --- a/src/terminal/parser/ut_parser/StateMachineTest.cpp +++ b/src/terminal/parser/ut_parser/StateMachineTest.cpp @@ -28,18 +28,25 @@ using namespace Microsoft::Console::VirtualTerminal; class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStateMachineEngine { public: + void ResetTestState() + { + printed.clear(); + passedThrough.clear(); + csiParams.reset(); + } + bool ActionExecute(const wchar_t /* wch */) override { return true; }; bool ActionExecuteFromEscape(const wchar_t /* wch */) override { return true; }; bool ActionPrint(const wchar_t /* wch */) override { return true; }; bool ActionPrintString(const std::wstring_view string) override { - printed = string; + printed += string; return true; }; bool ActionPassThroughString(const std::wstring_view string) override { - passedThrough = string; + passedThrough += string; return true; }; @@ -52,7 +59,15 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat bool ActionOscDispatch(const wchar_t /* wch */, const size_t /* parameter */, - const std::wstring_view /* string */) override { return true; }; + const std::wstring_view /* string */) override + { + if (pfnFlushToTerminal) + { + pfnFlushToTerminal(); + return true; + } + return true; + }; bool ActionSs3Dispatch(const wchar_t /* wch */, const std::basic_string_view /* parameters */) override { return true; }; @@ -111,6 +126,7 @@ class Microsoft::Console::VirtualTerminal::StateMachineTest TEST_METHOD(PassThroughUnhandled); TEST_METHOD(RunStorageBeforeEscape); TEST_METHOD(BulkTextPrint); + TEST_METHOD(PassThroughUnhandledSplitAcrossWrites); }; void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachother() @@ -182,3 +198,49 @@ void StateMachineTest::BulkTextPrint() // Then ensure the entire buffered run was printed all at once back to us. VERIFY_ARE_EQUAL(String(L"12345 Hello World"), String(engine.printed.c_str())); } + +void StateMachineTest::PassThroughUnhandledSplitAcrossWrites() +{ + auto enginePtr{ std::make_unique() }; + // this dance is required because StateMachine presumes to take ownership of its engine. + auto& engine{ *enginePtr.get() }; + StateMachine machine{ std::move(enginePtr) }; + + // Hook up the passthrough function. + engine.pfnFlushToTerminal = std::bind(&StateMachine::FlushToTerminal, &machine); + + // Broken in two pieces (test case from GH#3081) + machine.ProcessString(L"\x1b[?12"); + VERIFY_ARE_EQUAL(L"", engine.passedThrough); // nothing out yet + VERIFY_ARE_EQUAL(L"", engine.printed); + + machine.ProcessString(L"34h"); + VERIFY_ARE_EQUAL(L"\x1b[?1234h", engine.passedThrough); // whole sequence out, no other output + VERIFY_ARE_EQUAL(L"", engine.printed); + + engine.ResetTestState(); + + // Three pieces + machine.ProcessString(L"\x1b[?2"); + VERIFY_ARE_EQUAL(L"", engine.passedThrough); // nothing out yet + VERIFY_ARE_EQUAL(L"", engine.printed); + + machine.ProcessString(L"34"); + VERIFY_ARE_EQUAL(L"", engine.passedThrough); // nothing out yet + VERIFY_ARE_EQUAL(L"", engine.printed); + + machine.ProcessString(L"5h"); + VERIFY_ARE_EQUAL(L"\x1b[?2345h", engine.passedThrough); // whole sequence out, no other output + VERIFY_ARE_EQUAL(L"", engine.printed); + + engine.ResetTestState(); + + // Split during OSC terminator (test case from GH#3080) + machine.ProcessString(L"\x1b]99;foo\x1b"); + VERIFY_ARE_EQUAL(L"", engine.passedThrough); // nothing out yet + VERIFY_ARE_EQUAL(L"", engine.printed); + + machine.ProcessString(L"\\"); + VERIFY_ARE_EQUAL(L"\x1b]99;foo\x1b\\", engine.passedThrough); + VERIFY_ARE_EQUAL(L"", engine.printed); +}