Skip to content

Commit

Permalink
Fix parsing of chunked win32-input-mode sequences (#16466)
Browse files Browse the repository at this point in the history
Even with the previous fixes we still randomly encounter win32-
input-mode sequences that are broken up in exactly such a way that
e.g. lone escape keys are encounters. Those for instance clear the
current prompt. The remaining parts of the sequence are then visible.

This changeset fixes the issue by skipping the entire force-to-ground
code whenever we saw at least 1 win32-input-mode sequence.

Related to #16343

## Validation Steps Performed
* Host a ConPTY inside ConPTY (= double the trouble) with cmd.exe
* Paste random amounts of text
* In the old code spurious `[..._` strings are seen
* In the new code they're consistently gone ✅

(cherry picked from commit bc18348)
Service-Card-Id: 91337332
Service-Version: 1.19
  • Loading branch information
lhecker authored and DHowett committed Dec 15, 2023
1 parent 91e97c1 commit 2546c02
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/terminal/parser/IStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ namespace Microsoft::Console::VirtualTerminal
IStateMachineEngine& operator=(const IStateMachineEngine&) = default;
IStateMachineEngine& operator=(IStateMachineEngine&&) = default;

virtual bool EncounteredWin32InputModeSequence() const noexcept = 0;

virtual bool ActionExecute(const wchar_t wch) = 0;
virtual bool ActionExecuteFromEscape(const wchar_t wch) = 0;
virtual bool ActionPrint(const wchar_t wch) = 0;
Expand Down
6 changes: 6 additions & 0 deletions src/terminal/parser/InputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ InputStateMachineEngine::InputStateMachineEngine(std::unique_ptr<IInteractDispat
THROW_HR_IF_NULL(E_INVALIDARG, _pDispatch.get());
}

bool InputStateMachineEngine::EncounteredWin32InputModeSequence() const noexcept
{
return _encounteredWin32InputModeSequence;
}

void InputStateMachineEngine::SetLookingForDSR(const bool looking) noexcept
{
_lookingForDSR = looking;
Expand Down Expand Up @@ -448,6 +453,7 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
// Ctrl+C, Ctrl+Break are handled correctly.
const auto key = _GenerateWin32Key(parameters);
success = _pDispatch->WriteCtrlKey(key);
_encounteredWin32InputModeSequence = true;
break;
}
default:
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/parser/InputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ namespace Microsoft::Console::VirtualTerminal
InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch,
const bool lookingForDSR);

bool EncounteredWin32InputModeSequence() const noexcept override;
void SetLookingForDSR(const bool looking) noexcept;

bool ActionExecute(const wchar_t wch) override;
Expand Down Expand Up @@ -167,6 +168,7 @@ namespace Microsoft::Console::VirtualTerminal
const std::unique_ptr<IInteractDispatch> _pDispatch;
std::function<bool()> _pfnFlushToInputQueue;
bool _lookingForDSR;
bool _encounteredWin32InputModeSequence = false;
DWORD _mouseButtonState = 0;
std::chrono::milliseconds _doubleClickTime;
std::optional<til::point> _lastMouseClickPos{};
Expand Down
5 changes: 5 additions & 0 deletions src/terminal/parser/OutputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ OutputStateMachineEngine::OutputStateMachineEngine(std::unique_ptr<ITermDispatch
THROW_HR_IF_NULL(E_INVALIDARG, _dispatch.get());
}

bool OutputStateMachineEngine::EncounteredWin32InputModeSequence() const noexcept
{
return false;
}

const ITermDispatch& OutputStateMachineEngine::Dispatch() const noexcept
{
return *_dispatch;
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/parser/OutputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ namespace Microsoft::Console::VirtualTerminal

OutputStateMachineEngine(std::unique_ptr<ITermDispatch> pDispatch);

bool EncounteredWin32InputModeSequence() const noexcept override;

bool ActionExecute(const wchar_t wch) override;
bool ActionExecuteFromEscape(const wchar_t wch) override;

Expand Down
8 changes: 7 additions & 1 deletion src/terminal/parser/stateMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2144,9 +2144,15 @@ void StateMachine::ProcessString(const std::wstring_view string)
// 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 we encountered a win32-input-mode sequence before, we know that our \x1b[?9001h
// request to enable them was successful. While a client may still send \x1b{some char}
// intentionally, it's far more likely now that we're looking at a broken up sequence.
// The most common win32-input-mode is ConPTY itself after all, and we never emit
// \x1b{some char} once it's enabled.
if (_isEngineForInput)
{
if (run.size() <= 2 && run.front() == L'\x1b')
const auto win32 = _engine->EncounteredWin32InputModeSequence();
if (!win32 && run.size() <= 2 && run.front() == L'\x1b')
{
_EnterGround();
if (run.size() == 1)
Expand Down
5 changes: 5 additions & 0 deletions src/terminal/parser/ut_parser/StateMachineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStat
dcsDataString.clear();
}

bool EncounteredWin32InputModeSequence() const noexcept override
{
return false;
}

bool ActionExecute(const wchar_t wch) override
{
executed += wch;
Expand Down

0 comments on commit 2546c02

Please sign in to comment.