From afcec4d62b86fb9d03132ec64df82974e74fb702 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 9 Sep 2019 16:15:03 -0500 Subject: [PATCH 1/4] 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 --- src/host/ut_host/ScreenBufferTests.cpp | 41 +++++++++++++++++++ .../parser/OutputStateMachineEngine.cpp | 5 +++ 2 files changed, 46 insertions(+) diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 38d4faf2684..46c390b0504 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -105,6 +105,8 @@ class ScreenBufferTests TEST_METHOD(EraseAllTests); + TEST_METHOD(OutputNULTest); + TEST_METHOD(VtResize); TEST_METHOD(VtResizeComprehensive); TEST_METHOD(VtResizeDECCOLM); @@ -802,6 +804,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. diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index a17699a5247..0d0508989f8 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -42,6 +42,11 @@ ITermDispatch& OutputStateMachineEngine::Dispatch() noexcept // - true iff we successfully dispatched the sequence. bool OutputStateMachineEngine::ActionExecute(const wchar_t wch) { + if (wch == AsciiChars::NUL) + { + return true; + } + _dispatch->Execute(wch); _ClearLastChar(); return true; From 9171c44afa3346fd1e6c3d3a7f17474eb4446cd8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 1 Oct 2019 09:37:15 -0500 Subject: [PATCH 2/4] add a comment --- src/terminal/parser/OutputStateMachineEngine.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index 0d0508989f8..619d790bf66 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -42,6 +42,9 @@ 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; From 1b8a93de151134024a114e4c957d31b90e0ca5b1 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 1 Oct 2019 15:55:36 -0500 Subject: [PATCH 3/4] woah hey this test was wrong --- src/host/ut_host/ScreenBufferTests.cpp | 6 +++--- tools/bx.ps1 | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index f6664c1aa32..7abe51fe8fc 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -375,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); diff --git a/tools/bx.ps1 b/tools/bx.ps1 index 4332c7bb643..c49438a2a50 100644 --- a/tools/bx.ps1 +++ b/tools/bx.ps1 @@ -20,6 +20,11 @@ $targets = $Metaproj.Project.Target # For Conhost\Server, this will match: # [Conhost\Server, Conhost\Server:Clean, Conhost\Server:Rebuild, Conhost\Server:Publish] $matchingTargets = $targets | Where-Object { $_.MSBuild.Condition -eq $msBuildCondition } +if ($matchingTargets.length -eq 0) +{ + $conditionNoMeta = "'%(ProjectReference.Identity)' == '$projectPath'" + $matchingTargets = $targets | Where-Object { $_.MSBuild.Condition -eq $conditionNoMeta } +} # Further filter to the targets that dont have a suffix (like ":Clean") $matchingTargets = $matchingTargets | Where-Object { $hasProperty = $_.MsBuild.PSobject.Properties.name -match "Targets" ; return -Not $hasProperty } From ff03b354d879cfcebf7321a361caf9a3ce448230 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett (MSFT)" Date: Tue, 1 Oct 2019 14:45:02 -0700 Subject: [PATCH 4/4] Revert bx.ps1 --- tools/bx.ps1 | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/bx.ps1 b/tools/bx.ps1 index c49438a2a50..4332c7bb643 100644 --- a/tools/bx.ps1 +++ b/tools/bx.ps1 @@ -20,11 +20,6 @@ $targets = $Metaproj.Project.Target # For Conhost\Server, this will match: # [Conhost\Server, Conhost\Server:Clean, Conhost\Server:Rebuild, Conhost\Server:Publish] $matchingTargets = $targets | Where-Object { $_.MSBuild.Condition -eq $msBuildCondition } -if ($matchingTargets.length -eq 0) -{ - $conditionNoMeta = "'%(ProjectReference.Identity)' == '$projectPath'" - $matchingTargets = $targets | Where-Object { $_.MSBuild.Condition -eq $conditionNoMeta } -} # Further filter to the targets that dont have a suffix (like ":Clean") $matchingTargets = $matchingTargets | Where-Object { $hasProperty = $_.MsBuild.PSobject.Properties.name -match "Targets" ; return -Not $hasProperty }