Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Disable the acceptance of C1 control codes by default #11690

Merged
11 commits merged into from
Nov 17, 2021
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ dealloc
Debian
debolden
debugtype
DECAC
DECALN
DECANM
DECAUPSS
Expand Down
8 changes: 8 additions & 0 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ Terminal::Terminal() :

_stateMachine = std::make_unique<StateMachine>(std::move(engine));

// Until we have a true pass-through mode (GH#1173), the decision as to
// whether C1 controls are interpreted or not is made at the conhost level.
// If they are being filtered out, then we will simply never receive them.
// But if they are being accepted by conhost, there's a chance they may get
// passed through in some situations, so it's important that our state
// machine is always prepared to accept them.
_stateMachine->SetParserMode(StateMachine::Mode::AcceptC1, true);

auto passAlongInput = [&](std::deque<std::unique_ptr<IInputEvent>>& inEventsToWrite) {
if (!_pfnWriteInput)
{
Expand Down
58 changes: 29 additions & 29 deletions src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ void TerminalCoreUnitTests::TerminalApiTest::AddHyperlink()
auto& stateMachine = *(term._stateMachine);

// Process the opening osc 8 sequence
stateMachine.ProcessString(L"\x1b]8;;test.url\x9c");
stateMachine.ProcessString(L"\x1b]8;;test.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");

Expand All @@ -281,7 +281,7 @@ void TerminalCoreUnitTests::TerminalApiTest::AddHyperlink()
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");

// Process the closing osc 8 sequences
stateMachine.ProcessString(L"\x1b]8;;\x9c");
stateMachine.ProcessString(L"\x1b]8;;\x1b\\");
VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink());
}

Expand All @@ -297,7 +297,7 @@ void TerminalCoreUnitTests::TerminalApiTest::AddHyperlinkCustomId()
auto& stateMachine = *(term._stateMachine);

// Process the opening osc 8 sequence
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
Expand All @@ -309,7 +309,7 @@ void TerminalCoreUnitTests::TerminalApiTest::AddHyperlinkCustomId()
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());

// Process the closing osc 8 sequences
stateMachine.ProcessString(L"\x1b]8;;\x9c");
stateMachine.ProcessString(L"\x1b]8;;\x1b\\");
VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink());
}

Expand All @@ -325,15 +325,15 @@ void TerminalCoreUnitTests::TerminalApiTest::AddHyperlinkCustomIdDifferentUri()
auto& stateMachine = *(term._stateMachine);

// Process the opening osc 8 sequence
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());

const auto oldAttributes{ tbi.GetCurrentAttributes() };

// Send any other text
stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x9c");
stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"other.url");
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"other.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
Expand All @@ -356,58 +356,58 @@ void TerminalCoreUnitTests::TerminalApiTest::SetTaskbarProgress()
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(0));

// Set some values for taskbar state and progress through state machine
stateMachine.ProcessString(L"\x1b]9;4;1;50\x9c");
stateMachine.ProcessString(L"\x1b]9;4;1;50\x1b\\");
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(1));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(50));

// Reset to 0
stateMachine.ProcessString(L"\x1b]9;4;0;0\x9c");
stateMachine.ProcessString(L"\x1b]9;4;0;0\x1b\\");
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(0));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(0));

// Set an out of bounds value for state
stateMachine.ProcessString(L"\x1b]9;4;5;50\x9c");
stateMachine.ProcessString(L"\x1b]9;4;5;50\x1b\\");
// Nothing should have changed (dispatch should have returned false)
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(0));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(0));

// Set an out of bounds value for progress
stateMachine.ProcessString(L"\x1b]9;4;1;999\x9c");
stateMachine.ProcessString(L"\x1b]9;4;1;999\x1b\\");
// Progress should have been clamped to 100
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(1));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(100));

// Don't specify any params
stateMachine.ProcessString(L"\x1b]9;4\x9c");
stateMachine.ProcessString(L"\x1b]9;4\x1b\\");
// State and progress should both be reset to 0
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(0));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(0));

// Specify additional params
stateMachine.ProcessString(L"\x1b]9;4;1;80;123\x9c");
stateMachine.ProcessString(L"\x1b]9;4;1;80;123\x1b\\");
// Additional params should be ignored, state and progress still set normally
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(1));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(80));

// Edge cases + trailing semicolon testing
stateMachine.ProcessString(L"\x1b]9;4;2;\x9c");
stateMachine.ProcessString(L"\x1b]9;4;2;\x1b\\");
// String should be processed correctly despite the trailing semicolon,
// taskbar progress should remain unchanged from previous value
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(2));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(80));

stateMachine.ProcessString(L"\x1b]9;4;3;75\x9c");
stateMachine.ProcessString(L"\x1b]9;4;3;75\x1b\\");
// Given progress value should be ignored because this is the indeterminate state,
// so the progress value should remain unchanged
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(3));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(80));

stateMachine.ProcessString(L"\x1b]9;4;0;50\x9c");
stateMachine.ProcessString(L"\x1b]9;4;0;50\x1b\\");
// Taskbar progress should be 0 (the given value should be ignored)
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(0));
VERIFY_ARE_EQUAL(term.GetTaskbarProgress(), gsl::narrow<size_t>(0));

stateMachine.ProcessString(L"\x1b]9;4;2;\x9c");
stateMachine.ProcessString(L"\x1b]9;4;2;\x1b\\");
// String should be processed correctly despite the trailing semicolon,
// taskbar progress should be set to a 'minimum', non-zero value
VERIFY_ARE_EQUAL(term.GetTaskbarState(), gsl::narrow<size_t>(2));
Expand All @@ -427,45 +427,45 @@ void TerminalCoreUnitTests::TerminalApiTest::SetWorkingDirectory()
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());

// Invalid sequences should not change CWD
stateMachine.ProcessString(L"\x1b]9;9\x9c");
stateMachine.ProcessString(L"\x1b]9;9\x1b\\");
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());

stateMachine.ProcessString(L"\x1b]9;9\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9\"\x1b\\");
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());

stateMachine.ProcessString(L"\x1b]9;9\"C:\\\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9\"C:\\\"\x1b\\");
VERIFY_IS_TRUE(term.GetWorkingDirectory().empty());

// Valid sequences should change CWD
stateMachine.ProcessString(L"\x1b]9;9;\"C:\\\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9;\"C:\\\"\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"C:\\");

stateMachine.ProcessString(L"\x1b]9;9;\"C:\\Program Files\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9;\"C:\\Program Files\"\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"C:\\Program Files");

stateMachine.ProcessString(L"\x1b]9;9;\"D:\\中文\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9;\"D:\\中文\"\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"D:\\中文");

// Test OSC 9;9 sequences without quotation marks
stateMachine.ProcessString(L"\x1b]9;9;C:\\\x9c");
stateMachine.ProcessString(L"\x1b]9;9;C:\\\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"C:\\");

stateMachine.ProcessString(L"\x1b]9;9;C:\\Program Files\x9c");
stateMachine.ProcessString(L"\x1b]9;9;C:\\Program Files\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"C:\\Program Files");

stateMachine.ProcessString(L"\x1b]9;9;D:\\中文\x9c");
stateMachine.ProcessString(L"\x1b]9;9;D:\\中文\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"D:\\中文");

// These OSC 9;9 sequences will result in invalid CWD. We shouldn't crash on these.
stateMachine.ProcessString(L"\x1b]9;9;\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9;\"\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"");

stateMachine.ProcessString(L"\x1b]9;9;\"\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9;\"\"\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"\"");

stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"");

stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\"\x9c");
stateMachine.ProcessString(L"\x1b]9;9;\"\"\"\"\x1b\\");
VERIFY_ARE_EQUAL(term.GetWorkingDirectory(), L"\"\"");
}
28 changes: 21 additions & 7 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

using namespace Microsoft::Console;
using Microsoft::Console::Interactivity::ServiceLocator;
using Microsoft::Console::VirtualTerminal::StateMachine;
using Microsoft::Console::VirtualTerminal::TerminalInput;

WriteBuffer::WriteBuffer(_In_ Microsoft::Console::IIoProvider& io) :
Expand Down Expand Up @@ -263,22 +264,35 @@ bool ConhostInternalGetSet::SetInputMode(const TerminalInput::Mode mode, const b
}

// Routine Description:
// - Sets the terminal emulation mode to either ANSI-compatible or VT52.
// PrivateSetAnsiMode is an internal-only "API" call that the vt commands can execute,
// - Sets the various StateMachine parser modes.
// SetParserMode is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on out public API surface.
// Arguments:
// - ansiMode - set to true to enable the ANSI mode, false for VT52 mode.
// - mode - the parser mode to change.
// - enabled - set to true to enable the mode, false to disable it.
// Return Value:
// - true if successful. false otherwise.
bool ConhostInternalGetSet::PrivateSetAnsiMode(const bool ansiMode)
bool ConhostInternalGetSet::SetParserMode(const StateMachine::Mode mode, const bool enabled)
{
auto& stateMachine = _io.GetActiveOutputBuffer().GetStateMachine();
stateMachine.SetAnsiMode(ansiMode);
auto& terminalInput = _io.GetActiveInputBuffer()->GetTerminalInput();
terminalInput.SetInputMode(TerminalInput::Mode::Ansi, ansiMode);
stateMachine.SetParserMode(mode, enabled);
return true;
}

// Routine Description:
// - Retrieves the various StateMachine parser modes.
// GetParserMode is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on out public API surface.
// Arguments:
// - mode - the parser mode to query.
// Return Value:
// - true if the mode is enabled. false if disabled.
bool ConhostInternalGetSet::GetParserMode(const Microsoft::Console::VirtualTerminal::StateMachine::Mode mode) const
{
auto& stateMachine = _io.GetActiveOutputBuffer().GetStateMachine();
return stateMachine.GetParserMode(mode);
}

// Routine Description:
// - Connects the PrivateSetScreenMode call directly into our Driver Message servicing call inside Conhost.exe
// PrivateSetScreenMode is an internal-only "API" call that the vt commands can execute,
Expand Down
3 changes: 2 additions & 1 deletion src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
const SMALL_RECT& window) override;

bool SetInputMode(const Microsoft::Console::VirtualTerminal::TerminalInput::Mode mode, const bool enabled) override;
bool SetParserMode(const Microsoft::Console::VirtualTerminal::StateMachine::Mode mode, const bool enabled) override;
bool GetParserMode(const Microsoft::Console::VirtualTerminal::StateMachine::Mode mode) const override;

bool PrivateSetAnsiMode(const bool ansiMode) override;
bool PrivateSetScreenMode(const bool reverseMode) override;
bool PrivateSetAutoWrapMode(const bool wrapAtEOL) override;

Expand Down
12 changes: 6 additions & 6 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5978,7 +5978,7 @@ void ScreenBufferTests::TestAddHyperlink()
auto& stateMachine = si.GetStateMachine();

// Process the opening osc 8 sequence with no custom id
stateMachine.ProcessString(L"\x1b]8;;test.url\x9c");
stateMachine.ProcessString(L"\x1b]8;;test.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");

Expand All @@ -5988,7 +5988,7 @@ void ScreenBufferTests::TestAddHyperlink()
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");

// Process the closing osc 8 sequences
stateMachine.ProcessString(L"\x1b]8;;\x9c");
stateMachine.ProcessString(L"\x1b]8;;\x1b\\");
VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink());
}

Expand All @@ -6001,7 +6001,7 @@ void ScreenBufferTests::TestAddHyperlinkCustomId()
auto& stateMachine = si.GetStateMachine();

// Process the opening osc 8 sequence with a custom id
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
Expand All @@ -6013,7 +6013,7 @@ void ScreenBufferTests::TestAddHyperlinkCustomId()
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());

// Process the closing osc 8 sequences
stateMachine.ProcessString(L"\x1b]8;;\x9c");
stateMachine.ProcessString(L"\x1b]8;;\x1b\\");
VERIFY_IS_FALSE(tbi.GetCurrentAttributes().IsHyperlink());
}

Expand All @@ -6026,15 +6026,15 @@ void ScreenBufferTests::TestAddHyperlinkCustomIdDifferentUri()
auto& stateMachine = si.GetStateMachine();

// Process the opening osc 8 sequence with a custom id
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x9c");
stateMachine.ProcessString(L"\x1b]8;id=myId;test.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"test.url");
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"test.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());

const auto oldAttributes{ tbi.GetCurrentAttributes() };

// Send any other text
stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x9c");
stateMachine.ProcessString(L"\x1b]8;id=myId;other.url\x1b\\");
VERIFY_IS_TRUE(tbi.GetCurrentAttributes().IsHyperlink());
VERIFY_ARE_EQUAL(tbi.GetHyperlinkUriFromId(tbi.GetCurrentAttributes().GetHyperlinkId()), L"other.url");
VERIFY_ARE_EQUAL(tbi.GetHyperlinkId(L"other.url", L"myId"), tbi.GetCurrentAttributes().GetHyperlinkId());
Expand Down
1 change: 1 addition & 0 deletions src/terminal/adapter/ITermDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
virtual bool LockingShift(const size_t gsetNumber) = 0; // LS0, LS1, LS2, LS3
virtual bool LockingShiftRight(const size_t gsetNumber) = 0; // LS1R, LS2R, LS3R
virtual bool SingleShift(const size_t gsetNumber) = 0; // SS2, SS3
virtual bool AcceptC1Controls(const bool enabled) = 0; // DECAC1

virtual bool SoftReset() = 0; // DECSTR
virtual bool HardReset() = 0; // RIS
Expand Down
Loading