From 577e65d4eed551de2e43b288cad94ea109095c8a Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 8 Oct 2022 20:01:25 +0100 Subject: [PATCH 01/12] Add a mechanism for injecting into the state machine. --- src/terminal/parser/stateMachine.cpp | 36 ++++++++++++++++++++++++++++ src/terminal/parser/stateMachine.hpp | 6 +++++ 2 files changed, 42 insertions(+) diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index 36848694f32..d6a9aa31489 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -1175,6 +1175,7 @@ void StateMachine::_EventCsiEntry(const wchar_t wch) { _ActionCsiDispatch(wch); _EnterGround(); + _ExecuteCsiCompleteCallback(); } } @@ -1213,6 +1214,7 @@ void StateMachine::_EventCsiIntermediate(const wchar_t wch) { _ActionCsiDispatch(wch); _EnterGround(); + _ExecuteCsiCompleteCallback(); } } @@ -1294,6 +1296,7 @@ void StateMachine::_EventCsiParam(const wchar_t wch) { _ActionCsiDispatch(wch); _EnterGround(); + _ExecuteCsiCompleteCallback(); } } @@ -1996,6 +1999,18 @@ bool StateMachine::IsProcessingLastCharacter() const noexcept return _processingLastCharacter; } +// Routine Description: +// - Registers a function that will be called once the current CSI action is +// complete and the state machine has returned to the ground state. +// Arguments: +// - callback - The function that will be called +// Return Value: +// - +void StateMachine::OnCsiComplete(const std::function callback) +{ + _onCsiCompleteCallback = callback; +} + // Routine Description: // - Wherever the state machine is, whatever it's going, go back to ground. // This is used by conhost to "jiggle the handle" - when VT support is @@ -2058,3 +2073,24 @@ bool StateMachine::_SafeExecuteWithLog(const wchar_t wch, TLambda&& lambda) } return success; } + +void StateMachine::_ExecuteCsiCompleteCallback() +{ + if (_onCsiCompleteCallback) + { + // We need to save the state of the string that we're currently + // processing in case the callback injects another string. + const auto savedCurrentString = _currentString; + const auto savedRunOffset = _runOffset; + const auto savedRunSize = _runSize; + // We also need to take ownership of the callback function before + // executing it so there's no risk of it being run more than once. + const auto callback = std::move(_onCsiCompleteCallback); + callback(); + // Once the callback has returned, we can restore the original state + // and continue where we left off. + _currentString = savedCurrentString; + _runOffset = savedRunOffset; + _runSize = savedRunSize; + } +} diff --git a/src/terminal/parser/stateMachine.hpp b/src/terminal/parser/stateMachine.hpp index 41bce50f97e..a51351e456f 100644 --- a/src/terminal/parser/stateMachine.hpp +++ b/src/terminal/parser/stateMachine.hpp @@ -61,6 +61,8 @@ namespace Microsoft::Console::VirtualTerminal void ProcessString(const std::wstring_view string); bool IsProcessingLastCharacter() const noexcept; + void OnCsiComplete(const std::function callback); + void ResetState() noexcept; bool FlushToTerminal(); @@ -142,6 +144,8 @@ namespace Microsoft::Console::VirtualTerminal template bool _SafeExecuteWithLog(const wchar_t wch, TLambda&& lambda); + void _ExecuteCsiCompleteCallback(); + enum class VTStates { Ground, @@ -202,5 +206,7 @@ namespace Microsoft::Console::VirtualTerminal // can start and finish a sequence. bool _processingIndividually; bool _processingLastCharacter; + + std::function _onCsiCompleteCallback; }; } From de9a2304d953b5159e6694630f3c33af5ee7cbec Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 8 Oct 2022 21:19:39 +0100 Subject: [PATCH 02/12] Hook up the DECDMAC sequence to the dispatcher. --- src/terminal/adapter/DispatchTypes.hpp | 12 ++++++++++++ src/terminal/adapter/ITermDispatch.hpp | 4 ++++ src/terminal/adapter/adaptDispatch.cpp | 16 ++++++++++++++++ src/terminal/adapter/adaptDispatch.hpp | 4 ++++ src/terminal/adapter/termDispatch.hpp | 4 ++++ src/terminal/parser/OutputStateMachineEngine.cpp | 3 +++ src/terminal/parser/OutputStateMachineEngine.hpp | 1 + 7 files changed, 44 insertions(+) diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index 44f3de5dbb3..96629207333 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -507,6 +507,18 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes Size96 = 1 }; + enum class MacroDeleteControl : VTInt + { + DeleteId = 0, + DeleteAll = 1 + }; + + enum class MacroEncoding : VTInt + { + Text = 0, + HexPair = 1 + }; + enum class ReportFormat : VTInt { TerminalStateReport = 1, diff --git a/src/terminal/adapter/ITermDispatch.hpp b/src/terminal/adapter/ITermDispatch.hpp index eab7c1b4000..912843e34d7 100644 --- a/src/terminal/adapter/ITermDispatch.hpp +++ b/src/terminal/adapter/ITermDispatch.hpp @@ -159,6 +159,10 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch const VTParameter cellHeight, const DispatchTypes::DrcsCharsetSize charsetSize) = 0; // DECDLD + virtual StringHandler DefineMacro(const VTInt macroId, + const DispatchTypes::MacroDeleteControl deleteControl, + const DispatchTypes::MacroEncoding encoding) = 0; // DECDMAC + virtual StringHandler RestoreTerminalState(const DispatchTypes::ReportFormat format) = 0; // DECRSTS virtual StringHandler RequestSetting() = 0; // DECRQSS diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 03eae9ae443..8a6fffb0f42 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3177,6 +3177,22 @@ ITermDispatch::StringHandler AdaptDispatch::_CreateDrcsPassthroughHandler(const return nullptr; } +// Method Description: +// - DECDMAC - Defines a string of characters as a macro that can later be +// invoked with a DECINVM sequence. +// Arguments: +// - macroId - a number to identify the macro when invoked. +// - deleteControl - what gets deleted before loading the new macro data. +// - encoding - whether the data is encoded as plain text or hex digits. +// Return Value: +// - a function to receive the macro data or nullptr if parameters are invalid. +ITermDispatch::StringHandler AdaptDispatch::DefineMacro(const VTInt /*macroId*/, + const DispatchTypes::MacroDeleteControl /*deleteControl*/, + const DispatchTypes::MacroEncoding /*encoding*/) +{ + return nullptr; +} + // Method Description: // - DECRSTS - Restores the terminal state from a stream of data previously // saved with a DECRQTSR query. diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index aa233ce145d..bcf90c72fa6 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -153,6 +153,10 @@ namespace Microsoft::Console::VirtualTerminal const VTParameter cellHeight, const DispatchTypes::DrcsCharsetSize charsetSize) override; // DECDLD + StringHandler DefineMacro(const VTInt macroId, + const DispatchTypes::MacroDeleteControl deleteControl, + const DispatchTypes::MacroEncoding encoding) override; // DECDMAC + StringHandler RestoreTerminalState(const DispatchTypes::ReportFormat format) override; // DECRSTS StringHandler RequestSetting() override; // DECRQSS diff --git a/src/terminal/adapter/termDispatch.hpp b/src/terminal/adapter/termDispatch.hpp index 683bd14e621..a0494ebc07f 100644 --- a/src/terminal/adapter/termDispatch.hpp +++ b/src/terminal/adapter/termDispatch.hpp @@ -152,6 +152,10 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons const VTParameter /*cellHeight*/, const DispatchTypes::DrcsCharsetSize /*charsetSize*/) override { return nullptr; } // DECDLD + StringHandler DefineMacro(const VTInt /*macroId*/, + const DispatchTypes::MacroDeleteControl /*deleteControl*/, + const DispatchTypes::MacroEncoding /*encoding*/) override { return nullptr; } // DECDMAC + StringHandler RestoreTerminalState(const DispatchTypes::ReportFormat /*format*/) override { return nullptr; }; // DECRSTS StringHandler RequestSetting() override { return nullptr; }; // DECRQSS diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index c7fa93a2bbf..b893f52647d 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -715,6 +715,9 @@ IStateMachineEngine::StringHandler OutputStateMachineEngine::ActionDcsDispatch(c parameters.at(6), parameters.at(7)); break; + case DcsActionCodes::DECDMAC_DefineMacro: + handler = _dispatch->DefineMacro(parameters.at(0).value_or(0), parameters.at(1), parameters.at(2)); + break; case DcsActionCodes::DECRSTS_RestoreTerminalState: handler = _dispatch->RestoreTerminalState(parameters.at(0)); break; diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp index b125c7b714e..429c088df9b 100644 --- a/src/terminal/parser/OutputStateMachineEngine.hpp +++ b/src/terminal/parser/OutputStateMachineEngine.hpp @@ -162,6 +162,7 @@ namespace Microsoft::Console::VirtualTerminal enum DcsActionCodes : uint64_t { DECDLD_DownloadDRCS = VTID("{"), + DECDMAC_DefineMacro = VTID("!z"), DECRSTS_RestoreTerminalState = VTID("$p"), DECRQSS_RequestSetting = VTID("$q") }; From 3fbb750ac270c14d058b722da79b8a94e9603228 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 9 Oct 2022 17:52:21 +0100 Subject: [PATCH 03/12] Hook up the DECINVM sequence to the dispatcher. --- src/terminal/adapter/ITermDispatch.hpp | 1 + src/terminal/adapter/adaptDispatch.cpp | 12 ++++++++++++ src/terminal/adapter/adaptDispatch.hpp | 1 + src/terminal/adapter/termDispatch.hpp | 1 + src/terminal/parser/OutputStateMachineEngine.cpp | 4 ++++ src/terminal/parser/OutputStateMachineEngine.hpp | 1 + src/terminal/parser/telemetry.cpp | 1 + src/terminal/parser/telemetry.hpp | 1 + 8 files changed, 22 insertions(+) diff --git a/src/terminal/adapter/ITermDispatch.hpp b/src/terminal/adapter/ITermDispatch.hpp index 912843e34d7..2e65c7e51ab 100644 --- a/src/terminal/adapter/ITermDispatch.hpp +++ b/src/terminal/adapter/ITermDispatch.hpp @@ -162,6 +162,7 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch virtual StringHandler DefineMacro(const VTInt macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding) = 0; // DECDMAC + virtual bool InvokeMacro(const VTInt macroId) = 0; // DECINVM virtual StringHandler RestoreTerminalState(const DispatchTypes::ReportFormat format) = 0; // DECRSTS diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 8a6fffb0f42..fc5a713a7a1 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3193,6 +3193,18 @@ ITermDispatch::StringHandler AdaptDispatch::DefineMacro(const VTInt /*macroId*/, return nullptr; } +// Method Description: +// - DECINVM - Invokes a previously defined macro, executing the macro content +// as if it had been received directly from the host. +// Arguments: +// - macroId - the id number of the macro to be invoked. +// Return Value: +// - True +bool AdaptDispatch::InvokeMacro(const VTInt /*macroId*/) +{ + return true; +} + // Method Description: // - DECRSTS - Restores the terminal state from a stream of data previously // saved with a DECRQTSR query. diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index bcf90c72fa6..741f3d737a6 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -156,6 +156,7 @@ namespace Microsoft::Console::VirtualTerminal StringHandler DefineMacro(const VTInt macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding) override; // DECDMAC + bool InvokeMacro(const VTInt macroId) override; // DECINVM StringHandler RestoreTerminalState(const DispatchTypes::ReportFormat format) override; // DECRSTS diff --git a/src/terminal/adapter/termDispatch.hpp b/src/terminal/adapter/termDispatch.hpp index a0494ebc07f..b292401594d 100644 --- a/src/terminal/adapter/termDispatch.hpp +++ b/src/terminal/adapter/termDispatch.hpp @@ -155,6 +155,7 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons StringHandler DefineMacro(const VTInt /*macroId*/, const DispatchTypes::MacroDeleteControl /*deleteControl*/, const DispatchTypes::MacroEncoding /*encoding*/) override { return nullptr; } // DECDMAC + bool InvokeMacro(const VTInt /*macroId*/) override { return false; } // DECINVM StringHandler RestoreTerminalState(const DispatchTypes::ReportFormat /*format*/) override { return nullptr; }; // DECRSTS diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index b893f52647d..fd9d519f2a7 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -664,6 +664,10 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParamete success = _dispatch->SelectAttributeChangeExtent(parameters.at(0)); TermTelemetry::Instance().Log(TermTelemetry::Codes::DECSACE); break; + case CsiActionCodes::DECINVM_InvokeMacro: + success = _dispatch->InvokeMacro(parameters.at(0).value_or(0)); + TermTelemetry::Instance().Log(TermTelemetry::Codes::DECINVM); + break; case CsiActionCodes::DECAC_AssignColor: success = _dispatch->AssignColor(parameters.at(0), parameters.at(1).value_or(0), parameters.at(2).value_or(0)); TermTelemetry::Instance().Log(TermTelemetry::Codes::DECAC); diff --git a/src/terminal/parser/OutputStateMachineEngine.hpp b/src/terminal/parser/OutputStateMachineEngine.hpp index 429c088df9b..c7f528ba806 100644 --- a/src/terminal/parser/OutputStateMachineEngine.hpp +++ b/src/terminal/parser/OutputStateMachineEngine.hpp @@ -155,6 +155,7 @@ namespace Microsoft::Console::VirtualTerminal DECSERA_SelectiveEraseRectangularArea = VTID("${"), DECSCPP_SetColumnsPerPage = VTID("$|"), DECSACE_SelectAttributeChangeExtent = VTID("*x"), + DECINVM_InvokeMacro = VTID("*z"), DECAC_AssignColor = VTID(",|"), DECPS_PlaySound = VTID(",~") }; diff --git a/src/terminal/parser/telemetry.cpp b/src/terminal/parser/telemetry.cpp index 4812fb77544..a55c76e88aa 100644 --- a/src/terminal/parser/telemetry.cpp +++ b/src/terminal/parser/telemetry.cpp @@ -290,6 +290,7 @@ void TermTelemetry::WriteFinalTraceLog() const TraceLoggingUInt32(_uiTimesUsed[DECERA], "DECERA"), TraceLoggingUInt32(_uiTimesUsed[DECSERA], "DECSERA"), TraceLoggingUInt32(_uiTimesUsed[DECSACE], "DECSACE"), + TraceLoggingUInt32(_uiTimesUsed[DECINVM], "DECINVM"), TraceLoggingUInt32(_uiTimesUsed[DECAC], "DECAC"), TraceLoggingUInt32(_uiTimesUsed[DECPS], "DECPS"), TraceLoggingUInt32Array(_uiTimesFailed, ARRAYSIZE(_uiTimesFailed), "Failed"), diff --git a/src/terminal/parser/telemetry.hpp b/src/terminal/parser/telemetry.hpp index 283a163402b..d8b00571f8b 100644 --- a/src/terminal/parser/telemetry.hpp +++ b/src/terminal/parser/telemetry.hpp @@ -117,6 +117,7 @@ namespace Microsoft::Console::VirtualTerminal DECERA, DECSERA, DECSACE, + DECINVM, DECAC, DECPS, // Only use this last enum as a count of the number of codes. From 282d753d5c1c1afdfc4e3cc8ef0737dedd6c24a3 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 9 Oct 2022 17:57:55 +0100 Subject: [PATCH 04/12] Create a class for managing the macro buffer. --- src/terminal/adapter/MacroBuffer.cpp | 252 ++++++++++++++++++ src/terminal/adapter/MacroBuffer.hpp | 70 +++++ src/terminal/adapter/lib/adapter.vcxproj | 2 + .../adapter/lib/adapter.vcxproj.filters | 6 + src/terminal/adapter/sources.inc | 1 + 5 files changed, 331 insertions(+) create mode 100644 src/terminal/adapter/MacroBuffer.cpp create mode 100644 src/terminal/adapter/MacroBuffer.hpp diff --git a/src/terminal/adapter/MacroBuffer.cpp b/src/terminal/adapter/MacroBuffer.cpp new file mode 100644 index 00000000000..ad7216a8c22 --- /dev/null +++ b/src/terminal/adapter/MacroBuffer.cpp @@ -0,0 +1,252 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" +#include "MacroBuffer.hpp" +#include "../parser/ascii.hpp" +#include "../parser/stateMachine.hpp" + +using namespace Microsoft::Console::VirtualTerminal; + +size_t MacroBuffer::GetSpaceAvailable() const noexcept +{ + return MAX_SPACE - _spaceUsed; +} + +std::wstring_view MacroBuffer::GetMacroSequence(const size_t macroId) const noexcept +{ + return macroId < _macros.size() ? til::at(_macros, macroId) : std::wstring_view{}; +} + +void MacroBuffer::InvokeMacroSequence(const std::wstring_view macroSequence, StateMachine& stateMachine) +{ + // Macros can invoke other macros up to a depth of 16, but we don't allow + // the total sequence length to exceed the maximum buffer size, since that's + // likely to facilitate a denial-of-service attack. + const auto allowedLength = MAX_SPACE - _invokedSequenceLength; + if (_invokedDepth < 16 && macroSequence.length() < allowedLength) + { + _invokedSequenceLength += macroSequence.length(); + _invokedDepth++; + auto resetInvokeDepth = wil::scope_exit([&] { + // Once the invoke depth reaches zero, we know we've reached the end + // of the root invoke, so we can reset the sequence length tracker. + if (--_invokedDepth == 0) + { + _invokedSequenceLength = 0; + } + }); + stateMachine.ProcessString(macroSequence); + } +} + +void MacroBuffer::ClearMacrosIfInUse() +{ + // If we receive an RIS from within a macro invocation, we can't release the + // buffer because it's still being used. Instead we'll just replace all the + // macro definitions with NUL characters to prevent any further output. The + // buffer will eventually be released once the invocation finishes. + if (_invokedDepth > 0) + { + for (auto& macro : _macros) + { + std::fill(macro.begin(), macro.end(), AsciiChars::NUL); + } + } +} + +bool MacroBuffer::InitParser(const size_t macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding) noexcept +{ + // We're checking the invoked depth here to make sure we aren't defining + // a macro from within a macro invocation. + if (macroId < _macros.size() && _invokedDepth == 0) + { + _activeMacroId = macroId; + _decodedChar = 0; + _repeatPending = false; + + switch (encoding) + { + case DispatchTypes::MacroEncoding::HexPair: + _parseState = State::ExpectingHexDigit; + break; + case DispatchTypes::MacroEncoding::Text: + _parseState = State::ExpectingText; + break; + default: + return false; + } + + switch (deleteControl) + { + case DispatchTypes::MacroDeleteControl::DeleteId: + _deleteMacro(_activeMacro()); + return true; + case DispatchTypes::MacroDeleteControl::DeleteAll: + for (auto& macro : _macros) + { + _deleteMacro(macro); + } + return true; + default: + return false; + } + } + return false; +} + +bool MacroBuffer::ParseDefinition(const wchar_t ch) +{ + // Once we receive an ESC, that marks the end of the definition, but if + // an unterminated repeat is still pending, we should apply that now. + if (ch == AsciiChars::ESC) + { + if (_repeatPending && !_applyPendingRepeat()) + { + _deleteMacro(_activeMacro()); + } + return false; + } + + // Any other control characters are just ignored. + if (ch < L' ') + { + return true; + } + + // For "text encoded" macros, we'll always be in the ExpectingText state. + // For "hex encoded" macros, we'll typically be alternating between the + // ExpectingHexDigit and ExpectingSecondHexDigit states as we parse the two + // digits of each hex pair. But we also need to deal with repeat sequences, + // which start with `!`, followed by a numeric repeat count, and then a + // range of hex pairs between two `;` characters. When parsing the repeat + // count, we use the ExpectingRepeatCount state, but when parsing the hex + // pairs of the repeat, we just use the regular ExpectingHexDigit states. + + auto success = true; + switch (_parseState) + { + case State::ExpectingText: + success = _appendToActiveMacro(ch); + break; + case State::ExpectingHexDigit: + if (_decodeHexDigit(ch)) + { + _parseState = State::ExpectingSecondHexDigit; + } + else if (ch == L'!' && !_repeatPending) + { + _parseState = State::ExpectingRepeatCount; + _repeatCount = 0; + } + else if (ch == L';' && _repeatPending) + { + success = _applyPendingRepeat(); + } + else + { + success = false; + } + break; + case State::ExpectingSecondHexDigit: + success = _decodeHexDigit(ch) && _appendToActiveMacro(_decodedChar); + _decodedChar = 0; + _parseState = State::ExpectingHexDigit; + break; + case State::ExpectingRepeatCount: + if (ch >= L'0' && ch <= L'9') + { + _repeatCount = _repeatCount * 10 + (ch - L'0'); + _repeatCount = std::min(_repeatCount, MAX_PARAMETER_VALUE); + } + else if (ch == L';') + { + _repeatPending = true; + _repeatStart = _activeMacro().length(); + _parseState = State::ExpectingHexDigit; + } + else + { + success = false; + } + break; + default: + success = false; + break; + } + + // If there is an error in the definition, clear everything received so far. + if (!success) + { + _deleteMacro(_activeMacro()); + } + return success; +} + +bool MacroBuffer::_decodeHexDigit(const wchar_t ch) noexcept +{ + _decodedChar <<= 4; + if (ch >= L'0' && ch <= L'9') + { + _decodedChar += (ch - L'0'); + return true; + } + else if (ch >= L'A' && ch <= L'F') + { + _decodedChar += (ch - L'A' + 10); + return true; + } + else if (ch >= L'a' && ch <= L'f') + { + _decodedChar += (ch - L'a' + 10); + return true; + } + return false; +} + +bool MacroBuffer::_appendToActiveMacro(const wchar_t ch) +{ + if (GetSpaceAvailable() > 0) + { + _activeMacro().push_back(ch); + _spaceUsed++; + return true; + } + return false; +} + +std::wstring& MacroBuffer::_activeMacro() noexcept +{ + return til::at(_macros, _activeMacroId); +} + +void MacroBuffer::_deleteMacro(std::wstring& macro) noexcept +{ + _spaceUsed -= macro.length(); + std::wstring{}.swap(macro); +} + +bool MacroBuffer::_applyPendingRepeat() +{ + auto& activeMacro = _activeMacro(); + const auto sequenceLength = activeMacro.length() - _repeatStart; + if (sequenceLength && _repeatCount > 1) + { + // Note that the repeat sequence has already been written to the buffer + // once while it was being parsed, so we only need to append additional + // copies for repeat counts that are greater than one. If there is not + // enough space for the additional content, we'll just abort the macro. + const auto spaceRequired = (_repeatCount - 1) * sequenceLength; + if (spaceRequired > GetSpaceAvailable()) + { + return false; + } + for (auto i = 1u; i < _repeatCount; i++) + { + activeMacro.append(activeMacro.substr(_repeatStart, sequenceLength)); + _spaceUsed += sequenceLength; + } + } + _repeatPending = false; + return true; +} diff --git a/src/terminal/adapter/MacroBuffer.hpp b/src/terminal/adapter/MacroBuffer.hpp new file mode 100644 index 00000000000..3c51a0eacb3 --- /dev/null +++ b/src/terminal/adapter/MacroBuffer.hpp @@ -0,0 +1,70 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- MacrosBuffer.hpp + +Abstract: +- This manages the parsing and storage of macros defined by the DECDMAC control sequence. +--*/ + +#pragma once + +#include "DispatchTypes.hpp" +#include +#include +#include + +namespace Microsoft::Console::VirtualTerminal +{ + class StateMachine; + + class MacroBuffer + { + public: + // The original DEC terminals only supported 6K of memory, which is + // probably a bit low for modern usage. But we also don't want to make + // this value too large, otherwise it could be used in a denial-of- + // service attack. So for now this is probably a sufficient limit, but + // we may need to increase it in the future if we intend to support + // macros containing sixel sequences. + static constexpr size_t MAX_SPACE = 0x40000; + + MacroBuffer() = default; + ~MacroBuffer() = default; + + size_t GetSpaceAvailable() const noexcept; + std::wstring_view GetMacroSequence(const size_t macroId) const noexcept; + void InvokeMacroSequence(const std::wstring_view macroSequence, StateMachine& stateMachine); + void ClearMacrosIfInUse(); + bool InitParser(const size_t macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding) noexcept; + bool ParseDefinition(const wchar_t ch); + + private: + bool _decodeHexDigit(const wchar_t ch) noexcept; + bool _appendToActiveMacro(const wchar_t ch); + std::wstring& _activeMacro() noexcept; + void _deleteMacro(std::wstring& macro) noexcept; + bool _applyPendingRepeat(); + + enum class State + { + ExpectingText, + ExpectingHexDigit, + ExpectingSecondHexDigit, + ExpectingRepeatCount + }; + + State _parseState{ State::ExpectingText }; + wchar_t _decodedChar{ 0 }; + bool _repeatPending{ false }; + size_t _repeatCount{ 0 }; + size_t _repeatStart{ 0 }; + std::array _macros; + size_t _activeMacroId{ 0 }; + size_t _spaceUsed{ 0 }; + size_t _invokedDepth{ 0 }; + size_t _invokedSequenceLength{ 0 }; + }; +} diff --git a/src/terminal/adapter/lib/adapter.vcxproj b/src/terminal/adapter/lib/adapter.vcxproj index 5bc8488f891..3ecefa5e838 100644 --- a/src/terminal/adapter/lib/adapter.vcxproj +++ b/src/terminal/adapter/lib/adapter.vcxproj @@ -14,6 +14,7 @@ + @@ -29,6 +30,7 @@ + diff --git a/src/terminal/adapter/lib/adapter.vcxproj.filters b/src/terminal/adapter/lib/adapter.vcxproj.filters index 10a5aa55ff7..21524f61cd6 100644 --- a/src/terminal/adapter/lib/adapter.vcxproj.filters +++ b/src/terminal/adapter/lib/adapter.vcxproj.filters @@ -39,6 +39,9 @@ Source Files + + Source Files + @@ -80,6 +83,9 @@ Header Files + + Header Files + diff --git a/src/terminal/adapter/sources.inc b/src/terminal/adapter/sources.inc index 87c5507685a..d1e63430bb4 100644 --- a/src/terminal/adapter/sources.inc +++ b/src/terminal/adapter/sources.inc @@ -33,6 +33,7 @@ SOURCES= \ ..\adaptDispatch.cpp \ ..\FontBuffer.cpp \ ..\InteractDispatch.cpp \ + ..\MacroBuffer.cpp \ ..\adaptDispatchGraphics.cpp \ ..\terminalOutput.cpp \ ..\telemetry.cpp \ From 5ad7f0bb17e7d11bb02e8fd0fe3282f861fc4b76 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 13 Oct 2022 09:46:46 +0100 Subject: [PATCH 05/12] Connect the dispatcher to the macro buffer. --- src/terminal/adapter/adaptDispatch.cpp | 45 +++++++++++++++++++++++--- src/terminal/adapter/adaptDispatch.hpp | 2 ++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index fc5a713a7a1..aac1a7b6dcf 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2358,6 +2358,13 @@ bool AdaptDispatch::HardReset() // Reset the attribute change extent (DECSACE) to a stream. _isChangeExtentRectangular = false; + // Clear and release the macro buffer. + if (_macroBuffer) + { + _macroBuffer->ClearMacrosIfInUse(); + _macroBuffer = nullptr; + } + // GH#2715 - If all this succeeded, but we're in a conpty, return `false` to // make the state machine propagate this RIS sequence to the connected // terminal application. We've reset our state, but the connected terminal @@ -3186,10 +3193,22 @@ ITermDispatch::StringHandler AdaptDispatch::_CreateDrcsPassthroughHandler(const // - encoding - whether the data is encoded as plain text or hex digits. // Return Value: // - a function to receive the macro data or nullptr if parameters are invalid. -ITermDispatch::StringHandler AdaptDispatch::DefineMacro(const VTInt /*macroId*/, - const DispatchTypes::MacroDeleteControl /*deleteControl*/, - const DispatchTypes::MacroEncoding /*encoding*/) +ITermDispatch::StringHandler AdaptDispatch::DefineMacro(const VTInt macroId, + const DispatchTypes::MacroDeleteControl deleteControl, + const DispatchTypes::MacroEncoding encoding) { + if (!_macroBuffer) + { + _macroBuffer = std::make_shared(); + } + + if (_macroBuffer->InitParser(macroId, deleteControl, encoding)) + { + return [&](const auto ch) { + return _macroBuffer->ParseDefinition(ch); + }; + } + return nullptr; } @@ -3200,8 +3219,26 @@ ITermDispatch::StringHandler AdaptDispatch::DefineMacro(const VTInt /*macroId*/, // - macroId - the id number of the macro to be invoked. // Return Value: // - True -bool AdaptDispatch::InvokeMacro(const VTInt /*macroId*/) +bool AdaptDispatch::InvokeMacro(const VTInt macroId) { + if (_macroBuffer) + { + const auto macroSequence = _macroBuffer->GetMacroSequence(macroId); + if (!macroSequence.empty()) + { + // In order to inject our macro sequence into the state machine + // we need to register a callback that will be executed only + // once it has finished processing the current operation, and + // has returned to the ground state. Note that we're capturing + // a copy of the _macroBuffer pointer here to make sure it won't + // be deleted (e.g. from an invoked RIS) while still in use. + const auto macroBuffer = _macroBuffer; + auto& stateMachine = _api.GetStateMachine(); + stateMachine.OnCsiComplete([=, &stateMachine]() { + macroBuffer->InvokeMacroSequence(macroSequence, stateMachine); + }); + } + } return true; } diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index 741f3d737a6..a61d92ca0b4 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -17,6 +17,7 @@ Author(s): #include "termDispatch.hpp" #include "ITerminalApi.hpp" #include "FontBuffer.hpp" +#include "MacroBuffer.hpp" #include "terminalOutput.hpp" #include "../input/terminalInput.hpp" #include "../../types/inc/sgrStack.hpp" @@ -249,6 +250,7 @@ namespace Microsoft::Console::VirtualTerminal TerminalInput& _terminalInput; TerminalOutput _termOutput; std::unique_ptr _fontBuffer; + std::shared_ptr _macroBuffer; std::optional _initialCodePage; // We have two instances of the saved cursor state, because we need From c9cfd62aa93d0c638fe941725db19f4d9f1efbda Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sat, 29 Oct 2022 18:49:50 +0100 Subject: [PATCH 06/12] Add support for the macro space report. --- src/terminal/adapter/DispatchTypes.hpp | 1 + src/terminal/adapter/adaptDispatch.cpp | 17 +++++++++++++++++ src/terminal/adapter/adaptDispatch.hpp | 1 + 3 files changed, 19 insertions(+) diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index 96629207333..a79d7af0181 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -386,6 +386,7 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes OS_OperatingStatus = ANSIStandardStatus(5), CPR_CursorPositionReport = ANSIStandardStatus(6), ExCPR_ExtendedCursorPositionReport = DECPrivateStatus(6), + MSR_MacroSpaceReport = DECPrivateStatus(62), }; using ANSIStandardMode = FlaggedEnumValue<0x00000000>; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index aac1a7b6dcf..86ee078abc1 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -1181,6 +1181,9 @@ bool AdaptDispatch::DeviceStatusReport(const DispatchTypes::StatusType statusTyp case DispatchTypes::StatusType::ExCPR_ExtendedCursorPositionReport: _CursorPositionReport(true); return true; + case DispatchTypes::StatusType::MSR_MacroSpaceReport: + _MacroSpaceReport(); + return true; default: return false; } @@ -1336,6 +1339,20 @@ void AdaptDispatch::_CursorPositionReport(const bool extendedReport) } } +// Routine Description: +// - DECMSR - Reports the amount of space available for macro definitions. +// Arguments: +// - +// Return Value: +// - +void AdaptDispatch::_MacroSpaceReport() const +{ + const auto spaceInBytes = _macroBuffer ? _macroBuffer->GetSpaceAvailable() : MacroBuffer::MAX_SPACE; + // The available space is measured in blocks of 16 bytes, so we need to divide by 16. + const auto response = wil::str_printf(L"\x1b[%zu*{", spaceInBytes / 16); + _api.ReturnResponse(response); +} + // Routine Description: // - Generalizes scrolling movement for up/down // Arguments: diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index a61d92ca0b4..1b2c407cc99 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -219,6 +219,7 @@ namespace Microsoft::Console::VirtualTerminal const VTInt bottomMargin); void _OperatingStatus() const; void _CursorPositionReport(const bool extendedReport); + void _MacroSpaceReport() const; bool _GetParserMode(const StateMachine::Mode mode) const; void _SetParserMode(const StateMachine::Mode mode, const bool enable); From d7e2a83ffa016a882be3d33a9ec5a1cf069c3514 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Sun, 30 Oct 2022 11:43:42 +0000 Subject: [PATCH 07/12] Add support for the macro checksum report. --- src/terminal/adapter/DispatchTypes.hpp | 1 + src/terminal/adapter/ITermDispatch.hpp | 2 +- src/terminal/adapter/MacroBuffer.cpp | 19 ++++++++++++++++++ src/terminal/adapter/MacroBuffer.hpp | 1 + src/terminal/adapter/adaptDispatch.cpp | 20 ++++++++++++++++++- src/terminal/adapter/adaptDispatch.hpp | 3 ++- src/terminal/adapter/termDispatch.hpp | 2 +- .../adapter/ut_adapter/adapterTest.cpp | 12 +++++------ .../parser/OutputStateMachineEngine.cpp | 4 ++-- .../parser/ut_parser/OutputEngineTest.cpp | 2 +- 10 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index a79d7af0181..eaa437b5320 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -387,6 +387,7 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes CPR_CursorPositionReport = ANSIStandardStatus(6), ExCPR_ExtendedCursorPositionReport = DECPrivateStatus(6), MSR_MacroSpaceReport = DECPrivateStatus(62), + MEM_MemoryChecksum = DECPrivateStatus(63), }; using ANSIStandardMode = FlaggedEnumValue<0x00000000>; diff --git a/src/terminal/adapter/ITermDispatch.hpp b/src/terminal/adapter/ITermDispatch.hpp index 2e65c7e51ab..eee04779643 100644 --- a/src/terminal/adapter/ITermDispatch.hpp +++ b/src/terminal/adapter/ITermDispatch.hpp @@ -112,7 +112,7 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch virtual bool ResetMode(const DispatchTypes::ModeParams param) = 0; // DECRST - virtual bool DeviceStatusReport(const DispatchTypes::StatusType statusType) = 0; // DSR, DSR-OS, DSR-CPR + virtual bool DeviceStatusReport(const DispatchTypes::StatusType statusType, const VTParameter id) = 0; // DSR virtual bool DeviceAttributes() = 0; // DA1 virtual bool SecondaryDeviceAttributes() = 0; // DA2 virtual bool TertiaryDeviceAttributes() = 0; // DA3 diff --git a/src/terminal/adapter/MacroBuffer.cpp b/src/terminal/adapter/MacroBuffer.cpp index ad7216a8c22..532cfee8554 100644 --- a/src/terminal/adapter/MacroBuffer.cpp +++ b/src/terminal/adapter/MacroBuffer.cpp @@ -13,6 +13,25 @@ size_t MacroBuffer::GetSpaceAvailable() const noexcept return MAX_SPACE - _spaceUsed; } +uint16_t MacroBuffer::CalculateChecksum() const noexcept +{ + // The algorithm that we're using here is intended to match the checksums + // produced by the original DEC VT420 terminal. Although note that a real + // VT420 would have included the entire macro memory area in the checksum, + // which could still contain remnants of previous macro definitions that + // are no longer active. We don't replicate that behavior, since that's of + // no benefit to applications that might want to use the checksum. + uint16_t checksum = 0; + for (auto& macro : _macros) + { + for (auto ch : macro) + { + checksum -= ch; + } + } + return checksum; +} + std::wstring_view MacroBuffer::GetMacroSequence(const size_t macroId) const noexcept { return macroId < _macros.size() ? til::at(_macros, macroId) : std::wstring_view{}; diff --git a/src/terminal/adapter/MacroBuffer.hpp b/src/terminal/adapter/MacroBuffer.hpp index 3c51a0eacb3..c9294c4c574 100644 --- a/src/terminal/adapter/MacroBuffer.hpp +++ b/src/terminal/adapter/MacroBuffer.hpp @@ -35,6 +35,7 @@ namespace Microsoft::Console::VirtualTerminal ~MacroBuffer() = default; size_t GetSpaceAvailable() const noexcept; + uint16_t CalculateChecksum() const noexcept; std::wstring_view GetMacroSequence(const size_t macroId) const noexcept; void InvokeMacroSequence(const std::wstring_view macroSequence, StateMachine& stateMachine); void ClearMacrosIfInUse(); diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 86ee078abc1..44acffe3040 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -1166,9 +1166,10 @@ bool AdaptDispatch::SetLineRendition(const LineRendition rendition) // - DSR - Reports status of a console property back to the STDIN based on the type of status requested. // Arguments: // - statusType - status type indicating what property we should report back +// - id - a numeric label used to identify the request in DECCKSR reports // Return Value: // - True if handled successfully. False otherwise. -bool AdaptDispatch::DeviceStatusReport(const DispatchTypes::StatusType statusType) +bool AdaptDispatch::DeviceStatusReport(const DispatchTypes::StatusType statusType, const VTParameter id) { switch (statusType) { @@ -1184,6 +1185,9 @@ bool AdaptDispatch::DeviceStatusReport(const DispatchTypes::StatusType statusTyp case DispatchTypes::StatusType::MSR_MacroSpaceReport: _MacroSpaceReport(); return true; + case DispatchTypes::StatusType::MEM_MemoryChecksum: + _MacroChecksumReport(id); + return true; default: return false; } @@ -1353,6 +1357,20 @@ void AdaptDispatch::_MacroSpaceReport() const _api.ReturnResponse(response); } +// Routine Description: +// - DECCKSR - Reports a checksum of the current macro definitions. +// Arguments: +// - id - a numeric label used to identify the DSR request +// Return Value: +// - +void AdaptDispatch::_MacroChecksumReport(const VTParameter id) const +{ + const auto requestId = id.value_or(0); + const auto checksum = _macroBuffer ? _macroBuffer->CalculateChecksum() : 0; + const auto response = wil::str_printf(L"\033P%d!~%04X\033\\", requestId, checksum); + _api.ReturnResponse(response); +} + // Routine Description: // - Generalizes scrolling movement for up/down // Arguments: diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index 1b2c407cc99..7318dbd0042 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -68,7 +68,7 @@ namespace Microsoft::Console::VirtualTerminal bool SetCharacterProtectionAttribute(const VTParameters options) override; // DECSCA bool PushGraphicsRendition(const VTParameters options) override; // XTPUSHSGR bool PopGraphicsRendition() override; // XTPOPSGR - bool DeviceStatusReport(const DispatchTypes::StatusType statusType) override; // DSR, DSR-OS, DSR-CPR + bool DeviceStatusReport(const DispatchTypes::StatusType statusType, const VTParameter id) override; // DSR bool DeviceAttributes() override; // DA1 bool SecondaryDeviceAttributes() override; // DA2 bool TertiaryDeviceAttributes() override; // DA3 @@ -220,6 +220,7 @@ namespace Microsoft::Console::VirtualTerminal void _OperatingStatus() const; void _CursorPositionReport(const bool extendedReport); void _MacroSpaceReport() const; + void _MacroChecksumReport(const VTParameter id) const; bool _GetParserMode(const StateMachine::Mode mode) const; void _SetParserMode(const StateMachine::Mode mode, const bool enable); diff --git a/src/terminal/adapter/termDispatch.hpp b/src/terminal/adapter/termDispatch.hpp index b292401594d..6ee2f413e82 100644 --- a/src/terminal/adapter/termDispatch.hpp +++ b/src/terminal/adapter/termDispatch.hpp @@ -105,7 +105,7 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons bool ResetMode(const DispatchTypes::ModeParams /*param*/) override { return false; } // DECRST - bool DeviceStatusReport(const DispatchTypes::StatusType /*statusType*/) override { return false; } // DSR, DSR-OS, DSR-CPR + bool DeviceStatusReport(const DispatchTypes::StatusType /*statusType*/, const VTParameter /*id*/) override { return false; } // DSR bool DeviceAttributes() override { return false; } // DA1 bool SecondaryDeviceAttributes() override { return false; } // DA2 bool TertiaryDeviceAttributes() override { return false; } // DA3 diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index 74e1381328c..6b01f434c7d 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -1358,7 +1358,7 @@ class AdapterTest Log::Comment(L"Test 1: Verify failure when using bad status."); _testGetSet->PrepData(); - VERIFY_IS_FALSE(_pDispatch->DeviceStatusReport((DispatchTypes::StatusType)-1)); + VERIFY_IS_FALSE(_pDispatch->DeviceStatusReport((DispatchTypes::StatusType)-1, {})); } TEST_METHOD(DeviceStatus_OperatingStatusTests) @@ -1367,7 +1367,7 @@ class AdapterTest Log::Comment(L"Test 1: Verify good operating condition."); _testGetSet->PrepData(); - VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::OS_OperatingStatus)); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::OS_OperatingStatus, {})); _testGetSet->ValidateInputEvent(L"\x1b[0n"); } @@ -1390,7 +1390,7 @@ class AdapterTest coordCursorExpected.X++; coordCursorExpected.Y++; - VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport)); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport, {})); wchar_t pwszBuffer[50]; @@ -1414,7 +1414,7 @@ class AdapterTest // Then note that VT is 1,1 based for the top left, so add 1. (The rest of the console uses 0,0 for array index bases.) coordCursorExpectedFirst += til::point{ 1, 1 }; - VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport)); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport, {})); auto cursorPos = _testGetSet->_textBuffer->GetCursor().GetPosition(); cursorPos.X++; @@ -1424,7 +1424,7 @@ class AdapterTest auto coordCursorExpectedSecond{ coordCursorExpectedFirst }; coordCursorExpectedSecond += til::point{ 1, 1 }; - VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport)); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::CPR_CursorPositionReport, {})); wchar_t pwszBuffer[50]; @@ -1453,7 +1453,7 @@ class AdapterTest // Until we support paging (GH#13892) the reported page number should always be 1. const auto pageExpected = 1; - VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::ExCPR_ExtendedCursorPositionReport)); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::ExCPR_ExtendedCursorPositionReport, {})); wchar_t pwszBuffer[50]; swprintf_s(pwszBuffer, ARRAYSIZE(pwszBuffer), L"\x1b[?%d;%d;%dR", coordCursorExpected.Y, coordCursorExpected.X, pageExpected); diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index fd9d519f2a7..2cd56a3252a 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -530,11 +530,11 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParamete TermTelemetry::Instance().Log(TermTelemetry::Codes::SGR); break; case CsiActionCodes::DSR_DeviceStatusReport: - success = _dispatch->DeviceStatusReport(DispatchTypes::ANSIStandardStatus(parameters.at(0))); + success = _dispatch->DeviceStatusReport(DispatchTypes::ANSIStandardStatus(parameters.at(0)), parameters.at(1)); TermTelemetry::Instance().Log(TermTelemetry::Codes::DSR); break; case CsiActionCodes::DSR_PrivateDeviceStatusReport: - success = _dispatch->DeviceStatusReport(DispatchTypes::DECPrivateStatus(parameters.at(0))); + success = _dispatch->DeviceStatusReport(DispatchTypes::DECPrivateStatus(parameters.at(0)), parameters.at(1)); TermTelemetry::Instance().Log(TermTelemetry::Codes::DSR); break; case CsiActionCodes::DA_DeviceAttributes: diff --git a/src/terminal/parser/ut_parser/OutputEngineTest.cpp b/src/terminal/parser/ut_parser/OutputEngineTest.cpp index 12246eb14d1..2c70286b3f9 100644 --- a/src/terminal/parser/ut_parser/OutputEngineTest.cpp +++ b/src/terminal/parser/ut_parser/OutputEngineTest.cpp @@ -1203,7 +1203,7 @@ class StatefulDispatch final : public TermDispatch } CATCH_LOG_RETURN_FALSE() - bool DeviceStatusReport(const DispatchTypes::StatusType statusType) noexcept override + bool DeviceStatusReport(const DispatchTypes::StatusType statusType, const VTParameter /*id*/) noexcept override { _deviceStatusReport = true; _statusReportType = statusType; From 3744a27f40ff33aa43e77ead65e1b17a657addd6 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 15 Nov 2022 19:45:05 +0000 Subject: [PATCH 08/12] Add some unit tests. --- src/terminal/adapter/MacroBuffer.hpp | 9 + src/terminal/adapter/adaptDispatch.hpp | 9 + .../adapter/ut_adapter/adapterTest.cpp | 216 +++++++++++++++++- 3 files changed, 233 insertions(+), 1 deletion(-) diff --git a/src/terminal/adapter/MacroBuffer.hpp b/src/terminal/adapter/MacroBuffer.hpp index c9294c4c574..64e0a61ba1e 100644 --- a/src/terminal/adapter/MacroBuffer.hpp +++ b/src/terminal/adapter/MacroBuffer.hpp @@ -16,6 +16,11 @@ Module Name: #include #include +// fwdecl unittest classes +#ifdef UNIT_TESTING +class AdapterTest; +#endif + namespace Microsoft::Console::VirtualTerminal { class StateMachine; @@ -67,5 +72,9 @@ namespace Microsoft::Console::VirtualTerminal size_t _spaceUsed{ 0 }; size_t _invokedDepth{ 0 }; size_t _invokedSequenceLength{ 0 }; + +#ifdef UNIT_TESTING + friend class AdapterTest; +#endif }; } diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index 7318dbd0042..13617aeb786 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -22,6 +22,11 @@ Author(s): #include "../input/terminalInput.hpp" #include "../../types/inc/sgrStack.hpp" +// fwdecl unittest classes +#ifdef UNIT_TESTING +class AdapterTest; +#endif + namespace Microsoft::Console::VirtualTerminal { class AdaptDispatch : public ITermDispatch @@ -279,5 +284,9 @@ namespace Microsoft::Console::VirtualTerminal TextAttribute& attr) noexcept; void _ApplyGraphicsOptions(const VTParameters options, TextAttribute& attr) noexcept; + +#ifdef UNIT_TESTING + friend class AdapterTest; +#endif }; } diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index 6b01f434c7d..6bcefd3effb 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -60,8 +60,9 @@ using namespace Microsoft::Console::VirtualTerminal; class TestGetSet final : public ITerminalApi { public: - void PrintString(const std::wstring_view /*string*/) override + void PrintString(const std::wstring_view string) override { + _printed += string; } void ReturnResponse(const std::wstring_view response) override @@ -291,6 +292,8 @@ class TestGetSet final : public ITerminalApi _response.clear(); _retainResponse = false; + + _printed.clear(); } void PrepCursor(CursorX xact, CursorY yact) @@ -382,6 +385,8 @@ class TestGetSet final : public ITerminalApi til::inclusive_rect _expectedScrollRegion; til::inclusive_rect _activeScrollRegion; + std::wstring _printed; + til::point _expectedCursorPos; TextAttribute _expectedAttribute = {}; @@ -1460,6 +1465,73 @@ class AdapterTest _testGetSet->ValidateInputEvent(pwszBuffer); } + TEST_METHOD(DeviceStatus_MacroSpaceReportTest) + { + Log::Comment(L"Starting test..."); + + // Space is measured in blocks of 16 bytes. + const auto availableSpace = MacroBuffer::MAX_SPACE / 16; + + Log::Comment(L"Test 1: Verify maximum space available"); + _testGetSet->PrepData(); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::MSR_MacroSpaceReport, {})); + + wchar_t pwszBuffer[50]; + swprintf_s(pwszBuffer, ARRAYSIZE(pwszBuffer), L"\x1b[%zu*{", availableSpace); + _testGetSet->ValidateInputEvent(pwszBuffer); + + Log::Comment(L"Test 2: Verify space decrease"); + _testGetSet->PrepData(); + // Define four 8-byte macros, i.e. 32 byes (2 macro blocks). + _stateMachine->ProcessString(L"\033P1;0;0!z12345678\033\\"); + _stateMachine->ProcessString(L"\033P2;0;0!z12345678\033\\"); + _stateMachine->ProcessString(L"\033P3;0;0!z12345678\033\\"); + _stateMachine->ProcessString(L"\033P4;0;0!z12345678\033\\"); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::MSR_MacroSpaceReport, {})); + + swprintf_s(pwszBuffer, ARRAYSIZE(pwszBuffer), L"\x1b[%zu*{", availableSpace - 2); + _testGetSet->ValidateInputEvent(pwszBuffer); + + Log::Comment(L"Test 3: Verify space reset"); + _testGetSet->PrepData(); + VERIFY_IS_TRUE(_pDispatch->HardReset()); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::MSR_MacroSpaceReport, {})); + + swprintf_s(pwszBuffer, ARRAYSIZE(pwszBuffer), L"\x1b[%zu*{", availableSpace); + _testGetSet->ValidateInputEvent(pwszBuffer); + } + + TEST_METHOD(DeviceStatus_MemoryChecksumReportTest) + { + Log::Comment(L"Starting test..."); + + Log::Comment(L"Test 1: Verify initial checksum is 0"); + _testGetSet->PrepData(); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::MEM_MemoryChecksum, 12)); + + _testGetSet->ValidateInputEvent(L"\033P12!~0000\033\\"); + + Log::Comment(L"Test 2: Verify checksum after macros defined"); + _testGetSet->PrepData(); + // Define a couple of text macros + _stateMachine->ProcessString(L"\033P1;0;0!zABCD\033\\"); + _stateMachine->ProcessString(L"\033P2;0;0!zabcd\033\\"); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::MEM_MemoryChecksum, 34)); + + // Checksum is a 16-bit negated sum of the macro buffer characters. + const auto checksum = gsl::narrow_cast(-('A' + 'B' + 'C' + 'D' + 'a' + 'b' + 'c' + 'd')); + wchar_t pwszBuffer[50]; + swprintf_s(pwszBuffer, ARRAYSIZE(pwszBuffer), L"\033P34!~%04X\033\\", checksum); + _testGetSet->ValidateInputEvent(pwszBuffer); + + Log::Comment(L"Test 3: Verify checksum resets to 0"); + _testGetSet->PrepData(); + VERIFY_IS_TRUE(_pDispatch->HardReset()); + VERIFY_IS_TRUE(_pDispatch->DeviceStatusReport(DispatchTypes::StatusType::MEM_MemoryChecksum, 56)); + + _testGetSet->ValidateInputEvent(L"\033P56!~0000\033\\"); + } + TEST_METHOD(DeviceAttributesTests) { Log::Comment(L"Starting test..."); @@ -2292,6 +2364,148 @@ class AdapterTest VERIFY_IS_FALSE(_stateMachine->GetParserMode(StateMachine::Mode::AcceptC1)); } + TEST_METHOD(MacroDefinitions) + { + const auto getMacroText = [&](const auto id) { + return _pDispatch->_macroBuffer->_macros.at(id); + }; + + Log::Comment(L"Text encoding"); + _stateMachine->ProcessString(L"\033P1;0;0!zText Encoding\033\\"); + VERIFY_ARE_EQUAL(L"Text Encoding", getMacroText(1)); + + Log::Comment(L"Hex encoding (uppercase)"); + _stateMachine->ProcessString(L"\033P2;0;1!z486578204A4B4C4D4E4F\033\\"); + VERIFY_ARE_EQUAL(L"Hex JKLMNO", getMacroText(2)); + + Log::Comment(L"Hex encoding (lowercase)"); + _stateMachine->ProcessString(L"\033P3;0;1!z486578206a6b6c6d6e6f\033\\"); + VERIFY_ARE_EQUAL(L"Hex jklmno", getMacroText(3)); + + Log::Comment(L"Default encoding is text"); + _stateMachine->ProcessString(L"\033P4;0;!zDefault Encoding\033\\"); + VERIFY_ARE_EQUAL(L"Default Encoding", getMacroText(4)); + + Log::Comment(L"Default ID is 0"); + _stateMachine->ProcessString(L"\033P;0;0!zDefault ID\033\\"); + VERIFY_ARE_EQUAL(L"Default ID", getMacroText(0)); + + Log::Comment(L"Replacing a single macro"); + _stateMachine->ProcessString(L"\033P1;0;0!zRetained\033\\"); + _stateMachine->ProcessString(L"\033P2;0;0!zReplaced\033\\"); + _stateMachine->ProcessString(L"\033P2;0;0!zNew\033\\"); + VERIFY_ARE_EQUAL(L"Retained", getMacroText(1)); + VERIFY_ARE_EQUAL(L"New", getMacroText(2)); + + Log::Comment(L"Replacing all macros"); + _stateMachine->ProcessString(L"\033P1;0;0!zErased\033\\"); + _stateMachine->ProcessString(L"\033P2;0;0!zReplaced\033\\"); + _stateMachine->ProcessString(L"\033P2;1;0!zNew\033\\"); + VERIFY_ARE_EQUAL(L"", getMacroText(1)); + VERIFY_ARE_EQUAL(L"New", getMacroText(2)); + + Log::Comment(L"Default replacement is a single macro"); + _stateMachine->ProcessString(L"\033P1;0;0!zRetained\033\\"); + _stateMachine->ProcessString(L"\033P2;0;0!zReplaced\033\\"); + _stateMachine->ProcessString(L"\033P2;;0!zNew\033\\"); + VERIFY_ARE_EQUAL(L"Retained", getMacroText(1)); + VERIFY_ARE_EQUAL(L"New", getMacroText(2)); + + Log::Comment(L"Repeating three times"); + _stateMachine->ProcessString(L"\033P5;0;1!z526570656174!3;206563686F;207468726565\033\\"); + VERIFY_ARE_EQUAL(L"Repeat echo echo echo three", getMacroText(5)); + + Log::Comment(L"Zero repeats once"); + _stateMachine->ProcessString(L"\033P6;0;1!z526570656174!0;206563686F;207A65726F\033\\"); + VERIFY_ARE_EQUAL(L"Repeat echo zero", getMacroText(6)); + + Log::Comment(L"Default repeats once"); + _stateMachine->ProcessString(L"\033P7;0;1!z526570656174!;206563686F;2064656661756C74\033\\"); + VERIFY_ARE_EQUAL(L"Repeat echo default", getMacroText(7)); + + Log::Comment(L"Unterminated repeat sequence"); + _stateMachine->ProcessString(L"\033P8;0;1!z556E7465726D696E61746564!3;206563686F\033\\"); + VERIFY_ARE_EQUAL(L"Unterminated echo echo echo", getMacroText(8)); + + Log::Comment(L"Unexpected semicolon cancels definition"); + _stateMachine->ProcessString(L"\033P9;0;0!zReplaced\033\\"); + _stateMachine->ProcessString(L"\033P9;0;1!z526570656174!3;206563;686F;207468726565\033\\"); + VERIFY_ARE_EQUAL(L"", getMacroText(9)); + + Log::Comment(L"Control characters in a text encoding"); + _stateMachine->ProcessString(L"\033P10;0;0!zA\aB\bC\tD\nE\vF\fG\rH\033\\"); + VERIFY_ARE_EQUAL(L"ABCDEFGH", getMacroText(10)); + + Log::Comment(L"Control characters in a hex encoding"); + _stateMachine->ProcessString(L"\033P11;0;1!z41\a42\b43\t44\n45\v46\f47\r48\033\\"); + VERIFY_ARE_EQUAL(L"ABCDEFGH", getMacroText(11)); + + Log::Comment(L"Control characters in a repeat"); + _stateMachine->ProcessString(L"\033P12;0;1!z!\a3\b;\t4\n1\v4\f2\r4\a3\b;\033\\"); + VERIFY_ARE_EQUAL(L"ABCABCABC", getMacroText(12)); + + Log::Comment(L"Encoded control characters"); + _stateMachine->ProcessString(L"\033P13;0;1!z410742084309440A450B460C470D481B49\033\\"); + VERIFY_ARE_EQUAL(L"A\aB\bC\tD\nE\vF\fG\rH\033I", getMacroText(13)); + + _pDispatch->_macroBuffer = nullptr; + } + + TEST_METHOD(MacroInvokes) + { + _pDispatch->_macroBuffer = std::make_shared(); + + const auto setMacroText = [&](const auto id, const auto value) { + _pDispatch->_macroBuffer->_macros.at(id) = value; + }; + + setMacroText(0, L"Macro 0"); + setMacroText(1, L"Macro 1"); + setMacroText(2, L"Macro 2"); + setMacroText(63, L"Macro 63"); + + Log::Comment(L"Simple macro invoke"); + _testGetSet->_printed.clear(); + _stateMachine->ProcessString(L"\033[2*z"); + VERIFY_ARE_EQUAL(L"Macro 2", _testGetSet->_printed); + + Log::Comment(L"Default macro invoke"); + _testGetSet->_printed.clear(); + _stateMachine->ProcessString(L"\033[*z"); + VERIFY_ARE_EQUAL(L"Macro 0", _testGetSet->_printed); + + Log::Comment(L"Maximum ID number"); + _testGetSet->_printed.clear(); + _stateMachine->ProcessString(L"\033[63*z"); + VERIFY_ARE_EQUAL(L"Macro 63", _testGetSet->_printed); + + Log::Comment(L"Out of range ID number"); + _testGetSet->_printed.clear(); + _stateMachine->ProcessString(L"\033[64*z"); + VERIFY_ARE_EQUAL(L"", _testGetSet->_printed); + + Log::Comment(L"Only one ID parameter allowed"); + _testGetSet->_printed.clear(); + _stateMachine->ProcessString(L"\033[2;0;1*z"); + VERIFY_ARE_EQUAL(L"Macro 2", _testGetSet->_printed); + + Log::Comment(L"DECDMAC ignored when inside a macro"); + setMacroText(10, L"[\033P1;0;0!zReplace Macro 1\033\\]"); + _testGetSet->_printed.clear(); + _stateMachine->ProcessString(L"\033[10*z"); + _stateMachine->ProcessString(L"\033[1*z"); + VERIFY_ARE_EQUAL(L"[]Macro 1", _testGetSet->_printed); + + Log::Comment(L"Maximum recursive depth is 16"); + setMacroText(0, L"<\033[1*z>"); + setMacroText(1, L"[\033[0*z]"); + _testGetSet->_printed.clear(); + _stateMachine->ProcessString(L"\033[0*z"); + VERIFY_ARE_EQUAL(L"<[<[<[<[<[<[<[<[]>]>]>]>]>]>]>]>", _testGetSet->_printed); + + _pDispatch->_macroBuffer = nullptr; + } + private: TerminalInput _terminalInput{ nullptr }; std::unique_ptr _testGetSet; From 310172d48731f30dd7d443470cb58b54778ffae8 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 29 Nov 2022 18:21:25 +0000 Subject: [PATCH 09/12] Double check that _activeMacroId is in range. --- src/terminal/adapter/MacroBuffer.cpp | 6 +++--- src/terminal/adapter/MacroBuffer.hpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/terminal/adapter/MacroBuffer.cpp b/src/terminal/adapter/MacroBuffer.cpp index 532cfee8554..1b191913fd9 100644 --- a/src/terminal/adapter/MacroBuffer.cpp +++ b/src/terminal/adapter/MacroBuffer.cpp @@ -74,7 +74,7 @@ void MacroBuffer::ClearMacrosIfInUse() } } -bool MacroBuffer::InitParser(const size_t macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding) noexcept +bool MacroBuffer::InitParser(const size_t macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding) { // We're checking the invoked depth here to make sure we aren't defining // a macro from within a macro invocation. @@ -234,9 +234,9 @@ bool MacroBuffer::_appendToActiveMacro(const wchar_t ch) return false; } -std::wstring& MacroBuffer::_activeMacro() noexcept +std::wstring& MacroBuffer::_activeMacro() { - return til::at(_macros, _activeMacroId); + return _macros.at(_activeMacroId); } void MacroBuffer::_deleteMacro(std::wstring& macro) noexcept diff --git a/src/terminal/adapter/MacroBuffer.hpp b/src/terminal/adapter/MacroBuffer.hpp index 64e0a61ba1e..5f94e0f817e 100644 --- a/src/terminal/adapter/MacroBuffer.hpp +++ b/src/terminal/adapter/MacroBuffer.hpp @@ -44,13 +44,13 @@ namespace Microsoft::Console::VirtualTerminal std::wstring_view GetMacroSequence(const size_t macroId) const noexcept; void InvokeMacroSequence(const std::wstring_view macroSequence, StateMachine& stateMachine); void ClearMacrosIfInUse(); - bool InitParser(const size_t macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding) noexcept; + bool InitParser(const size_t macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding); bool ParseDefinition(const wchar_t ch); private: bool _decodeHexDigit(const wchar_t ch) noexcept; bool _appendToActiveMacro(const wchar_t ch); - std::wstring& _activeMacro() noexcept; + std::wstring& _activeMacro(); void _deleteMacro(std::wstring& macro) noexcept; bool _applyPendingRepeat(); From 6a6ada8ac4c4f21f618bfd71f50f96862742495e Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 29 Nov 2022 19:40:52 +0000 Subject: [PATCH 10/12] Merge GetMacroSequence into InvokeMacroSequence. --- src/terminal/adapter/MacroBuffer.cpp | 43 +++++++++++++------------- src/terminal/adapter/MacroBuffer.hpp | 3 +- src/terminal/adapter/adaptDispatch.cpp | 26 +++++++--------- 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/src/terminal/adapter/MacroBuffer.cpp b/src/terminal/adapter/MacroBuffer.cpp index 1b191913fd9..02caaeb192d 100644 --- a/src/terminal/adapter/MacroBuffer.cpp +++ b/src/terminal/adapter/MacroBuffer.cpp @@ -32,30 +32,29 @@ uint16_t MacroBuffer::CalculateChecksum() const noexcept return checksum; } -std::wstring_view MacroBuffer::GetMacroSequence(const size_t macroId) const noexcept +void MacroBuffer::InvokeMacro(const size_t macroId, StateMachine& stateMachine) { - return macroId < _macros.size() ? til::at(_macros, macroId) : std::wstring_view{}; -} - -void MacroBuffer::InvokeMacroSequence(const std::wstring_view macroSequence, StateMachine& stateMachine) -{ - // Macros can invoke other macros up to a depth of 16, but we don't allow - // the total sequence length to exceed the maximum buffer size, since that's - // likely to facilitate a denial-of-service attack. - const auto allowedLength = MAX_SPACE - _invokedSequenceLength; - if (_invokedDepth < 16 && macroSequence.length() < allowedLength) + if (macroId < _macros.size()) { - _invokedSequenceLength += macroSequence.length(); - _invokedDepth++; - auto resetInvokeDepth = wil::scope_exit([&] { - // Once the invoke depth reaches zero, we know we've reached the end - // of the root invoke, so we can reset the sequence length tracker. - if (--_invokedDepth == 0) - { - _invokedSequenceLength = 0; - } - }); - stateMachine.ProcessString(macroSequence); + const auto& macroSequence = til::at(_macros, macroId); + // Macros can invoke other macros up to a depth of 16, but we don't allow + // the total sequence length to exceed the maximum buffer size, since that's + // likely to facilitate a denial-of-service attack. + const auto allowedLength = MAX_SPACE - _invokedSequenceLength; + if (_invokedDepth < 16 && macroSequence.length() < allowedLength) + { + _invokedSequenceLength += macroSequence.length(); + _invokedDepth++; + auto resetInvokeDepth = wil::scope_exit([&] { + // Once the invoke depth reaches zero, we know we've reached the end + // of the root invoke, so we can reset the sequence length tracker. + if (--_invokedDepth == 0) + { + _invokedSequenceLength = 0; + } + }); + stateMachine.ProcessString(macroSequence); + } } } diff --git a/src/terminal/adapter/MacroBuffer.hpp b/src/terminal/adapter/MacroBuffer.hpp index 5f94e0f817e..92c1c07c73f 100644 --- a/src/terminal/adapter/MacroBuffer.hpp +++ b/src/terminal/adapter/MacroBuffer.hpp @@ -41,8 +41,7 @@ namespace Microsoft::Console::VirtualTerminal size_t GetSpaceAvailable() const noexcept; uint16_t CalculateChecksum() const noexcept; - std::wstring_view GetMacroSequence(const size_t macroId) const noexcept; - void InvokeMacroSequence(const std::wstring_view macroSequence, StateMachine& stateMachine); + void InvokeMacro(const size_t macroId, StateMachine& stateMachine); void ClearMacrosIfInUse(); bool InitParser(const size_t macroId, const DispatchTypes::MacroDeleteControl deleteControl, const DispatchTypes::MacroEncoding encoding); bool ParseDefinition(const wchar_t ch); diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 44acffe3040..3f3bd72edfd 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3258,21 +3258,17 @@ bool AdaptDispatch::InvokeMacro(const VTInt macroId) { if (_macroBuffer) { - const auto macroSequence = _macroBuffer->GetMacroSequence(macroId); - if (!macroSequence.empty()) - { - // In order to inject our macro sequence into the state machine - // we need to register a callback that will be executed only - // once it has finished processing the current operation, and - // has returned to the ground state. Note that we're capturing - // a copy of the _macroBuffer pointer here to make sure it won't - // be deleted (e.g. from an invoked RIS) while still in use. - const auto macroBuffer = _macroBuffer; - auto& stateMachine = _api.GetStateMachine(); - stateMachine.OnCsiComplete([=, &stateMachine]() { - macroBuffer->InvokeMacroSequence(macroSequence, stateMachine); - }); - } + // In order to inject our macro sequence into the state machine + // we need to register a callback that will be executed only + // once it has finished processing the current operation, and + // has returned to the ground state. Note that we're capturing + // a copy of the _macroBuffer pointer here to make sure it won't + // be deleted (e.g. from an invoked RIS) while still in use. + const auto macroBuffer = _macroBuffer; + auto& stateMachine = _api.GetStateMachine(); + stateMachine.OnCsiComplete([=, &stateMachine]() { + macroBuffer->InvokeMacro(macroId, stateMachine); + }); } return true; } From 9d38c6138a774156677648d16868d9a538334489 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Tue, 29 Nov 2022 19:48:38 +0000 Subject: [PATCH 11/12] Drop unnecessary sequenceLength test. --- src/terminal/adapter/MacroBuffer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/terminal/adapter/MacroBuffer.cpp b/src/terminal/adapter/MacroBuffer.cpp index 02caaeb192d..7ae1f05e5b7 100644 --- a/src/terminal/adapter/MacroBuffer.cpp +++ b/src/terminal/adapter/MacroBuffer.cpp @@ -246,10 +246,10 @@ void MacroBuffer::_deleteMacro(std::wstring& macro) noexcept bool MacroBuffer::_applyPendingRepeat() { - auto& activeMacro = _activeMacro(); - const auto sequenceLength = activeMacro.length() - _repeatStart; - if (sequenceLength && _repeatCount > 1) + if (_repeatCount > 1) { + auto& activeMacro = _activeMacro(); + const auto sequenceLength = activeMacro.length() - _repeatStart; // Note that the repeat sequence has already been written to the buffer // once while it was being parsed, so we only need to append additional // copies for repeat counts that are greater than one. If there is not @@ -259,7 +259,7 @@ bool MacroBuffer::_applyPendingRepeat() { return false; } - for (auto i = 1u; i < _repeatCount; i++) + for (size_t i = 1; i < _repeatCount; i++) { activeMacro.append(activeMacro.substr(_repeatStart, sequenceLength)); _spaceUsed += sequenceLength; From 5517ac387e590e8b34f8f21251b4f6c56a9e55c8 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Wed, 30 Nov 2022 00:06:33 +0000 Subject: [PATCH 12/12] Give spelling-bot much happy. --- .github/actions/spelling/expect/expect.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index 8f8f4828d0a..909d4df0481 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -410,20 +410,24 @@ DECAWM DECBKM DECCARA DECCKM +DECCKSR DECCOLM DECCRA DECCTR DECDHL decdld +DECDMAC DECDWL DECEKBD DECERA DECFRA DECID +DECINVM DECKPAM DECKPM DECKPNM DECLRMM +DECMSR DECNKM DECNRCM DECOM @@ -2288,6 +2292,7 @@ YOffset YSubstantial YVIRTUALSCREEN YWalk +zabcd Zabcdefghijklmnopqrstuvwxyz ZCmd ZCtrl