From 76120443636a385ed05e6d9304a7932b4023f64e Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 30 Apr 2020 16:53:31 -0500 Subject: [PATCH] Implement a pair of shims for `cls`, `Clear-Host` in conpty mode (#5627) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request This PR implements a pair of shims for `cmd` and `powershell`, so that their `cls` and `Clear-Host` functions will clear the entire terminal buffer (like they do in conhost), instead of just the viewport. With the conpty viewport and buffer being the same size, there's effectively no way to know if an application is calling these API's in this way with the intention of clearing the buffer or the viewport. We absolutely have to guess. Each of these shims checks to see if the way that the API is being called exactly matches the way `cmd` or `powershell` would call these APIs. If it does, we manually write a `^[[3J` to the connected terminal, to get he Terminal to clear it's own scrollback. ~~_⚠️ If another application were trying to clear the **viewport** with an exactly similar API call, this would also cause the terminal scrollback to get cleared ⚠️_~~ * [x] Should these shims be restricted to when the process that's calling them is actually `cmd.exe` or `powershell.exe`? Can I even do this? I think we've done such a good job of isolating the client process information from the rest of the host code that I can't figure out how to do this. - YES, this can be done, and I did it. * [ ] **TODO**: _While I'm here_, should I have `DoSrvPrivateEraseAll` (the implementation for `^[[2J`, in `getset.cpp`) also manually trigger a EraseAll in the terminal in conpty mode? ## PR Checklist * [x] Closes #3126 * [x] Actually closes #1305 too, which is really the same thing, but probably deserves a callout * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed * ran tests * checked `cls` in the Terminal * checked `Clear-Host` in the Terminal * Checked running `powershell clear-host` from `cmd.exe` --- .../ConptyRoundtripTests.cpp | 184 ++++++++++++++---- src/host/ApiRoutines.h | 6 +- src/host/VtIo.cpp | 24 ++- src/host/VtIo.hpp | 4 +- src/host/_output.cpp | 31 ++- src/host/getset.cpp | 45 ++++- src/host/globals.cpp | 6 +- src/host/globals.h | 2 +- src/host/ut_host/ConptyOutputTests.cpp | 24 +-- src/inc/til/size.h | 8 + src/renderer/vt/VtSequences.cpp | 5 + src/renderer/vt/Xterm256Engine.cpp | 14 ++ src/renderer/vt/Xterm256Engine.hpp | 2 + src/renderer/vt/state.cpp | 17 ++ src/renderer/vt/vtrenderer.hpp | 3 + src/server/ApiDispatchers.cpp | 10 +- src/server/ConsoleShimPolicy.cpp | 82 ++++++++ src/server/ConsoleShimPolicy.h | 35 ++++ src/server/IApiRoutines.h | 6 +- src/server/ProcessHandle.cpp | 11 +- src/server/ProcessHandle.h | 3 + src/server/lib/server.vcxproj | 4 +- src/server/sources.inc | 1 + src/til/ut_til/SizeTests.cpp | 20 ++ 24 files changed, 475 insertions(+), 72 deletions(-) create mode 100644 src/server/ConsoleShimPolicy.cpp create mode 100644 src/server/ConsoleShimPolicy.h diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index da44d82e3..66d46bce1 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -114,28 +114,28 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final wil::unique_hfile hFile = wil::unique_hfile(INVALID_HANDLE_VALUE); Viewport initialViewport = currentBuffer.GetViewport(); - _pVtRenderEngine = std::make_unique(std::move(hFile), - gci, - initialViewport, - gci.GetColorTable(), - static_cast(gci.GetColorTableSize())); + auto vtRenderEngine = std::make_unique(std::move(hFile), + gci, + initialViewport, + gci.GetColorTable(), + static_cast(gci.GetColorTableSize())); auto pfn = std::bind(&ConptyRoundtripTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2); - _pVtRenderEngine->SetTestCallback(pfn); + vtRenderEngine->SetTestCallback(pfn); // Enable the resize quirk, as the Terminal is going to be reacting as if it's enabled. - _pVtRenderEngine->SetResizeQuirk(true); + vtRenderEngine->SetResizeQuirk(true); // Configure the OutputStateMachine's _pfnFlushToTerminal // Use OutputStateMachineEngine::SetTerminalConnection - g.pRender->AddRenderEngine(_pVtRenderEngine.get()); - gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get()); + g.pRender->AddRenderEngine(vtRenderEngine.get()); + gci.GetActiveOutputBuffer().SetTerminalConnection(vtRenderEngine.get()); _pConApi = std::make_unique(gci); // Manually set the console into conpty mode. We're not actually going // to set up the pipes for conpty, but we want the console to behave // like it would in conpty mode. - g.EnableConptyModeForTests(); + g.EnableConptyModeForTests(std::move(vtRenderEngine)); expectedOutput.clear(); _checkConptyOutput = true; @@ -196,6 +196,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(ResizeRepaintVimExeBuffer); + TEST_METHOD(ClsAndClearHostClearsScrollbackTest); + TEST_METHOD(TestResizeWithCookedRead); TEST_METHOD(NewLinesAtBottomWithBackground); @@ -208,7 +210,7 @@ private: [[nodiscard]] std::tuple _performResize(const til::size& newSize); std::deque expectedOutput; - std::unique_ptr _pVtRenderEngine; + std::unique_ptr m_state; std::unique_ptr _pConApi; @@ -218,6 +220,8 @@ private: DummyRenderTarget emptyRT; std::unique_ptr term; + + ApiRoutines _apiRoutines; }; bool ConptyRoundtripTests::_writeCallback(const char* const pch, size_t const cch) @@ -271,10 +275,9 @@ void ConptyRoundtripTests::_resizeConpty(const unsigned short sx, // Largely taken from implementation in PtySignalInputThread::_InputThread if (DispatchCommon::s_ResizeWindow(*_pConApi, sx, sy)) { - // Instead of going through the VtIo to suppress the resize repaint, - // just call the method directly on the renderer. This is implemented in - // VtIo::SuppressResizeRepaint - VERIFY_SUCCEEDED(_pVtRenderEngine->SuppressResizeRepaint()); + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + VERIFY_SUCCEEDED(gci.GetVtIo()->SuppressResizeRepaint()); } } @@ -295,7 +298,6 @@ void ConptyRoundtripTests::ConptyOutputTestCanary() { Log::Comment(NoThrowString().Format( L"This is a simple test to make sure that everything is working as expected.")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); _flushFirstFrame(); } @@ -305,7 +307,6 @@ void ConptyRoundtripTests::SimpleWriteOutputTest() Log::Comment(NoThrowString().Format( L"Write some simple output, and make sure it gets rendered largely " L"unmodified to the terminal")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -328,7 +329,6 @@ void ConptyRoundtripTests::WriteTwoLinesUsesNewline() { Log::Comment(NoThrowString().Format( L"Write two lines of output. We should use \r\n to move the cursor")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -364,7 +364,6 @@ void ConptyRoundtripTests::WriteAFewSimpleLines() { Log::Comment(NoThrowString().Format( L"Write more lines of outout. We should use \r\n to move the cursor")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -645,7 +644,6 @@ void ConptyRoundtripTests::TestExactWrappingWithSpaces() void ConptyRoundtripTests::MoveCursorAtEOL() { // This is a test for GH#1245 - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -927,7 +925,6 @@ void ConptyRoundtripTests::PassthroughCursorShapeImmediately() Log::Comment(NoThrowString().Format( L"Change the cursor shape with VT. This should immediately be flushed to the Terminal.")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& gci = g.getConsoleInformation(); @@ -954,7 +951,6 @@ void ConptyRoundtripTests::PassthroughClearScrollback() { Log::Comment(NoThrowString().Format( L"Write more lines of output than there are lines in the viewport. Clear the scrollback with ^[[3J")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1030,7 +1026,6 @@ void ConptyRoundtripTests::PassthroughHardReset() // This test is highly similar to PassthroughClearScrollback. Log::Comment(NoThrowString().Format( L"Write more lines of output than there are lines in the viewport. Clear everything with ^[c")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1095,7 +1090,6 @@ void ConptyRoundtripTests::OutputWrappedLinesAtTopOfBuffer() { Log::Comment( L"Case 1: Write a wrapped line right at the start of the buffer, before any circling"); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1144,7 +1138,6 @@ void ConptyRoundtripTests::OutputWrappedLinesAtBottomOfBuffer() { Log::Comment( L"Case 2: Write a wrapped line at the end of the buffer, once the conpty started circling"); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1278,7 +1271,6 @@ void ConptyRoundtripTests::ScrollWithChangesInMiddle() L" output will cause us to scroll the viewport in one frame, but we need to" L" make sure the wrapped line _stays_ wrapped, and the scrolled text appears in" L" the right place."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1624,7 +1616,6 @@ void ConptyRoundtripTests::DontWrapMoveCursorInSingleFrame() L"the cursor didn't end the frame at the end of line (waiting " L"for more wrapped text). We should still move the cursor in " L"this case."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1719,7 +1710,6 @@ void ConptyRoundtripTests::ClearHostTrickeryTest() // - printTextAfterSpaces= // // All the possible cases are left here though, to catch potential future regressions. - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1828,7 +1818,6 @@ void ConptyRoundtripTests::OverstrikeAtBottomOfBuffer() Log::Comment(L"This test replicates the zsh menu-complete functionality. In" L" the course of a single frame, we're going to both scroll " L"the frame and print multiple lines of text above the bottom line."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1907,7 +1896,6 @@ void ConptyRoundtripTests::MarginsWithStatusLine() // then they re-printing the status line. Log::Comment(L"Newline, and scroll the bottom lines of the buffer down with" L" ScrollConsoleScreenBuffer to emulate how cygwin VIM works"); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -1999,7 +1987,6 @@ void ConptyRoundtripTests::OutputWrappedLineWithSpace() // See https://github.com/microsoft/terminal/pull/5181#issuecomment-610110348 Log::Comment(L"Ensures that a buffer line in conhost that wrapped _on a " L"space_ will still be emitted as wrapped."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -2066,7 +2053,6 @@ void ConptyRoundtripTests::OutputWrappedLineWithSpaceAtBottomOfBuffer() // the buffer, so we get scrolling behavior as well. Log::Comment(L"Ensures that a buffer line in conhost that wrapped _on a " L"space_ will still be emitted as wrapped."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -2222,7 +2208,6 @@ void ConptyRoundtripTests::BreakLinesOnCursorMovement() L" ends of blank lines, not EL. This test ensures we emit text" L" from conpty such that the terminal re-creates the state of" L" the host, which includes wrapped lines of lots of spaces."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -2434,7 +2419,6 @@ void ConptyRoundtripTests::ResizeRepaintVimExeBuffer() // See https://github.com/microsoft/terminal/issues/5428 Log::Comment(L"This test emulates what happens when you decrease the width " L"of the window while running vim.exe."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -2554,6 +2538,135 @@ void ConptyRoundtripTests::ResizeRepaintVimExeBuffer() verifyBuffer(*termTb, term->_mutableViewport.ToInclusive()); } +void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest() +{ + // See https://github.com/microsoft/terminal/issues/3126#issuecomment-620677742 + + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"Data:clearBufferMethod", L"{0, 1}") + END_TEST_METHOD_PROPERTIES(); + constexpr int ClearLikeCls = 0; + constexpr int ClearLikeClearHost = 1; + INIT_TEST_PROPERTY(int, clearBufferMethod, L"Controls whether we clear the buffer like cmd or like powershell"); + + Log::Comment(L"This test checks the shims for cmd.exe and powershell.exe. " + L"Their build in commands for clearing the console buffer " + L"should work to clear the terminal buffer, not just the " + L"terminal viewport."); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto* hostTb = &si.GetTextBuffer(); + auto* termTb = term->_buffer.get(); + + auto& sm = si.GetStateMachine(); + + _flushFirstFrame(); + + _checkConptyOutput = false; + _logConpty = true; + + const auto hostView = si.GetViewport(); + const auto end = 2 * hostView.Height(); + for (auto i = 0; i < end; i++) + { + if (i > 0) + { + sm.ProcessString(L"\r\n"); + } + + sm.ProcessString(L"~"); + } + + auto verifyBuffer = [&](const TextBuffer& tb, const til::rectangle viewport, const bool afterClear = false) { + const auto firstRow = viewport.top(); + const auto width = viewport.width(); + + // "~" rows + for (short row = 0; row < viewport.bottom(); row++) + { + Log::Comment(NoThrowString().Format(L"Checking row %d", row)); + VERIFY_IS_FALSE(tb.GetRowByOffset(row).GetCharRow().WasWrapForced()); + auto iter = tb.GetCellDataAt({ 0, row }); + if (afterClear) + { + TestUtils::VerifySpanOfText(L" ", iter, 0, width); + } + else + { + TestUtils::VerifySpanOfText(L"~", iter, 0, 1); + TestUtils::VerifySpanOfText(L" ", iter, 0, width - 1); + } + } + }; + + Log::Comment(L"========== Checking the host buffer state (before) =========="); + verifyBuffer(*hostTb, si.GetViewport().ToInclusive()); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the terminal buffer state (before) =========="); + verifyBuffer(*termTb, term->_mutableViewport.ToInclusive()); + + if (clearBufferMethod == ClearLikeCls) + { + // Execute the cls, EXACTLY LIKE CMD. + + CONSOLE_SCREEN_BUFFER_INFOEX csbiex{ 0 }; + csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX); + _apiRoutines.GetConsoleScreenBufferInfoExImpl(si, csbiex); + + SMALL_RECT src{ 0 }; + src.Top = 0; + src.Left = 0; + src.Right = csbiex.dwSize.X; + src.Bottom = csbiex.dwSize.Y; + + COORD tgt{ 0, -csbiex.dwSize.Y }; + VERIFY_SUCCEEDED(_apiRoutines.ScrollConsoleScreenBufferWImpl(si, + src, + tgt, + std::nullopt, // no clip provided, + L' ', + csbiex.wAttributes, + true)); + } + else if (clearBufferMethod == ClearLikeClearHost) + { + CONSOLE_SCREEN_BUFFER_INFOEX csbiex{ 0 }; + csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX); + _apiRoutines.GetConsoleScreenBufferInfoExImpl(si, csbiex); + + const auto totalCellsInBuffer = csbiex.dwSize.X * csbiex.dwSize.Y; + size_t cellsWritten = 0; + VERIFY_SUCCEEDED(_apiRoutines.FillConsoleOutputCharacterWImpl(si, + L' ', + totalCellsInBuffer, + { 0, 0 }, + cellsWritten, + true)); + VERIFY_SUCCEEDED(_apiRoutines.FillConsoleOutputAttributeImpl(si, + csbiex.wAttributes, + totalCellsInBuffer, + { 0, 0 }, + cellsWritten)); + } + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the host buffer state (after) =========="); + verifyBuffer(*hostTb, si.GetViewport().ToInclusive(), true); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + Log::Comment(L"========== Checking the terminal buffer state (after) =========="); + verifyBuffer(*termTb, term->_mutableViewport.ToInclusive(), true); +} + void ConptyRoundtripTests::TestResizeWithCookedRead() { // see https://github.com/microsoft/terminal/issues/1856 @@ -2583,8 +2696,6 @@ void ConptyRoundtripTests::TestResizeWithCookedRead() INIT_TEST_PROPERTY(int, dx, L"The change in width of the buffer"); INIT_TEST_PROPERTY(int, dy, L"The change in height of the buffer"); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); - auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; auto& gci = g.getConsoleInformation(); @@ -2632,7 +2743,6 @@ void ConptyRoundtripTests::NewLinesAtBottomWithBackground() L"a colored background. When that happens, we should make " L"sure to still print the spaces, because the information " L"about their background color is important."); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; diff --git a/src/host/ApiRoutines.h b/src/host/ApiRoutines.h index b02909335..22ca2235a 100644 --- a/src/host/ApiRoutines.h +++ b/src/host/ApiRoutines.h @@ -129,7 +129,8 @@ public: const wchar_t character, const size_t lengthToWrite, const COORD startingCoordinate, - size_t& cellsModified) noexcept override; + size_t& cellsModified, + const bool enablePowershellShim = false) noexcept override; //// Process based. Restrict in protocol side? //HRESULT GenerateConsoleCtrlEventImpl(const ULONG ProcessGroupFilter, @@ -179,7 +180,8 @@ public: const COORD target, std::optional clip, const wchar_t fillCharacter, - const WORD fillAttribute) noexcept override; + const WORD fillAttribute, + const bool enableCmdShim = false) noexcept override; [[nodiscard]] HRESULT SetConsoleTextAttributeImpl(SCREEN_INFORMATION& context, const WORD attribute) noexcept override; diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index 6743983fa..09b3af630 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -440,12 +440,13 @@ void VtIo::EndResize() // true to `IsUsingVt`, which will cause the console host to act in conpty // mode. // Arguments: -// - +// - vtRenderEngine: a VT renderer that our VtIo should use as the vt engine during these tests // Return Value: // - -void VtIo::EnableConptyModeForTests() +void VtIo::EnableConptyModeForTests(std::unique_ptr vtRenderEngine) { _objectsCreated = true; + _pVtRenderEngine = std::move(vtRenderEngine); } #endif @@ -464,3 +465,22 @@ bool VtIo::IsResizeQuirkEnabled() const { return _resizeQuirk; } + +// Method Description: +// - Manually tell the renderer that it should emit a "Erase Scrollback" +// sequence to the connected terminal. We need to do this in certain cases +// that we've identified where we believe the client wanted the entire +// terminal buffer cleared, not just the viewport. For more information, see +// GH#3126. +// Arguments: +// - +// Return Value: +// - S_OK if we wrote the sequences successfully, otherwise an appropriate HRESULT +[[nodiscard]] HRESULT VtIo::ManuallyClearScrollback() const noexcept +{ + if (_pVtRenderEngine) + { + return _pVtRenderEngine->ManuallyClearScrollback(); + } + return S_OK; +} diff --git a/src/host/VtIo.hpp b/src/host/VtIo.hpp index 37186ef0c..3ee6654d8 100644 --- a/src/host/VtIo.hpp +++ b/src/host/VtIo.hpp @@ -40,11 +40,13 @@ namespace Microsoft::Console::VirtualTerminal void EndResize(); #ifdef UNIT_TESTING - void EnableConptyModeForTests(); + void EnableConptyModeForTests(std::unique_ptr vtRenderEngine); #endif bool IsResizeQuirkEnabled() const; + [[nodiscard]] HRESULT ManuallyClearScrollback() const noexcept; + private: // After CreateIoHandlers is called, these will be invalid. wil::unique_hfile _hInput; diff --git a/src/host/_output.cpp b/src/host/_output.cpp index bdd1dfc1a..67ad7fedf 100644 --- a/src/host/_output.cpp +++ b/src/host/_output.cpp @@ -242,13 +242,17 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region) // - lengthToWrite - the number of elements to write // - startingCoordinate - Screen buffer coordinate to begin writing to. // - cellsModified - the number of elements written +// - enablePowershellShim - true iff the client process that's calling this +// method is "powershell.exe". Used to enable certain compatibility shims for +// conpty mode. See GH#3126. // Return Value: // - S_OK or suitable HRESULT code from failure to write (memory issues, invalid arg, etc.) [[nodiscard]] HRESULT ApiRoutines::FillConsoleOutputCharacterWImpl(IConsoleOutputObject& OutContext, const wchar_t character, const size_t lengthToWrite, const COORD startingCoordinate, - size_t& cellsModified) noexcept + size_t& cellsModified, + const bool enablePowershellShim) noexcept { // Set modified cells to 0 from the beginning. cellsModified = 0; @@ -269,6 +273,7 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region) return S_OK; } + HRESULT hr = S_OK; try { const OutputCellIterator it(character, lengthToWrite); @@ -282,10 +287,32 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region) auto endingCoordinate = startingCoordinate; bufferSize.MoveInBounds(cellsModified, endingCoordinate); screenInfo.NotifyAccessibilityEventing(startingCoordinate.X, startingCoordinate.Y, endingCoordinate.X, endingCoordinate.Y); + + // GH#3126 - This is a shim for powershell's `Clear-Host` function. In + // the vintage console, `Clear-Host` is supposed to clear the entire + // buffer. In conpty however, there's no difference between the viewport + // and the entirety of the buffer. We're going to see if this API call + // exactly matched the way we expect powershell to call it. If it does, + // then let's manually emit a ^[[3J to the connected terminal, so that + // their entire buffer will be cleared as well. + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + if (enablePowershellShim && gci.IsInVtIoMode()) + { + const til::size currentBufferDimensions{ screenInfo.GetBufferSize().Dimensions() }; + + const bool wroteWholeBuffer = lengthToWrite == (currentBufferDimensions.area()); + const bool startedAtOrigin = startingCoordinate == COORD{ 0, 0 }; + const bool wroteSpaces = character == UNICODE_SPACE; + + if (wroteWholeBuffer && startedAtOrigin && wroteSpaces) + { + hr = gci.GetVtIo()->ManuallyClearScrollback(); + } + } } CATCH_RETURN(); - return S_OK; + return hr; } // Routine Description: diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 0c96032d4..160faf580 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -127,7 +127,13 @@ void ApiRoutines::GetConsoleScreenBufferInfoExImpl(const SCREEN_INFORMATION& con &data.dwMaximumWindowSize, &data.wPopupAttributes, data.ColorTable); - // Callers of this function expect to receive an exclusive rect, not an inclusive one. + + // Callers of this function expect to receive an exclusive rect, not an + // inclusive one. The driver will mangle this value for us + // - For GetConsoleScreenBufferInfoEx, it will re-decrement these values + // to return an inclusive rect. + // - For GetConsoleScreenBufferInfo, it will leave these values + // untouched, returning an exclusive rect. data.srWindow.Right += 1; data.srWindow.Bottom += 1; } @@ -828,6 +834,9 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont // - clip - The rectangle inside which all operations should be bounded (or no bounds if not given) // - fillCharacter - Fills in the region left behind when the source is "lifted" out of its original location. The symbol to display. // - fillAttribute - Fills in the region left behind when the source is "lifted" out of its original location. The color to use. +// - enableCmdShim - true iff the client process that's calling this +// method is "cmd.exe". Used to enable certain compatibility shims for +// conpty mode. See GH#3126. // Return Value: // - S_OK, E_INVALIDARG, or failure code from thrown exception [[nodiscard]] HRESULT ApiRoutines::ScrollConsoleScreenBufferWImpl(SCREEN_INFORMATION& context, @@ -835,7 +844,8 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont const COORD target, std::optional clip, const wchar_t fillCharacter, - const WORD fillAttribute) noexcept + const WORD fillAttribute, + const bool enableCmdShim) noexcept { try { @@ -847,7 +857,36 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont TextAttribute useThisAttr(fillAttribute); ScrollRegion(buffer, source, clip, target, fillCharacter, useThisAttr); - return S_OK; + HRESULT hr = S_OK; + + // GH#3126 - This is a shim for cmd's `cls` function. In the + // legacy console, `cls` is supposed to clear the entire buffer. In + // conpty however, there's no difference between the viewport and the + // entirety of the buffer. We're going to see if this API call exactly + // matched the way we expect cmd to call it. If it does, then + // let's manually emit a ^[[3J to the connected terminal, so that their + // entire buffer will be cleared as well. + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + if (enableCmdShim && gci.IsInVtIoMode()) + { + const auto currentBufferDimensions = buffer.GetBufferSize().Dimensions(); + const bool sourceIsWholeBuffer = (source.Top == 0) && + (source.Left == 0) && + (source.Right == currentBufferDimensions.X) && + (source.Bottom == currentBufferDimensions.Y); + const bool targetIsNegativeBufferHeight = (target.X == 0) && + (target.Y == -currentBufferDimensions.Y); + const bool noClipProvided = clip == std::nullopt; + const bool fillIsBlank = (fillCharacter == UNICODE_SPACE) && + (fillAttribute == gci.GenerateLegacyAttributes(buffer.GetAttributes())); + + if (sourceIsWholeBuffer && targetIsNegativeBufferHeight && noClipProvided && fillIsBlank) + { + hr = gci.GetVtIo()->ManuallyClearScrollback(); + } + } + + return hr; } CATCH_RETURN(); } diff --git a/src/host/globals.cpp b/src/host/globals.cpp index f39a0d106..6cbff4b52 100644 --- a/src/host/globals.cpp +++ b/src/host/globals.cpp @@ -23,12 +23,12 @@ bool Globals::IsHeadless() const // true to `IsHeadless`, which will cause the console host to act in conpty // mode. // Arguments: -// - +// - vtRenderEngine: a VT renderer that our VtIo should use as the vt engine during these tests // Return Value: // - -void Globals::EnableConptyModeForTests() +void Globals::EnableConptyModeForTests(std::unique_ptr vtRenderEngine) { launchArgs.EnableConptyModeForTests(); - getConsoleInformation().GetVtIo()->EnableConptyModeForTests(); + getConsoleInformation().GetVtIo()->EnableConptyModeForTests(std::move(vtRenderEngine)); } #endif diff --git a/src/host/globals.h b/src/host/globals.h index bd320d90d..e0ed9f411 100644 --- a/src/host/globals.h +++ b/src/host/globals.h @@ -70,7 +70,7 @@ public: ApiRoutines api; #ifdef UNIT_TESTING - void EnableConptyModeForTests(); + void EnableConptyModeForTests(std::unique_ptr vtRenderEngine); #endif private: diff --git a/src/host/ut_host/ConptyOutputTests.cpp b/src/host/ut_host/ConptyOutputTests.cpp index 6b438ab9d..071e7dd21 100644 --- a/src/host/ut_host/ConptyOutputTests.cpp +++ b/src/host/ut_host/ConptyOutputTests.cpp @@ -85,23 +85,23 @@ class ConptyOutputTests wil::unique_hfile hFile = wil::unique_hfile(INVALID_HANDLE_VALUE); Viewport initialViewport = currentBuffer.GetViewport(); - _pVtRenderEngine = std::make_unique(std::move(hFile), - gci, - initialViewport, - gci.GetColorTable(), - static_cast(gci.GetColorTableSize())); + auto vtRenderEngine = std::make_unique(std::move(hFile), + gci, + initialViewport, + gci.GetColorTable(), + static_cast(gci.GetColorTableSize())); auto pfn = std::bind(&ConptyOutputTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2); - _pVtRenderEngine->SetTestCallback(pfn); + vtRenderEngine->SetTestCallback(pfn); - g.pRender->AddRenderEngine(_pVtRenderEngine.get()); - gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get()); + g.pRender->AddRenderEngine(vtRenderEngine.get()); + gci.GetActiveOutputBuffer().SetTerminalConnection(vtRenderEngine.get()); expectedOutput.clear(); // Manually set the console into conpty mode. We're not actually going // to set up the pipes for conpty, but we want the console to behave // like it would in conpty mode. - g.EnableConptyModeForTests(); + g.EnableConptyModeForTests(std::move(vtRenderEngine)); return true; } @@ -128,7 +128,6 @@ private: bool _writeCallback(const char* const pch, size_t const cch); void _flushFirstFrame(); std::deque expectedOutput; - std::unique_ptr _pVtRenderEngine; std::unique_ptr m_state; }; @@ -202,7 +201,6 @@ void ConptyOutputTests::ConptyOutputTestCanary() { Log::Comment(NoThrowString().Format( L"This is a simple test to make sure that everything is working as expected.")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); _flushFirstFrame(); } @@ -212,7 +210,6 @@ void ConptyOutputTests::SimpleWriteOutputTest() Log::Comment(NoThrowString().Format( L"Write some simple output, and make sure it gets rendered largely " L"unmodified to the terminal")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -232,7 +229,6 @@ void ConptyOutputTests::WriteTwoLinesUsesNewline() { Log::Comment(NoThrowString().Format( L"Write two lines of output. We should use \r\n to move the cursor")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -271,7 +267,6 @@ void ConptyOutputTests::WriteAFewSimpleLines() { Log::Comment(NoThrowString().Format( L"Write more lines of output. We should use \r\n to move the cursor")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; @@ -330,7 +325,6 @@ void ConptyOutputTests::InvalidateUntilOneBeforeEnd() { Log::Comment(NoThrowString().Format( L"Make sure we don't use EL and wipe out the last column of text")); - VERIFY_IS_NOT_NULL(_pVtRenderEngine.get()); auto& g = ServiceLocator::LocateGlobals(); auto& renderer = *g.pRender; diff --git a/src/inc/til/size.h b/src/inc/til/size.h index 67230b882..383cb13d9 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -250,6 +250,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return result; } + template + T area() const + { + T ret; + THROW_HR_IF(E_ABORT, !base::CheckMul(_width, _height).AssignIfValid(&ret)); + return ret; + } + #ifdef _WINCONTYPES_ operator COORD() const { diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 73460df82..0b9c7baaf 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -109,6 +109,11 @@ using namespace Microsoft::Console::Render; return _Write("\x1b[2J"); } +[[nodiscard]] HRESULT VtEngine::_ClearScrollback() noexcept +{ + return _Write("\x1b[3J"); +} + // Method Description: // - Formats and writes a sequence to either insert or delete a number of lines // into the buffer at the current cursor location. diff --git a/src/renderer/vt/Xterm256Engine.cpp b/src/renderer/vt/Xterm256Engine.cpp index bc1ea4a38..ba440db03 100644 --- a/src/renderer/vt/Xterm256Engine.cpp +++ b/src/renderer/vt/Xterm256Engine.cpp @@ -110,3 +110,17 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, return S_OK; } + +// Method Description: +// - Manually emit a "Erase Scrollback" sequence to the connected terminal. We +// need to do this in certain cases that we've identified where we believe the +// client wanted the entire terminal buffer cleared, not just the viewport. +// For more information, see GH#3126. +// Arguments: +// - +// Return Value: +// - S_OK if we wrote the sequences successfully, otherwise an appropriate HRESULT +[[nodiscard]] HRESULT Xterm256Engine::ManuallyClearScrollback() noexcept +{ + return _ClearScrollback(); +} diff --git a/src/renderer/vt/Xterm256Engine.hpp b/src/renderer/vt/Xterm256Engine.hpp index 48fe1fc39..3addb195e 100644 --- a/src/renderer/vt/Xterm256Engine.hpp +++ b/src/renderer/vt/Xterm256Engine.hpp @@ -37,6 +37,8 @@ namespace Microsoft::Console::Render const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; + [[nodiscard]] HRESULT ManuallyClearScrollback() noexcept override; + private: [[nodiscard]] HRESULT _UpdateExtendedAttrs(const ExtendedAttributes extendedAttrs) noexcept; diff --git a/src/renderer/vt/state.cpp b/src/renderer/vt/state.cpp index 8714f7855..099fa4877 100644 --- a/src/renderer/vt/state.cpp +++ b/src/renderer/vt/state.cpp @@ -492,3 +492,20 @@ void VtEngine::SetResizeQuirk(const bool resizeQuirk) { _resizeQuirk = resizeQuirk; } + +// Method Description: +// - Manually emit a "Erase Scrollback" sequence to the connected terminal. We +// need to do this in certain cases that we've identified where we believe the +// client wanted the entire terminal buffer cleared, not just the viewport. +// For more information, see GH#3126. +// - This is unimplemented in the win-telnet, xterm-ascii renderers - inbox +// telnet.exe doesn't know how to handle a ^[[3J. This _is_ implemented in the +// Xterm256Engine. +// Arguments: +// - +// Return Value: +// - S_OK if we wrote the sequences successfully, otherwise an appropriate HRESULT +[[nodiscard]] HRESULT VtEngine::ManuallyClearScrollback() noexcept +{ + return S_OK; +} diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index 671fcd9f7..5f683bc9b 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -108,6 +108,8 @@ namespace Microsoft::Console::Render void SetResizeQuirk(const bool resizeQuirk); + [[nodiscard]] virtual HRESULT ManuallyClearScrollback() noexcept; + protected: wil::unique_hfile _hFile; std::string _buffer; @@ -175,6 +177,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT _CursorPosition(const COORD coord) noexcept; [[nodiscard]] HRESULT _CursorHome() noexcept; [[nodiscard]] HRESULT _ClearScreen() noexcept; + [[nodiscard]] HRESULT _ClearScrollback() noexcept; [[nodiscard]] HRESULT _ChangeTitle(const std::string& title) noexcept; [[nodiscard]] HRESULT _SetGraphicsRendition16Color(const WORD wAttr, const bool fIsForeground) noexcept; diff --git a/src/server/ApiDispatchers.cpp b/src/server/ApiDispatchers.cpp index f013440cf..72384a584 100644 --- a/src/server/ApiDispatchers.cpp +++ b/src/server/ApiDispatchers.cpp @@ -502,11 +502,14 @@ case CONSOLE_REAL_UNICODE: case CONSOLE_FALSE_UNICODE: { + // GH#3126 if the client application is powershell.exe, then we might + // need to enable a compatibility shim. hr = m->_pApiRoutines->FillConsoleOutputCharacterWImpl(*pScreenInfo, a->Element, fill, a->WriteCoord, - amountWritten); + amountWritten, + m->GetProcessHandle()->GetShimPolicy().IsPowershellExe()); break; } case CONSOLE_ASCII: @@ -733,12 +736,15 @@ if (a->Unicode) { + // GH#3126 if the client application is cmd.exe, then we might need to + // enable a compatibility shim. return m->_pApiRoutines->ScrollConsoleScreenBufferWImpl(*pObj, a->ScrollRectangle, a->DestinationOrigin, a->Clip ? std::optional(a->ClipRectangle) : std::nullopt, a->Fill.Char.UnicodeChar, - a->Fill.Attributes); + a->Fill.Attributes, + m->GetProcessHandle()->GetShimPolicy().IsCmdExe()); } else { diff --git a/src/server/ConsoleShimPolicy.cpp b/src/server/ConsoleShimPolicy.cpp new file mode 100644 index 000000000..23556ed0f --- /dev/null +++ b/src/server/ConsoleShimPolicy.cpp @@ -0,0 +1,82 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" + +#include "ConsoleShimPolicy.h" + +// Routine Description: +// - Constructs a new instance of the shim policy class. +// Arguments: +// - All arguments specify a true/false status to a policy that could be applied to a console client app. +ConsoleShimPolicy::ConsoleShimPolicy(const bool isCmd, + const bool isPowershell) : + _isCmd{ isCmd }, + _isPowershell{ isPowershell } +{ +} + +// Routine Description: +// - Opens the process token for the given handle and resolves the process name. +// We'll initialize the new ConsoleShimPolicy based on whether the client +// process is "cmd.exe" or "powershell.exe". +// - For more info, see GH#3126 +// Arguments: +// - hProcess - Handle to a connected process +// Return Value: +// - ConsoleShimPolicy object containing resolved shim policy data. +ConsoleShimPolicy ConsoleShimPolicy::s_CreateInstance(const HANDLE hProcess) +{ + // If we cannot determine the exe name, then we're probably not cmd or powershell. + bool isCmd = false; + bool isPowershell = false; + + try + { + const std::filesystem::path processName = wil::GetModuleFileNameExW(hProcess, nullptr); + auto clientName = processName.filename().wstring(); + // For whatever reason, wil::GetModuleFileNameExW leaves trailing nulls, so get rid of them. + clientName.erase(std::find(clientName.begin(), clientName.end(), '\0'), clientName.end()); + + // Convert to lower case, just in case + std::transform(clientName.begin(), clientName.end(), clientName.begin(), std::towlower); + + isCmd = clientName.compare(L"cmd.exe") == 0; + + // For powershell, we need both Windows PowersShell (powershell.exe) and + // PowerShell Core (pwsh.exe). If PowerShell Core is ever updated to use + // ^[[3J for Clear-Host, then it won't ever hit the shim code path, but + // we're keeping this for the long tail of pwsh versions that still + // _don't_ use that sequence. + isPowershell = (clientName.compare(L"powershell.exe") == 0) || + (clientName.compare(L"pwsh.exe") == 0); + } + CATCH_LOG(); + + return ConsoleShimPolicy(isCmd, isPowershell); +} + +// Method Description: +// - Returns true if the connected client application is literally "cmd.exe". If +// it is, we'll need to enable certain compatibility shims. +// Arguments: +// - +// Return Value: +// - rue iff the process is "cmd.exe" +bool ConsoleShimPolicy::IsCmdExe() const noexcept +{ + return _isCmd; +} + +// Method Description: +// - Returns true if the connected client application is literally +// "powershell.exe" or "pwsh.exe". If it is, we'll need to enable certain +// compatibility shims. +// Arguments: +// - +// Return Value: +// - rue iff the process is "powershell.exe" or "pwsh.exe" +bool ConsoleShimPolicy::IsPowershellExe() const noexcept +{ + return _isPowershell; +} diff --git a/src/server/ConsoleShimPolicy.h b/src/server/ConsoleShimPolicy.h new file mode 100644 index 000000000..89006d283 --- /dev/null +++ b/src/server/ConsoleShimPolicy.h @@ -0,0 +1,35 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- ConsoleShimPolicy.h + +Abstract: +- This is a helper class to identify if the client process is cmd.exe or + powershell.exe. If it is, we might need to enable certain compatibility shims + for them. +- For more info, see GH#3126 + +Author: +- Mike Griese (migrie) 29-Apr-2020 + +--*/ + +#pragma once + +class ConsoleShimPolicy +{ +public: + static ConsoleShimPolicy s_CreateInstance(const HANDLE hProcess); + + bool IsCmdExe() const noexcept; + bool IsPowershellExe() const noexcept; + +private: + ConsoleShimPolicy(const bool isCmd, + const bool isPowershell); + + const bool _isCmd; + const bool _isPowershell; +}; diff --git a/src/server/IApiRoutines.h b/src/server/IApiRoutines.h index df2e4a7d4..33b7eab32 100644 --- a/src/server/IApiRoutines.h +++ b/src/server/IApiRoutines.h @@ -144,7 +144,8 @@ public: const wchar_t character, const size_t lengthToWrite, const COORD startingCoordinate, - size_t& cellsModified) noexcept = 0; + size_t& cellsModified, + const bool enablePowershellShim = false) noexcept = 0; virtual void SetConsoleActiveScreenBufferImpl(IConsoleOutputObject& newContext) noexcept = 0; @@ -190,7 +191,8 @@ public: const COORD target, std::optional clip, const wchar_t fillCharacter, - const WORD fillAttribute) noexcept = 0; + const WORD fillAttribute, + const bool enableCmdShim = false) noexcept = 0; [[nodiscard]] virtual HRESULT SetConsoleTextAttributeImpl(IConsoleOutputObject& context, const WORD attribute) noexcept = 0; diff --git a/src/server/ProcessHandle.cpp b/src/server/ProcessHandle.cpp index 531e445f3..28e698efb 100644 --- a/src/server/ProcessHandle.cpp +++ b/src/server/ProcessHandle.cpp @@ -26,7 +26,8 @@ ConsoleProcessHandle::ConsoleProcessHandle(const DWORD dwProcessId, _hProcess(LOG_LAST_ERROR_IF_NULL(OpenProcess(MAXIMUM_ALLOWED, FALSE, dwProcessId))), - _policy(ConsoleProcessPolicy::s_CreateInstance(_hProcess.get())) + _policy(ConsoleProcessPolicy::s_CreateInstance(_hProcess.get())), + _shimPolicy(ConsoleShimPolicy::s_CreateInstance(_hProcess.get())) { if (nullptr != _hProcess.get()) { @@ -50,3 +51,11 @@ const ConsoleProcessPolicy ConsoleProcessHandle::GetPolicy() const { return _policy; } + +// Routine Description: +// - Retrieves the policies set on this particular process handle +// - This specifies compatibility shims that we might need to make for certain applications. +const ConsoleShimPolicy ConsoleProcessHandle::GetShimPolicy() const +{ + return _shimPolicy; +} diff --git a/src/server/ProcessHandle.h b/src/server/ProcessHandle.h index bc8c16d98..f92ce054e 100644 --- a/src/server/ProcessHandle.h +++ b/src/server/ProcessHandle.h @@ -20,6 +20,7 @@ Revision History: #include "ObjectHandle.h" #include "WaitQueue.h" #include "ProcessPolicy.h" +#include "ConsoleShimPolicy.h" #include #include @@ -37,6 +38,7 @@ public: DWORD const dwThreadId; const ConsoleProcessPolicy GetPolicy() const; + const ConsoleShimPolicy GetShimPolicy() const; CD_CONNECTION_INFORMATION GetConnectionInformation() const; @@ -55,6 +57,7 @@ private: wil::unique_handle const _hProcess; const ConsoleProcessPolicy _policy; + const ConsoleShimPolicy _shimPolicy; friend class ConsoleProcessList; // ensure List manages lifetimes and not other classes. }; diff --git a/src/server/lib/server.vcxproj b/src/server/lib/server.vcxproj index e6bd12063..390efaa27 100644 --- a/src/server/lib/server.vcxproj +++ b/src/server/lib/server.vcxproj @@ -6,7 +6,7 @@ server Server ConServer - StaticLibrary + StaticLibrary @@ -15,6 +15,7 @@ + @@ -37,6 +38,7 @@ + diff --git a/src/server/sources.inc b/src/server/sources.inc index e5a4adc79..c5e7ccb23 100644 --- a/src/server/sources.inc +++ b/src/server/sources.inc @@ -39,6 +39,7 @@ SOURCES= \ ..\ApiSorter.cpp \ ..\DeviceComm.cpp \ ..\DeviceHandle.cpp \ + ..\ConsoleShimPolicy.cpp \ ..\Entrypoints.cpp \ ..\IoDispatchers.cpp \ ..\IoSorter.cpp \ diff --git a/src/til/ut_til/SizeTests.cpp b/src/til/ut_til/SizeTests.cpp index ce75c5835..880e186ce 100644 --- a/src/til/ut_til/SizeTests.cpp +++ b/src/til/ut_til/SizeTests.cpp @@ -460,6 +460,26 @@ class SizeTests VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); } } + TEST_METHOD(AreaCast) + { + Log::Comment(L"0.) Area of two things that should be in bounds."); + { + const til::size sz{ 5, 10 }; + VERIFY_ARE_EQUAL(static_cast(sz.area()), sz.area()); + } + + Log::Comment(L"1.) Area is out of bounds on multiplication."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ bigSize, bigSize }; + + auto fn = [&]() { + sz.area(); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } TEST_METHOD(CastToCoord) {