folkslinux
/
windows-terminal
mirror of https://github.com/microsoft/terminal.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix ConPTY inputs incorrectly being treated as plain text (#16352) 

This is my proposal to avoid aborting ConPTY input parsing because a
read accidentally got split up into more than one chunk. This happens a
lot with WSL for me, as I often get (for instance) a
`\x1b[67;46;99;0;32;` input followed immediately by a `1_` input. The
current logic would cause both of these to be flushed out to the client
application.

This PR fixes the issue by only flushing either a standalone escape
character or a escape+character combination. It basically limits the
previous code to just `VTStates::Ground` and `VTStates::Escape`.

I'm not using the `_state` member, because `VTStates::OscParam` makes no
distinction between `\x1b]` and `\x1b]1234` and I only want to flush the
former. I felt like checking the contents of `run` directly is easier to
understand.

Related to #16343

## Validation Steps Performed
* win32-input-mode sequences are now properly buffered ✅
* Standalone alt-key combinations are still being flushed ✅
This commit is contained in:
Leonard Hecker 2023-12-05 20:37:58 +01:00 committed by GitHub
parent 71a6f26e6e
commit 5f5ef10571
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
src/terminal/parser/stateMachine.cpp
View File

@ -2127,6 +2127,8 @@ void StateMachine::ProcessString(const std::wstring_view string)
// If we're at the end of the string and have remaining un-printed characters,
if (_state != VTStates::Ground)
{
const auto run = _CurrentRun();
// 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
// also the start of a CSI, and could be the start of a longer sequence,
@ -2136,60 +2138,28 @@ void StateMachine::ProcessString(const std::wstring_view string)
// <kbd>alt+[</kbd>, <kbd>A</kbd> would be processed like `\x1b[A`,
// which is _wrong_).
//
// Fortunately, for VT input, each keystroke comes in as an individual
// write operation. So, if at the end of processing a string for the
// InputEngine, we find that we're not in the Ground state, that implies
// that we've processed some input, but not dispatched it yet. This
// block at the end of `ProcessString` will then re-process the
// undispatched string, but it will ensure that it dispatches on the
// last character of the string. For our previous `\x1b[` scenario, that
// means we'll make sure to call `_ActionEscDispatch('[')`., which will
// properly decode the string as <kbd>alt+[</kbd>.
const auto run = _CurrentRun();
// At the same time, input may be broken up arbitrarily, depending on the pipe's
// buffer size, our read-buffer size, the sender's write-buffer size, and more.
// In fact, with the current WSL, input is broken up in 16 byte chunks (Why? :(),
// which breaks up many of our longer sequences, like our Win32InputMode ones.
//
// As a heuristic, this code specifically checks for a trailing Esc or Alt+key.
if (_isEngineForInput)
{
// Reset our state, and put all but the last char in again.
ResetState();
_processingLastCharacter = false;
// Chars to flush are [pwchSequenceStart, pwchCurr)
auto wchIter = run.cbegin();
while (wchIter < run.cend() - 1)
if (run.size() <= 2 && run.front() == L'\x1b')
{
ProcessCharacter(*wchIter);
wchIter++;
_EnterGround();
if (run.size() == 1)
{
_ActionExecute(L'\x1b');
}
else
{
_EnterEscape();
_ActionEscDispatch(run.back());
}
_EnterGround();
}
// Manually execute the last char [pwchCurr]
_processingLastCharacter = true;
switch (_state)
{
case VTStates::Ground:
_ActionExecute(*wchIter);
break;
case VTStates::Escape:
case VTStates::EscapeIntermediate:
_ActionEscDispatch(*wchIter);
break;
case VTStates::CsiEntry:
case VTStates::CsiIntermediate:
case VTStates::CsiIgnore:
case VTStates::CsiParam:
case VTStates::CsiSubParam:
_ActionCsiDispatch(*wchIter);
break;
case VTStates::OscParam:
case VTStates::OscString:
case VTStates::OscTermination:
_ActionOscDispatch(*wchIter);
break;
case VTStates::Ss3Entry:
case VTStates::Ss3Param:
_ActionSs3Dispatch(*wchIter);
break;
}
// microsoft/terminal#2746: Make sure to return to the ground state
// after dispatching the characters
_EnterGround();
}
else if (_state != VTStates::SosPmApcString && _state != VTStates::DcsPassThrough && _state != VTStates::DcsIgnore)
{

14
src/terminal/parser/ut_parser/InputEngineTest.cpp
View File

@ -260,6 +260,7 @@ class Microsoft::Console::VirtualTerminal::InputEngineTest
TEST_METHOD(AltCtrlDTest);
TEST_METHOD(AltIntermediateTest);
TEST_METHOD(AltBackspaceEnterTest);
TEST_METHOD(ChunkedSequence);
TEST_METHOD(SGRMouseTest_ButtonClick);
TEST_METHOD(SGRMouseTest_Modifiers);
TEST_METHOD(SGRMouseTest_Movement);
@ -1041,6 +1042,19 @@ void InputEngineTest::AltBackspaceEnterTest()
VerifyExpectedInputDrained();
}
void InputEngineTest::ChunkedSequence()
{
// This test ensures that a DSC sequence that's split up into multiple chunks isn't
// confused with a single Alt+key combination like in the AltBackspaceEnterTest().
// Basically, it tests the selectivity of the AltBackspaceEnterTest() fix.
auto dispatch = std::make_unique<TestInteractDispatch>(nullptr, nullptr);
auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
StateMachine stateMachine{ std::move(inputEngine) };
stateMachine.ProcessString(L"\x1b[1");
VERIFY_ARE_EQUAL(StateMachine::VTStates::CsiParam, stateMachine._state);
}
// Method Description:
// - Writes an SGR VT sequence based on the necessary parameters
// Arguments: