Skip to content

Commit

Permalink
[WPF] Add TermCore null checks to HwndTerminal (#14678)
Browse files Browse the repository at this point in the history 
#14461 is caused by a null pointer exception in `TerminalIsSelectionActive()`. Since this reveals that it is possible to enter a state where `_terminal` is null, I've gone ahead and added null-checks throughout the code to make it more stable.

Closes #14461

## Validation Steps Performed
Ran locally. Still works.
  • Loading branch information
@carlos-zamora
carlos-zamora authored Jan 13, 2023
1 parent 239b4d1 commit dc6d82e
Showing 1 changed file with 68 additions and 10 deletions.
78 changes: 68 additions & 10 deletions src/cascadia/PublicTerminalCore/HwndTerminal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@ try
}
break;
case WM_RBUTTONDOWN:
if (terminal->_terminal->IsSelectionActive())
if (const auto& termCore{ terminal->_terminal }; termCore && termCore->IsSelectionActive())
{
try
{
const auto bufferData = terminal->_terminal->RetrieveSelectedTextFromBuffer(false);
const auto bufferData = termCore->RetrieveSelectedTextFromBuffer(false);
LOG_IF_FAILED(terminal->_CopyTextToSystemClipboard(bufferData, true));
TerminalClearSelection(terminal);
}
Expand Down Expand Up @@ -260,6 +260,10 @@ CATCH_LOG();

void HwndTerminal::RegisterScrollCallback(std::function<void(int, int, int)> callback)
{
if (!_terminal)
{
return;
}
_terminal->SetScrollPositionChangedCallback(callback);
}

Expand Down Expand Up @@ -295,6 +299,10 @@ HWND HwndTerminal::GetHwnd() const noexcept

void HwndTerminal::_UpdateFont(int newDpi)
{
if (!_terminal)
{
return;
}
_currentDpi = newDpi;
auto lock = _terminal->LockForWriting();

Expand All @@ -311,6 +319,10 @@ IRawElementProviderSimple* HwndTerminal::_GetUiaProvider() noexcept
{
try
{
if (!_terminal)
{
return nullptr;
}
auto lock = _terminal->LockForWriting();
LOG_IF_FAILED(::Microsoft::WRL::MakeAndInitialize<HwndTerminalAutomationPeer>(&_uiaProvider, this->GetRenderData(), this));
_uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(_uiaProvider.Get());
Expand All @@ -329,6 +341,7 @@ IRawElementProviderSimple* HwndTerminal::_GetUiaProvider() noexcept

HRESULT HwndTerminal::Refresh(const til::size windowSize, _Out_ til::size* dimensions)
{
RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
RETURN_HR_IF_NULL(E_INVALIDARG, dimensions);

auto lock = _terminal->LockForWriting();
Expand Down Expand Up @@ -364,6 +377,10 @@ HRESULT HwndTerminal::Refresh(const til::size windowSize, _Out_ til::size* dimen

void HwndTerminal::SendOutput(std::wstring_view data)
{
if (!_terminal)
{
return;
}
_terminal->Write(data);
}

Expand Down Expand Up @@ -474,8 +491,10 @@ void _stdcall TerminalDpiChanged(void* terminal, int newDpi)

void _stdcall TerminalUserScroll(void* terminal, int viewTop)
{
const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
publicTerminal->_terminal->UserScrollViewport(viewTop);
if (const auto publicTerminal = static_cast<const HwndTerminal*>(terminal); publicTerminal && publicTerminal->_terminal)
{
publicTerminal->_terminal->UserScrollViewport(viewTop);
}
}

const unsigned int HwndTerminal::_NumberOfClicks(til::point point, std::chrono::steady_clock::time_point timestamp) noexcept
Expand All @@ -497,6 +516,7 @@ const unsigned int HwndTerminal::_NumberOfClicks(til::point point, std::chrono::
HRESULT HwndTerminal::_StartSelection(LPARAM lParam) noexcept
try
{
RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
const til::point cursorPosition{
GET_X_LPARAM(lParam),
GET_Y_LPARAM(lParam),
Expand Down Expand Up @@ -540,6 +560,7 @@ CATCH_RETURN();
HRESULT HwndTerminal::_MoveSelection(LPARAM lParam) noexcept
try
{
RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
const til::point cursorPosition{
GET_X_LPARAM(lParam),
GET_Y_LPARAM(lParam),
Expand Down Expand Up @@ -578,6 +599,10 @@ CATCH_RETURN();
void HwndTerminal::_ClearSelection() noexcept
try
{
if (!_terminal)
{
return;
}
auto lock{ _terminal->LockForWriting() };
_terminal->ClearSelection();
_renderer->TriggerSelection();
Expand All @@ -592,15 +617,21 @@ void _stdcall TerminalClearSelection(void* terminal)

bool _stdcall TerminalIsSelectionActive(void* terminal)
{
const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
const auto selectionActive = publicTerminal->_terminal->IsSelectionActive();
return selectionActive;
if (const auto publicTerminal = static_cast<const HwndTerminal*>(terminal); publicTerminal && publicTerminal->_terminal)
{
return publicTerminal->_terminal->IsSelectionActive();
}
return false;
}

// Returns the selected text in the terminal.
const wchar_t* _stdcall TerminalGetSelection(void* terminal)
{
auto publicTerminal = static_cast<HwndTerminal*>(terminal);
if (!publicTerminal || !publicTerminal->_terminal)
{
return nullptr;
}

const auto bufferData = publicTerminal->_terminal->RetrieveSelectedTextFromBuffer(false);
publicTerminal->_ClearSelection();
Expand Down Expand Up @@ -652,12 +683,17 @@ bool HwndTerminal::_CanSendVTMouseInput() const noexcept
{
// Only allow the transit of mouse events if shift isn't pressed.
const auto shiftPressed = GetKeyState(VK_SHIFT) < 0;
return !shiftPressed && _focused && _terminal->IsTrackingMouseInput();
return !shiftPressed && _focused && _terminal && _terminal->IsTrackingMouseInput();
}

bool HwndTerminal::_SendMouseEvent(UINT uMsg, WPARAM wParam, LPARAM lParam) noexcept
try
{
if (!_terminal)
{
return false;
}

til::point cursorPosition{
GET_X_LPARAM(lParam),
GET_Y_LPARAM(lParam),
Expand Down Expand Up @@ -690,6 +726,11 @@ catch (...)
void HwndTerminal::_SendKeyEvent(WORD vkey, WORD scanCode, WORD flags, bool keyDown) noexcept
try
{
if (!_terminal)
{
return;
}

auto modifiers = getControlKeyState();
if (WI_IsFlagSet(flags, ENHANCED_KEY))
{
Expand All @@ -706,7 +747,11 @@ CATCH_LOG();
void HwndTerminal::_SendCharEvent(wchar_t ch, WORD scanCode, WORD flags) noexcept
try
{
if (_terminal->IsSelectionActive())
if (!_terminal)
{
return;
}
else if (_terminal->IsSelectionActive())
{
_ClearSelection();
if (ch == UNICODE_ESC)
Expand Down Expand Up @@ -754,6 +799,10 @@ void _stdcall DestroyTerminal(void* terminal)
void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR fontFamily, til::CoordType fontSize, int newDpi)
{
const auto publicTerminal = static_cast<HwndTerminal*>(terminal);
if (!publicTerminal || !publicTerminal->_terminal)
{
return;
}
{
auto lock = publicTerminal->_terminal->LockForWriting();

Expand Down Expand Up @@ -789,7 +838,7 @@ void _stdcall TerminalSetTheme(void* terminal, TerminalTheme theme, LPCWSTR font
void _stdcall TerminalBlinkCursor(void* terminal)
{
const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
if (!publicTerminal->_terminal->IsCursorBlinkingAllowed() && publicTerminal->_terminal->IsCursorVisible())
if (!publicTerminal || !publicTerminal->_terminal || (!publicTerminal->_terminal->IsCursorBlinkingAllowed() && publicTerminal->_terminal->IsCursorVisible()))
{
return;
}
Expand All @@ -800,6 +849,10 @@ void _stdcall TerminalBlinkCursor(void* terminal)
void _stdcall TerminalSetCursorVisible(void* terminal, const bool visible)
{
const auto publicTerminal = static_cast<const HwndTerminal*>(terminal);
if (!publicTerminal || !publicTerminal->_terminal)
{
return;
}
publicTerminal->_terminal->SetCursorOn(visible);
}

Expand Down Expand Up @@ -831,6 +884,7 @@ void __stdcall TerminalKillFocus(void* terminal)
HRESULT HwndTerminal::_CopyTextToSystemClipboard(const TextBuffer::TextAndColor& rows, const bool fAlsoCopyFormatting)
try
{
RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _terminal);
std::wstring finalString;

// Concatenate strings into one giant string to put onto the clipboard.
Expand Down Expand Up @@ -987,6 +1041,10 @@ double HwndTerminal::GetScaleFactor() const noexcept

void HwndTerminal::ChangeViewport(const til::inclusive_rect& NewWindow)
{
if (!_terminal)
{
return;
}
_terminal->UserScrollViewport(NewWindow.top);
}

Expand Down

0 comments on commit dc6d82e

Please sign in to comment.