Skip to content

Commit

Permalink
Only passthrough input changes if the client's in VT input mode (#4913)
Browse files Browse the repository at this point in the history 
Closes #4911.
  • Loading branch information
@zadjii-msft
zadjii-msft authored Mar 13, 2020
1 parent 5a1b7b6 commit 57a80aa
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 65 deletions.
9 changes: 6 additions & 3 deletions src/host/outputStream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -769,14 +769,17 @@ bool ConhostInternalGetSet::SetCursorColor(const COLORREF cursorColor)

// Routine Description:
// - Connects the IsConsolePty call directly into our Driver Message servicing call inside Conhost.exe
// - NOTE: This ONE method behaves differently! The rest of the methods on this
// interface return true if successful. This one just returns the result.
// Arguments:
// - isPty: receives the bool indicating whether or not we're in pty mode.
// Return Value:
// - true if successful (see DoSrvIsConsolePty). false otherwise.
bool ConhostInternalGetSet::IsConsolePty(bool& isPty) const
// - true if we're in pty mode.
bool ConhostInternalGetSet::IsConsolePty() const
{
bool isPty = false;
DoSrvIsConsolePty(isPty);
return true;
return isPty;
}

bool ConhostInternalGetSet::DeleteLines(const size_t count)
Expand Down
2 changes: 1 addition & 1 deletion src/host/outputStream.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::

bool GetConsoleOutputCP(unsigned int& codepage) override;

bool IsConsolePty(bool& isPty) const override;
bool IsConsolePty() const override;

bool DeleteLines(const size_t count) override;
bool InsertLines(const size_t count) override;
Expand Down
99 changes: 47 additions & 52 deletions src/terminal/adapter/adaptDispatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,8 +583,7 @@ bool AdaptDispatch::EraseInDisplay(const DispatchTypes::EraseType eraseType)
// make the state machine propogate this ED sequence to the connected
// terminal application. While we're in conpty mode, we don't really
// have a scrollback, but the attached terminal might.
bool isPty = false;
_pConApi->IsConsolePty(isPty);
const bool isPty = _pConApi->IsConsolePty();
return eraseScrollbackResult && (!isPty);
}
else if (eraseType == DispatchTypes::EraseType::All)
Expand Down Expand Up @@ -1042,10 +1041,11 @@ bool AdaptDispatch::SetKeypadMode(const bool fApplicationMode)
bool success = true;
success = _pConApi->PrivateSetKeypadMode(fApplicationMode);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand All @@ -1063,10 +1063,11 @@ bool AdaptDispatch::SetCursorKeysMode(const bool applicationMode)
bool success = true;
success = _pConApi->PrivateSetCursorKeysMode(applicationMode);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand Down Expand Up @@ -1558,9 +1559,7 @@ bool AdaptDispatch::HardReset()
// make the state machine propogate this RIS sequence to the connected
// terminal application. We've reset our state, but the connected terminal
// might need to do more.
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
if (_pConApi->IsConsolePty())
{
return false;
}
Expand Down Expand Up @@ -1713,10 +1712,11 @@ bool AdaptDispatch::EnableVT200MouseMode(const bool enabled)
bool success = true;
success = _pConApi->PrivateEnableVT200MouseMode(enabled);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand All @@ -1736,10 +1736,11 @@ bool AdaptDispatch::EnableUTF8ExtendedMouseMode(const bool enabled)
bool success = true;
success = _pConApi->PrivateEnableUTF8ExtendedMouseMode(enabled);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand All @@ -1759,10 +1760,11 @@ bool AdaptDispatch::EnableSGRExtendedMouseMode(const bool enabled)
bool success = true;
success = _pConApi->PrivateEnableSGRExtendedMouseMode(enabled);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand All @@ -1781,10 +1783,11 @@ bool AdaptDispatch::EnableButtonEventMouseMode(const bool enabled)
bool success = true;
success = _pConApi->PrivateEnableButtonEventMouseMode(enabled);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand All @@ -1804,10 +1807,11 @@ bool AdaptDispatch::EnableAnyEventMouseMode(const bool enabled)
bool success = true;
success = _pConApi->PrivateEnableAnyEventMouseMode(enabled);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand All @@ -1827,10 +1831,11 @@ bool AdaptDispatch::EnableAlternateScroll(const bool enabled)
bool success = true;
success = _pConApi->PrivateEnableAlternateScroll(enabled);

// If we're a conpty, always return false
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
// If we're a conpty, AND WE'RE IN VT INPUT MODE, always return false
// The VT Input mode check is to work around ssh.exe v7.7, which uses VT
// output, but not Input. Once the conpty supports these types of input,
// this check can be removed. See GH#4911
if (_pConApi->IsConsolePty() && _pConApi->PrivateIsVtInputEnabled())
{
return false;
}
Expand Down Expand Up @@ -1889,9 +1894,7 @@ bool AdaptDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)

// If we're a conpty, always return false, so that this cursor state will be
// sent to the connected terminal
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
if (_pConApi->IsConsolePty())
{
return false;
}
Expand All @@ -1908,9 +1911,7 @@ bool AdaptDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)
// True if handled successfully. False otherwise.
bool AdaptDispatch::SetCursorColor(const COLORREF cursorColor)
{
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
if (_pConApi->IsConsolePty())
{
return false;
}
Expand Down Expand Up @@ -1938,9 +1939,7 @@ bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwCo
// value to the terminal. Still handle the sequence so apps that use
// the API or VT to query the values of the color table still read the
// correct color.
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
if (_pConApi->IsConsolePty())
{
return false;
}
Expand All @@ -1963,9 +1962,7 @@ bool Microsoft::Console::VirtualTerminal::AdaptDispatch::SetDefaultForeground(co
// value to the terminal. Still handle the sequence so apps that use
// the API or VT to query the values of the color table still read the
// correct color.
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
if (_pConApi->IsConsolePty())
{
return false;
}
Expand All @@ -1988,9 +1985,7 @@ bool Microsoft::Console::VirtualTerminal::AdaptDispatch::SetDefaultBackground(co
// value to the terminal. Still handle the sequence so apps that use
// the API or VT to query the values of the color table still read the
// correct color.
bool isPty = false;
_pConApi->IsConsolePty(isPty);
if (isPty)
if (_pConApi->IsConsolePty())
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/conGetSet.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ namespace Microsoft::Console::VirtualTerminal
virtual bool GetConsoleOutputCP(unsigned int& codepage) = 0;

virtual bool PrivateSuppressResizeRepaint() = 0;
virtual bool IsConsolePty(bool& isPty) const = 0;
virtual bool IsConsolePty() const = 0;

virtual bool DeleteLines(const size_t count) = 0;
virtual bool InsertLines(const size_t count) = 0;
Expand Down
10 changes: 2 additions & 8 deletions src/terminal/adapter/ut_adapter/adapterTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -610,14 +610,10 @@ class TestGetSet final : public ConGetSet
return _getConsoleOutputCPResult;
}

bool IsConsolePty(bool& isPty) const override
bool IsConsolePty() const override
{
Log::Comment(L"IsConsolePty MOCK called...");
if (_isConsolePtyResult)
{
isPty = _isPty;
}
return _isConsolePtyResult;
return _isPty;
}

bool DeleteLines(const size_t /*count*/) override
Expand Down Expand Up @@ -951,7 +947,6 @@ class TestGetSet final : public ConGetSet
bool _setCursorColorResult = false;
COLORREF _expectedCursorColor = 0;
bool _getConsoleOutputCPResult = false;
bool _isConsolePtyResult = false;
bool _privateSetDefaultAttributesResult = false;
bool _moveToBottomResult = false;

Expand Down Expand Up @@ -2270,7 +2265,6 @@ class AdapterTest

// Test in pty mode - we should fail, but PrivateSetColorTableEntry should still be called
_testGetSet->_isPty = true;
_testGetSet->_isConsolePtyResult = true;

_testGetSet->_expectedColorTableIndex = 15; // Windows BRIGHT_WHITE
VERIFY_IS_FALSE(_pDispatch.get()->SetColorTableEntry(15, testColor));
Expand Down

0 comments on commit 57a80aa

Please sign in to comment.