From 131728b17d50a81d0ecc0686589a2742b7742149 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 20 Aug 2024 01:22:56 +0100 Subject: [PATCH] Fix input sequences split across the buffer boundary (#17738) ## Summary of the Pull Request When conhost receives input from a conpty connection, and that input arrives in a block larger than our 4K buffer, we can end up with a VT sequence that's split at the buffer boundary. Previously that could result in the start of the sequence being dropped, and the remaining characters being interpreted as individual key presses. This PR attempts to fix the issue by caching the unprocessed characters from the start of the sequence, and then combining them with the second half of the sequence when it's later received. ## Validation Steps Performed I've confirmed that pasting into vim now works correctly with the sample data from issue #16655. I've also tested with a `DECCTR` report larger than 4K which would previously have been corrupted, and which now works as expected. ## PR Checklist - [x] Closes #16655 --- src/terminal/parser/stateMachine.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index 07918e902b..0eebe1308c 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -2029,6 +2029,7 @@ void StateMachine::ProcessString(const std::wstring_view string) if (_state != VTStates::Ground) { const auto run = _CurrentRun(); + auto cacheUnusedRun = true; // One of the "weird things" in VT input is the case of something like // alt+[. In VT, that's encoded as `\x1b[`. However, that's @@ -2066,15 +2067,22 @@ void StateMachine::ProcessString(const std::wstring_view string) _ActionEscDispatch(run.back()); } _EnterGround(); + // No need to cache the run, since we've dealt with it now. + cacheUnusedRun = false; } } - else if (_state != VTStates::SosPmApcString && _state != VTStates::DcsPassThrough && _state != VTStates::DcsIgnore) + else if (_state == VTStates::SosPmApcString || _state == VTStates::DcsPassThrough || _state == VTStates::DcsIgnore) { - // 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. There is no need to do this if we've - // reached one of the string processing states, though, since that data + // There is no need to cache the run if we've reached one of the + // string processing states in the output engine, since that data // will be dealt with as soon as it is received. + cacheUnusedRun = false; + } + + // If the run hasn't been dealt with in one of the cases above, we cache + // the partial sequence in case we have to flush the whole thing later. + if (cacheUnusedRun) + { if (!_cachedSequence) { _cachedSequence.emplace(std::wstring{});