Repository: microsoft/terminal (language: cpp)

Issue:
There may be a bug in unreleased code that throws a PSReadLine exception under certain conditions
### Windows Terminal version

0.0.1.0 (dev version, not before 1.13.10984 preview or after pr #13026)

### Windows build number

10.0.22000.652

### Other Software

PSReadLine 2.1.0, 2.2.5
Powershell Core 7.2.3, 7.3.0-preview.3

PSReadLine: 2.0.0
Windows PowerShell: 5.1.22000.652

_Reproduced in the above versions, other versions have not been tested._

### Steps to reproduce

1 Set Windows Terminal Dev as the default terminal
2 Keep the launch size in the Startup settings as 120 * 30.
3 Then open the terminal by opening powershell.

### Expected Behavior

Open powershell normally.

### Actual Behavior

When the first keystroke is made, powershell throws the following exception:

```pwsh
PS C:\>
Oops, something went wrong.
Please report this bug with ALL the details below, including both the 'Environment' and 'Exception' sections.
Please report on GitHub: https://github.com/PowerShell/PSReadLine/issues/new?template=Bug_Report.yaml
Thank you!

### Environment
PSReadLine: 2.2.5
PowerShell: 7.2.3
OS: Microsoft Windows 10.0.22000
BufferWidth: 120
BufferHeight: 30

Last 1 Keys:

 a

### Exception

System.IO.IOException: 数据无效。
   at System.ConsolePal.SetCursorPosition(Int32 left, Int32 top)
   at System.Console.SetCursorPosition(Int32 left, Int32 top)
   at Microsoft.PowerShell.Internal.VirtualTerminal.SetCursorPosition(Int32 left, Int32 top)
   at Microsoft.PowerShell.PSConsoleReadLine.CalculateWhereAndWhatToRender(Boolean cursorMovedToInitialPos, RenderData renderData, LineInfoForRendering& lineInfoForRendering)
   at Microsoft.PowerShell.PSConsoleReadLine.ReallyRender(RenderData renderData, String defaultColor)
   at Microsoft.PowerShell.PSConsoleReadLine.ForceRender()
   at Microsoft.PowerShell.PSConsoleReadLine.Render()
   at Microsoft.PowerShell.PSConsoleReadLine.Insert(Char c)
   at Microsoft.PowerShell.PSConsoleReadLine.SelfInsert(Nullable`1 key, Object arg)
   at Microsoft.PowerShell.PSConsoleReadLine.ProcessOneKey(PSKeyInfo key, Dictionary`2 dispatchTable, Boolean ignoreIfNoAction, Object arg)
   at Microsoft.PowerShell.PSConsoleReadLine.InputLoop()
   at Microsoft.PowerShell.PSConsoleReadLine.ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsics, CancellationToken cancellationToken, Nullable`1 lastRunStatus)
```

If the keystroke is <kbd>enter</kbd>, and the input timing is not too early, the exception is deferred until another non-<kbd>enter</kbd> keystroke has been made.

```pwsh
Last 6 Keys:

 Enter
 Enter
 Enter
 Enter
 Enter
 Escape

### Exception
...
```
After the exception is thrown, powershell works fine.

There are three points worth noting:
- If you open powershell directly from the terminal, there is no such exception.
- If any item in launch size is changed so that it is not exactly 120*30, there is no such exception.
- There is no such exception in [Microsoft.WindowsTerminalPreview_Win11_1.13.10984.0](https://github.com/microsoft/terminal/releases/download/v1.13.10983.0/Microsoft.WindowsTerminalPreview_Win11_1.13.10984.0_8wekyb3d8bbwe.msixbundle), this exception may be unique to the debug version, or introduced by subsequent commits.

Although the exception is thrown by PSReadLine, it does not occur in other versions of windows terminal with the same settings and opening methods.

_I didn't locate the problem code and don't know if anyone else can reproduce this exception._


Discussion / hints:
If this is a regression, I suspect PR #12972 might to be blame, but I don't see how. Based on the error message, it looks like the `SetConsoleCursorPositionImpl` method must be returning an error of some sort. One possibility would be that the coordinates are out of range, but they'd probably have to be negative, otherwise the dotnet code would throw a different exception (see [here](https://github.com/dotnet/corefx/blob/ec7a4c6e09d655421195580b7eea887000afc367/src/System.Console/src/System/ConsolePal.Windows.cs#L827-L831)). The more likely cause would be the `SetViewportOrigin` call is producing an out of range viewport, but I can't figure out how that would happen unless the viewport dimensions started off incorrect.

Hopefully there is someone else in a better position to debug this than me. I don't have Windows 11, so I can't change the default terminal to test.
I can totally reproduce this! _What._
```
OpenConsole!ApiRoutines::SetConsoleCursorPositionImpl+0x14a:
00007ff6`9f9a791a 41b101          mov     r9b,1
0:005> dx newWindowOrigin
newWindowOrigin                 : {0,-23} [Type: _COORD]
```

```
    // see if new window origin would extend window beyond extent of screen buffer
    const auto coordScreenBufferSize = GetBufferSize().Dimensions();
    if (NewWindow.Left < 0 ||
        NewWindow.Top < 0 ||
        NewWindow.Right < 0 ||
        NewWindow.Bottom < 0 ||
        NewWindow.Right >= coordScreenBufferSize.X ||
        NewWindow.Bottom >= coordScreenBufferSize.Y)
    {
===>    return STATUS_INVALID_PARAMETER;
    }
```

```
0:005> dx NewWindow.Top
NewWindow.Top    : -23 [Type: short]
```
`currentViewport.Top` is already -23 here:

```
        newWindowOrigin.Y = currentViewport.Top + delta.Y;
        // SetViewportOrigin will worry about clamping these values to the
```

and it comes from ...

```
        const auto currentViewport = buffer.GetVirtualViewport().ToInclusive();
```

calculated here:

```
Viewport SCREEN_INFORMATION::GetVirtualViewport() const noexcept
{
==>    const short newTop = _virtualBottom - _viewport.Height() + 1;
    return Viewport::FromDimensions({ _viewport.Left(), newTop }, _viewport.Dimensions());
}
```

with these values

```
0:007> dx this->_virtualBottom
this->_virtualBottom : 6 [Type: short]

0:007> dx this->_viewport._sr,!
this->_viewport._sr,!                 : {LT(0, 0) RB(119, 29) In:[120 x 30] Ex:[119 x 29]} [Type: _SMALL_RECT]
    [+0x000] Left             : 0 [Type: short]
    [+0x002] Top              : 0 [Type: short]
    [+0x004] Right            : 119 [Type: short]
    [+0x006] Bottom           : 29 [Type: short]
```

Good intuition, @j4james!
I have an idea of what's causing this. If we get a `ResizeWithReflow` when we're somewhere near the start of the buffer, the `_virtualBottom` can end up being less than the viewport height. See here:

https://github.com/microsoft/terminal/blob/f4e0d9f2bd1f5da3a6c378453b90320fb6b1c542/src/host/screenInfo.cpp#L1467-L1471

In some cases, `ResizeWithReflow` is followed by some other call that will correct the `_virtualBottom`, but not always. For example, I was able to trigger the bug with a `SetConsoleScreenBufferInfoEx` call. I could fix that by changing the `_virtualBottom` calculation to take the viewport height into account, like this:

```cpp
const auto cursorRow = newTextBuffer->GetCursor().GetPosition().Y;
const auto lastNonSpaceRow = newTextBuffer->GetLastNonSpaceCharacter().Y;
const auto viewportBottom = gsl::narrow_cast<short>(_viewport.Height() - 1);
_virtualBottom = std::max({ cursorRow, lastNonSpaceRow, viewportBottom });
```

Assuming the issue you are seeing is caused by the same thing, then hopefully that'll fix it for you too.

I need to get to sleep now, and I've got to work tomorrow, but I can try and put together a PR tomorrow evening once I've had a chance to test some more. But if you're in a rush to get this fixed, I think the code above is probably OK.

Fix patch:
diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp
index e4130641172..05ddded0f12 100644
--- a/src/host/screenInfo.cpp
+++ b/src/host/screenInfo.cpp
@@ -1465,10 +1465,13 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
     if (SUCCEEDED(hr))
     {
         // Make sure the new virtual bottom is far enough down to include both
-        // the cursor row and the last non-space row.
+        // the cursor row and the last non-space row. It also shouldn't be less
+        // than the height of the viewport, otherwise the top of the virtual
+        // viewport would end up negative.
         const auto cursorRow = newTextBuffer->GetCursor().GetPosition().Y;
         const auto lastNonSpaceRow = newTextBuffer->GetLastNonSpaceCharacter().Y;
-        _virtualBottom = std::max({ cursorRow, lastNonSpaceRow });
+        const auto viewportBottom = gsl::narrow_cast<short>(_viewport.Height() - 1);
+        _virtualBottom = std::max({ cursorRow, lastNonSpaceRow, viewportBottom });
 
         // Adjust the viewport so the cursor doesn't wildly fly off up or down.
         const auto sCursorHeightInViewportAfter = cursorRow - _viewport.Top();
diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp
index 27c887d699c..2bb59a62c77 100644
--- a/src/host/ut_host/ScreenBufferTests.cpp
+++ b/src/host/ut_host/ScreenBufferTests.cpp
@@ -230,6 +230,7 @@ class ScreenBufferTests
     TEST_METHOD(UpdateVirtualBottomWithSetConsoleCursorPosition);
     TEST_METHOD(UpdateVirtualBottomAfterInternalSetViewportSize);
     TEST_METHOD(UpdateVirtualBottomAfterResizeWithReflow);
+    TEST_METHOD(DontShrinkVirtualBottomDuringResizeWithReflowAtTop);
     TEST_METHOD(DontChangeVirtualBottomWithOffscreenLinefeed);
     TEST_METHOD(DontChangeVirtualBottomAfterResizeWindow);
     TEST_METHOD(DontChangeVirtualBottomWithMakeCursorVisible);
@@ -6292,6 +6293,35 @@ void ScreenBufferTests::UpdateVirtualBottomAfterResizeWithReflow()
     VERIFY_IS_GREATER_THAN_OR_EQUAL(si._virtualBottom, lastNonSpaceRow);
 }
 
+void ScreenBufferTests::DontShrinkVirtualBottomDuringResizeWithReflowAtTop()
+{
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& cursor = si.GetTextBuffer().GetCursor();
+    auto& stateMachine = si.GetStateMachine();
+
+    Log::Comment(L"Make sure the viewport is at the top of the buffer");
+    const auto bufferTop = COORD{ 0, 0 };
+    VERIFY_SUCCEEDED(si.SetViewportOrigin(true, bufferTop, true));
+    VERIFY_ARE_EQUAL(bufferTop, si.GetViewport().Origin());
+
+    Log::Comment(L"Make sure the virtual bottom is at the bottom of the viewport");
+    VERIFY_ARE_EQUAL(si.GetViewport().BottomInclusive(), si._virtualBottom);
+
+    Log::Comment(L"Make sure the cursor is at the top of the buffer");
+    stateMachine.ProcessString(L"\033[H");
+    VERIFY_ARE_EQUAL(bufferTop, cursor.GetPosition());
+
+    Log::Comment(L"Shrink the viewport width by a half");
+    auto bufferSize = si.GetTextBuffer().GetSize().Dimensions();
+    bufferSize.X /= 2;
+    VERIFY_NT_SUCCESS(si.ResizeWithReflow(bufferSize));
+
+    Log::Comment(L"Confirm that the virtual bottom is still at the bottom of the viewport");
+    VERIFY_ARE_EQUAL(si.GetViewport().BottomInclusive(), si._virtualBottom);
+}
+
 void ScreenBufferTests::DontChangeVirtualBottomWithOffscreenLinefeed()
 {
     auto& g = ServiceLocator::LocateGlobals();


Test patch: