Skip to content

Commit

Permalink
UIA: throw E_FAIL for out-of-bounds text (#8052)
Browse files Browse the repository at this point in the history
In nvaccess/nvda#11428 (comment),
Andre9642 reported a Conhost crash when switching to/from the alt buffer
a few times with a Braille display connected. Upon further
investigation, @carlos-zamora and I discovered that the FailFast was in
`GetText`: more checks similar to #7677 were needed for this case.

Tested with NVDA using a [Focus](https://www.freedomscientific.com/products/blindness/focus40brailledisplay/) Braille display.

Improves nvaccess/nvda#11428
  • Loading branch information
codeofdusk authored Oct 27, 2020
1 parent fc9a46d commit 60437b8
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
2 changes: 2 additions & 0 deletions .github/actions/spell-check/dictionary/names.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ mikemaccana
miloush
miniksa
niksa
nvaccess
nvda
oising
oldnewthing
osgwiki
Expand Down
16 changes: 9 additions & 7 deletions src/types/UiaTextRangeBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,7 @@ CATCH_RETURN();
// - the text that the UiaTextRange encompasses
#pragma warning(push)
#pragma warning(disable : 26447) // compiler isn't filtering throws inside the try/catch
std::wstring UiaTextRangeBase::_getTextValue(std::optional<unsigned int> maxLength) const noexcept
try
std::wstring UiaTextRangeBase::_getTextValue(std::optional<unsigned int> maxLength) const
{
_pData->LockConsole();
auto Unlock = wil::scope_exit([&]() noexcept {
Expand All @@ -540,6 +539,14 @@ try
const auto& buffer = _pData->GetTextBuffer();
const auto bufferSize = buffer.GetSize();

// TODO GH#5406: create a different UIA parent object for each TextBuffer
// nvaccess/nvda#11428: Ensure our endpoints are in bounds
// otherwise, we'll FailFast catastrophically
if (!bufferSize.IsInBounds(_start, true) || !bufferSize.IsInBounds(_end, true))
{
THROW_HR(E_FAIL);
}

// convert _end to be inclusive
auto inclusiveEnd = _end;
bufferSize.DecrementInBounds(inclusiveEnd, true);
Expand All @@ -564,11 +571,6 @@ try

return textData;
}
catch (...)
{
LOG_CAUGHT_EXCEPTION();
return {};
}
#pragma warning(pop)

IFACEMETHODIMP UiaTextRangeBase::Move(_In_ TextUnit unit,
Expand Down
2 changes: 1 addition & 1 deletion src/types/UiaTextRangeBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ namespace Microsoft::Console::Types
// This is used by tracing to extract the text value
// that the UiaTextRange currently encompasses.
// GetText() cannot be used as it's not const
std::wstring _getTextValue(std::optional<unsigned int> maxLength = std::nullopt) const noexcept;
std::wstring _getTextValue(std::optional<unsigned int> maxLength = std::nullopt) const;

RECT _getTerminalRect() const;

Expand Down

0 comments on commit 60437b8

Please sign in to comment.