From 2a2f6b32a2f46d8029259676d7e93782c8477685 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 3 Dec 2020 21:53:16 +0000 Subject: [PATCH] Correct horizontal coordinates in viewport overflow test (#8456) 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 --- src/host/getset.cpp | 4 ++-- src/types/inc/viewport.hpp | 1 + src/types/viewport.cpp | 11 +++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 1850b47c9..d9ae45ef8 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -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(overflow.X, 0), std::max(overflow.Y, 0) }; @@ -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(overflow.X, 0), std::max(overflow.Y, 0) }; diff --git a/src/types/inc/viewport.hpp b/src/types/inc/viewport.hpp index 9ad56ade8..23f3d3e55 100644 --- a/src/types/inc/viewport.hpp +++ b/src/types/inc/viewport.hpp @@ -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; diff --git a/src/types/viewport.cpp b/src/types/viewport.cpp index f6cec56e4..ac3ac498f 100644 --- a/src/types/viewport.cpp +++ b/src/types/viewport.cpp @@ -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: +// - +// 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