Skip to content

Commit

Permalink
Don't put NUL in the buffer in VT mode (#3015)
Browse files Browse the repository at this point in the history
* Potentially fixes #1825

  I haven't had a chance to test this fix on my machine with a CentOS VM quite yet, but this _should_ work

  Also adds a test

* add a comment

* woah hey this test was wrong

* Revert bx.ps1
  • Loading branch information
zadjii-msft authored Oct 3, 2019
1 parent a9f3849 commit a82d6b8
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 3 deletions.
47 changes: 44 additions & 3 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ class ScreenBufferTests

TEST_METHOD(EraseAllTests);

TEST_METHOD(OutputNULTest);

TEST_METHOD(VtResize);
TEST_METHOD(VtResizeComprehensive);
TEST_METHOD(VtResizeDECCOLM);
Expand Down Expand Up @@ -373,14 +375,14 @@ void ScreenBufferTests::TestReverseLineFeed()

cursor.SetPosition({ 0, 5 });
VERIFY_SUCCEEDED(screenInfo.SetViewportOrigin(true, { 0, 5 }, true));
stateMachine.ProcessString(L"ABCDEFGH", 9);
VERIFY_ARE_EQUAL(cursor.GetPosition().X, 9);
stateMachine.ProcessString(L"ABCDEFGH");
VERIFY_ARE_EQUAL(cursor.GetPosition().X, 8);
VERIFY_ARE_EQUAL(cursor.GetPosition().Y, 5);
VERIFY_ARE_EQUAL(screenInfo.GetViewport().Top(), 5);

LOG_IF_FAILED(DoSrvPrivateReverseLineFeed(screenInfo));

VERIFY_ARE_EQUAL(cursor.GetPosition().X, 9);
VERIFY_ARE_EQUAL(cursor.GetPosition().X, 8);
VERIFY_ARE_EQUAL(cursor.GetPosition().Y, 5);
viewport = screenInfo.GetViewport();
VERIFY_ARE_EQUAL(viewport.Top(), 5);
Expand Down Expand Up @@ -816,6 +818,45 @@ void ScreenBufferTests::EraseAllTests()
viewport.BottomInclusive()));
}

void ScreenBufferTests::OutputNULTest()
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& stateMachine = si.GetStateMachine();
auto& cursor = si._textBuffer->GetCursor();

VERIFY_ARE_EQUAL(0, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y);

Log::Comment(NoThrowString().Format(
L"Writing a single NUL"));
stateMachine.ProcessString(L"\0", 1);
VERIFY_ARE_EQUAL(0, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y);

Log::Comment(NoThrowString().Format(
L"Writing many NULs"));
stateMachine.ProcessString(L"\0\0\0\0\0\0\0\0", 8);
VERIFY_ARE_EQUAL(0, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y);

Log::Comment(NoThrowString().Format(
L"Testing a single NUL followed by real text"));
stateMachine.ProcessString(L"\0foo", 4);
VERIFY_ARE_EQUAL(3, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y);

stateMachine.ProcessString(L"\n");
VERIFY_ARE_EQUAL(0, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y);

Log::Comment(NoThrowString().Format(
L"Writing NULs in between other strings"));
stateMachine.ProcessString(L"\0foo\0bar\0", 9);
VERIFY_ARE_EQUAL(6, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y);
}

void ScreenBufferTests::VtResize()
{
// Run this test in isolation - for one reason or another, this breaks other tests.
Expand Down
8 changes: 8 additions & 0 deletions src/terminal/parser/OutputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ ITermDispatch& OutputStateMachineEngine::Dispatch() noexcept
// - true iff we successfully dispatched the sequence.
bool OutputStateMachineEngine::ActionExecute(const wchar_t wch)
{
// microsoft/terminal#1825 - VT applications expect to be able to write NUL
// and have _nothing_ happen. Filter the NULs here, so they don't fill the
// buffer with empty spaces.
if (wch == AsciiChars::NUL)
{
return true;
}

_dispatch->Execute(wch);
_ClearLastChar();

Expand Down

0 comments on commit a82d6b8

Please sign in to comment.