Skip to content

Commit

Permalink
Fix input sequences split across the buffer boundary (#17738)
Browse files Browse the repository at this point in the history
## 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

(cherry picked from commit 131728b)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgSCpCk
Service-Version: 1.21
  • Loading branch information
j4james authored and DHowett committed Sep 26, 2024
1 parent 78c36ca commit 0982abe
Showing 1 changed file with 13 additions and 5 deletions.
18 changes: 13 additions & 5 deletions src/terminal/parser/stateMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2136,6 +2136,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
// <kbd>alt+[</kbd>. In VT, that's encoded as `\x1b[`. However, that's
Expand Down Expand Up @@ -2173,15 +2174,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{});
Expand Down

0 comments on commit 0982abe

Please sign in to comment.