Skip to content

Commit

Permalink
Correct horizontal coordinates in viewport overflow test (#8456)
Browse files Browse the repository at this point in the history
When resizing the buffer in the `SetConsoleScreenBufferSize` and
`SetConsoleScreenBufferInfoEx` APIs, we have tests in place to make sure
that the resize doesn't result in the viewport extending past the bottom
or right of the buffer (since that can eventually result in exceptions
being thrown). Unfortunately these tests were using the wrong X
coordinate, so they failed to detect an overflow along the horizontal
axis. This PR corrects that mistake.

PR #8309 was where the overflow detection was initially added.

The original code was using the `Viewport::EndExclusive` method to
determine the extent of the viewport, mistakenly thinking that it
returned the bottom right coordinates (it actually returns the left
coordinate). So I've now added a `BottomRightExclusive` method to the
`Viewport` class, that actually does return the coordinates we need, and
have updated the overflow tests to use that method instead.

## Validation Steps Performed
I've manually confirmed that the test case is issue #8453 is no longer
throwing an exception.

Closes #8453

(cherry picked from commit 2a2f6b3)
  • Loading branch information
j4james authored and DHowett committed Jan 25, 2021
1 parent 305946c commit a315fd5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
}

// Make sure the viewport doesn't now overflow the buffer dimensions.
auto overflow = screenInfo.GetViewport().EndExclusive() - screenInfo.GetBufferSize().Dimensions();
auto overflow = screenInfo.GetViewport().BottomRightExclusive() - screenInfo.GetBufferSize().Dimensions();
if (overflow.X > 0 || overflow.Y > 0)
{
overflow = { std::max<SHORT>(overflow.X, 0), std::max<SHORT>(overflow.Y, 0) };
Expand Down Expand Up @@ -638,7 +638,7 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
// Note that it also doesn't set cursor position.

// However, we do need to make sure the viewport doesn't now overflow the buffer dimensions.
auto overflow = context.GetViewport().EndExclusive() - context.GetBufferSize().Dimensions();
auto overflow = context.GetViewport().BottomRightExclusive() - context.GetBufferSize().Dimensions();
if (overflow.X > 0 || overflow.Y > 0)
{
overflow = { std::max<SHORT>(overflow.X, 0), std::max<SHORT>(overflow.Y, 0) };
Expand Down
1 change: 1 addition & 0 deletions src/types/inc/viewport.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ namespace Microsoft::Console::Types
SHORT Height() const noexcept;
SHORT Width() const noexcept;
COORD Origin() const noexcept;
COORD BottomRightExclusive() const noexcept;
COORD EndExclusive() const noexcept;
COORD Dimensions() const noexcept;

Expand Down
11 changes: 11 additions & 0 deletions src/types/viewport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ COORD Viewport::Origin() const noexcept
return { Left(), Top() };
}

// Method Description:
// - Get a coord representing the bottom right of the viewport in exclusive terms.
// Arguments:
// - <none>
// Return Value:
// - the exclusive bottom right coordinates of this viewport.
COORD Viewport::BottomRightExclusive() const noexcept
{
return { RightExclusive(), BottomExclusive() };
}

// Method Description:
// - For Accessibility, get a COORD representing the end of this viewport in exclusive terms.
// - This is needed to represent an exclusive endpoint in UiaTextRange that includes the last
Expand Down

0 comments on commit a315fd5

Please sign in to comment.