From 26f4b0eacbe46e47d89960064edb161f18471d21 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Mon, 19 Jul 2021 19:51:47 +0000 Subject: [PATCH 01/11] Merged PR 6269653: Slim down conhost's dependency on shcore An internal change CommandLineToArgVW to an apiset. Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev a71b943e06c009085d6a2bb886dd50c2d0d2c276 Related work items: MSFT-32178383 --- src/host/sources.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/host/sources.inc b/src/host/sources.inc index 3ee3b70c3..e3115bda5 100644 --- a/src/host/sources.inc +++ b/src/host/sources.inc @@ -134,6 +134,7 @@ CRTLIBS = \ TARGETLIBS = \ $(TARGETLIBS) \ + $(MINCORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\api-ms-win-core-commandlinetoargv-l1.lib \ $(ONECORE_INTERNAL_SDK_LIB_PATH)\onecoreuuid.lib \ $(ONECOREUAP_INTERNAL_SDK_LIB_PATH)\onecoreuapuuid.lib \ $(ONECORE_INTERNAL_PRIV_SDK_LIB_VPATH_L)\onecore_internal.lib \ From dfda41074d8a3735c42a5ee17e1b4997571e00f9 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Mon, 19 Jul 2021 20:02:20 +0000 Subject: [PATCH 02/11] Merged PR 6274354: [Git2Git] Fix unbound read of cooked read buffer Fix unbound read of cooked read buffer Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 756c8dcd4cf9551f5bf090b98bf3fba5498f8eff Related work items: MSFT-32957145 --- src/host/readDataCooked.cpp | 10 +++++----- src/host/tracing.cpp | 5 +++-- src/host/tracing.hpp | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index 458f351b3..a85b1bcc3 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -1008,13 +1008,13 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline { // Figure out where real string ends (at carriage return or end of buffer). PWCHAR StringPtr = _backupLimit; - size_t StringLength = _bytesRead; + size_t StringLength = _bytesRead / sizeof(WCHAR); bool FoundCR = false; - for (size_t i = 0; i < (_bytesRead / sizeof(WCHAR)); i++) + for (size_t i = 0; i < StringLength; i++) { if (*StringPtr++ == UNICODE_CARRIAGERETURN) { - StringLength = i * sizeof(WCHAR); + StringLength = i; FoundCR = true; break; } @@ -1026,11 +1026,11 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline { // add to command line recall list if we have a history list. CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); - LOG_IF_FAILED(_commandHistory->Add({ _backupLimit, StringLength / sizeof(wchar_t) }, + LOG_IF_FAILED(_commandHistory->Add({ _backupLimit, StringLength }, WI_IsFlagSet(gci.Flags, CONSOLE_HISTORY_NODUP))); } - Tracing::s_TraceCookedRead(_backupLimit); + Tracing::s_TraceCookedRead(_backupLimit, base::saturated_cast(StringLength)); // check for alias ProcessAliases(LineCount); diff --git a/src/host/tracing.cpp b/src/host/tracing.cpp index 6e20fe33e..ebd323a0e 100644 --- a/src/host/tracing.cpp +++ b/src/host/tracing.cpp @@ -405,12 +405,13 @@ void Tracing::s_TraceInputRecord(const INPUT_RECORD& inputRecord) } } -void Tracing::s_TraceCookedRead(_In_z_ const wchar_t* pwszCookedBuffer) +void Tracing::s_TraceCookedRead(_In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength) { TraceLoggingWrite( g_hConhostV2EventTraceProvider, "CookedRead", - TraceLoggingWideString(pwszCookedBuffer, "ReadBuffer"), + TraceLoggingCountedWideString(pwchCookedBuffer, cchCookedBufferLength, "ReadBuffer"), + TraceLoggingULong(cchCookedBufferLength, "ReadBufferLength"), TraceLoggingKeyword(TIL_KEYWORD_TRACE), TraceLoggingKeyword(TraceKeywords::CookedRead)); } diff --git a/src/host/tracing.hpp b/src/host/tracing.hpp index 36ae55423..d4bcdf701 100644 --- a/src/host/tracing.hpp +++ b/src/host/tracing.hpp @@ -62,7 +62,7 @@ public: static void s_TraceWindowMessage(const MSG& msg); static void s_TraceInputRecord(const INPUT_RECORD& inputRecord); - static void s_TraceCookedRead(_In_z_ const wchar_t* pwszCookedBuffer); + static void Tracing::s_TraceCookedRead(_In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength); static void __stdcall TraceFailure(const wil::FailureInfo& failure) noexcept; From 01b519527548167e96ac5983e1881829429bba6a Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Wed, 21 Jul 2021 18:41:37 +0000 Subject: [PATCH 03/11] Merged PR 6277720: [Git2Git] Merged PR 6275065: Trace console attach/detatch As identified by Michael Niksa, our MDE heuristics for understanding relationship between conhost and related processes was incorrect. Exposing trace here to assist in correlation. Related work items: MSFT-32957145 Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 3c886da66d77d1aa36b52794929e388af292539c --- src/host/telemetry.cpp | 5 +++++ src/host/telemetry.hpp | 2 ++ src/host/tracing.cpp | 33 +++++++++++++++++++++++++++++++++ src/host/tracing.hpp | 3 ++- src/server/IoDispatchers.cpp | 4 ++++ src/server/ProcessHandle.cpp | 7 +++++++ src/server/ProcessHandle.h | 2 ++ 7 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/host/telemetry.cpp b/src/host/telemetry.cpp index 7b4ce210b..59080d3a9 100644 --- a/src/host/telemetry.cpp +++ b/src/host/telemetry.cpp @@ -587,3 +587,8 @@ void Telemetry::LogRipMessage(_In_z_ const char* pszMessage, ...) const TraceLoggingString(szMessageEvaluated, "Message")); } } + +bool Telemetry::IsUserInteractive() +{ + return _fUserInteractiveForTelemetry; +} diff --git a/src/host/telemetry.hpp b/src/host/telemetry.hpp index 00f9a0d92..2092115aa 100644 --- a/src/host/telemetry.hpp +++ b/src/host/telemetry.hpp @@ -51,6 +51,8 @@ public: void LogRipMessage(_In_z_ const char* pszMessage, ...) const; + bool IsUserInteractive(); + // Names are from the external API call names. Note that some names can be different // than the internal API calls. // Don't worry about the following APIs, because they are external to our conhost codebase and hard to track through diff --git a/src/host/tracing.cpp b/src/host/tracing.cpp index ebd323a0e..91efc57fc 100644 --- a/src/host/tracing.cpp +++ b/src/host/tracing.cpp @@ -21,6 +21,7 @@ enum TraceKeywords API = 0x400, UIA = 0x800, CookedRead = 0x1000, + ConsoleAttachDetach = 0x2000, All = 0x1FFF }; DEFINE_ENUM_FLAG_OPERATORS(TraceKeywords); @@ -416,6 +417,38 @@ void Tracing::s_TraceCookedRead(_In_reads_(cchCookedBufferLength) const wchar_t* TraceLoggingKeyword(TraceKeywords::CookedRead)); } +void Tracing::s_TraceConsoleAttachDetach(_In_ const ConsoleProcessHandle* pConsoleProcessHandle, _In_ bool bIsAttach) +{ + FILETIME ftCreationTime, ftDummyTime = { 0 }; + ULARGE_INTEGER creationTime = { 0 }; + + if (TraceLoggingProviderEnabled(g_hConhostV2EventTraceProvider, + WINEVENT_LEVEL_LOG_ALWAYS, + TraceKeywords::ConsoleAttachDetach)) { + + if (::GetProcessTimes(pConsoleProcessHandle->GetRawHandle(), + &ftCreationTime, + &ftDummyTime, + &ftDummyTime, + &ftDummyTime)) { + creationTime.HighPart = ftCreationTime.dwHighDateTime; + creationTime.LowPart = ftCreationTime.dwLowDateTime; + } + + bool bIsUserInteractive = Telemetry::Instance().IsUserInteractive(); + + TraceLoggingWrite( + g_hConhostV2EventTraceProvider, + "ConsoleAttachDetach", + TraceLoggingUInt32(pConsoleProcessHandle->dwProcessId, "ProcessId"), + TraceLoggingUInt64(creationTime.QuadPart, "ProcessCreationTime"), + TraceLoggingBool(bIsAttach, "IsAttach"), + TraceLoggingBool(bIsUserInteractive, "IsUserInteractive"), + TraceLoggingKeyword(TIL_KEYWORD_TRACE), + TraceLoggingKeyword(TraceKeywords::ConsoleAttachDetach)); + } +} + void __stdcall Tracing::TraceFailure(const wil::FailureInfo& failure) noexcept { TraceLoggingWrite( diff --git a/src/host/tracing.hpp b/src/host/tracing.hpp index d4bcdf701..d2f8eb150 100644 --- a/src/host/tracing.hpp +++ b/src/host/tracing.hpp @@ -62,7 +62,8 @@ public: static void s_TraceWindowMessage(const MSG& msg); static void s_TraceInputRecord(const INPUT_RECORD& inputRecord); - static void Tracing::s_TraceCookedRead(_In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength); + static void s_TraceCookedRead(_In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength); + static void s_TraceConsoleAttachDetach(_In_ const ConsoleProcessHandle* pConsoleProcessHandle, _In_ bool bIsAttach); static void __stdcall TraceFailure(const wil::FailureInfo& failure) noexcept; diff --git a/src/server/IoDispatchers.cpp b/src/server/IoDispatchers.cpp index 7ae2e954c..4044c41ca 100644 --- a/src/server/IoDispatchers.cpp +++ b/src/server/IoDispatchers.cpp @@ -431,6 +431,8 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API CommandHistory::s_Free((HANDLE)ProcessData); gci.ProcessHandleList.FreeProcessData(ProcessData); } + + Tracing::s_TraceConsoleAttachDetach(ProcessData, true); UnlockConsole(); @@ -470,6 +472,8 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleClientDisconnectRoutine(_In_ PCONSOLE_API pNotifier->NotifyConsoleEndApplicationEvent(pProcessData->dwProcessId); } + Tracing::s_TraceConsoleAttachDetach(pProcessData, false); + LOG_IF_FAILED(RemoveConsole(pProcessData)); pMessage->SetReplyStatus(STATUS_SUCCESS); diff --git a/src/server/ProcessHandle.cpp b/src/server/ProcessHandle.cpp index 14d9f7603..1b5647bcd 100644 --- a/src/server/ProcessHandle.cpp +++ b/src/server/ProcessHandle.cpp @@ -65,3 +65,10 @@ const ConsoleShimPolicy ConsoleProcessHandle::GetShimPolicy() const { return _shimPolicy; } + +// Routine Description: +// - Retrieves the raw process handle +const HANDLE ConsoleProcessHandle::GetRawHandle() const +{ + return _hProcess.get(); +} diff --git a/src/server/ProcessHandle.h b/src/server/ProcessHandle.h index eeafaebc9..e73706549 100644 --- a/src/server/ProcessHandle.h +++ b/src/server/ProcessHandle.h @@ -40,6 +40,8 @@ public: const ConsoleProcessPolicy GetPolicy() const; const ConsoleShimPolicy GetShimPolicy() const; + const HANDLE GetRawHandle() const; + CD_CONNECTION_INFORMATION GetConnectionInformation(IDeviceComm* deviceComm) const; private: From 184919fb24186b161b40be8e004bf2c0d99ff1ee Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Thu, 22 Jul 2021 13:39:34 +0000 Subject: [PATCH 04/11] Merged PR 6285331: [Git2Git] Merged PR 6278637: Expose attached client process context to cooked read trace Related work items: MSFT-32957145 Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev bdb25dc99dcb2f1ee483dffe883d0178ea9d18dc --- src/host/readDataCooked.cpp | 17 +++++++----- src/host/readDataCooked.hpp | 8 +++--- src/host/stream.cpp | 5 ++-- src/host/tracing.cpp | 49 ++++++++++++++--------------------- src/host/tracing.hpp | 4 +-- src/inc/test/CommonState.hpp | 4 +-- src/server/ApiDispatchers.cpp | 2 +- src/server/ProcessHandle.cpp | 28 +++++++++++++++++++- src/server/ProcessHandle.h | 4 +++ 9 files changed, 73 insertions(+), 48 deletions(-) diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index a85b1bcc3..169b09731 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -34,13 +34,13 @@ using Microsoft::Console::Interactivity::ServiceLocator; // - OriginalCursorPosition - // - NumberOfVisibleChars // - CtrlWakeupMask - Special client parameter to interrupt editing, end the wait, and return control to the client application -// - CommandHistory - // - Echo - // - InsertMode - // - Processed - // - Line - // - pTempHandle - A handle to the output buffer to prevent it from being destroyed while we're using it to present 'edit line' text. // - initialData - any text data that should be prepopulated into the buffer +// - pClientProcess - Attached process handle object // Return Value: // - THROW: Throws E_INVALIDARG for invalid pointers. COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, @@ -49,9 +49,9 @@ COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, _In_ size_t UserBufferSize, _In_ PWCHAR UserBuffer, _In_ ULONG CtrlWakeupMask, - _In_ CommandHistory* CommandHistory, - const std::wstring_view exeName, - const std::string_view initialData) : + _In_ const std::wstring_view exeName, + _In_ const std::string_view initialData, + _In_ ConsoleProcessHandle* const pClientProcess) : ReadData(pInputBuffer, pInputReadHandleData), _screenInfo{ screenInfo }, _bytesRead{ 0 }, @@ -62,7 +62,7 @@ COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, _exeName{ exeName }, _pdwNumBytes{ nullptr }, - _commandHistory{ CommandHistory }, + _commandHistory{ CommandHistory::s_Find((HANDLE)pClientProcess) }, _controlKeyState{ 0 }, _ctrlWakeupMask{ CtrlWakeupMask }, _visibleCharCount{ 0 }, @@ -73,7 +73,8 @@ COOKED_READ_DATA::COOKED_READ_DATA(_In_ InputBuffer* const pInputBuffer, _lineInput{ WI_IsFlagSet(pInputBuffer->InputMode, ENABLE_LINE_INPUT) }, _processedInput{ WI_IsFlagSet(pInputBuffer->InputMode, ENABLE_PROCESSED_INPUT) }, _insertMode{ ServiceLocator::LocateGlobals().getConsoleInformation().GetInsertMode() }, - _unicode{ false } + _unicode{ false }, + _clientProcess{ pClientProcess } { #ifndef UNIT_TESTING THROW_IF_FAILED(screenInfo.GetMainBuffer().AllocateIoHandle(ConsoleHandleData::HandleType::Output, @@ -1030,7 +1031,9 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline WI_IsFlagSet(gci.Flags, CONSOLE_HISTORY_NODUP))); } - Tracing::s_TraceCookedRead(_backupLimit, base::saturated_cast(StringLength)); + Tracing::s_TraceCookedRead(_clientProcess, + _backupLimit, + base::saturated_cast(StringLength)); // check for alias ProcessAliases(LineCount); diff --git a/src/host/readDataCooked.hpp b/src/host/readDataCooked.hpp index 47f7bb2e8..4eccee7f5 100644 --- a/src/host/readDataCooked.hpp +++ b/src/host/readDataCooked.hpp @@ -39,9 +39,9 @@ public: _In_ size_t UserBufferSize, _In_ PWCHAR UserBuffer, _In_ ULONG CtrlWakeupMask, - _In_ CommandHistory* CommandHistory, - const std::wstring_view exeName, - const std::string_view initialData); + _In_ const std::wstring_view exeName, + _In_ const std::string_view initialData, + _In_ ConsoleProcessHandle* const pClientProcess); ~COOKED_READ_DATA() override; COOKED_READ_DATA(COOKED_READ_DATA&&) = default; @@ -156,6 +156,8 @@ private: bool _insertMode; bool _unicode; + ConsoleProcessHandle* const _clientProcess; + [[nodiscard]] NTSTATUS _readCharInputLoop(const bool isUnicode, size_t& numBytes) noexcept; [[nodiscard]] NTSTATUS _handlePostCharInputLoop(const bool isUnicode, size_t& numBytes, ULONG& controlKeyState) noexcept; diff --git a/src/host/stream.cpp b/src/host/stream.cpp index 4b0ba6c67..12806de01 100644 --- a/src/host/stream.cpp +++ b/src/host/stream.cpp @@ -471,7 +471,6 @@ size_t RetrieveNumberOfSpaces(_In_ SHORT sOriginalCursorPositionX, RETURN_HR_IF(E_FAIL, !gci.HasActiveOutputBuffer()); SCREEN_INFORMATION& screenInfo = gci.GetActiveOutputBuffer(); - CommandHistory* const pCommandHistory = CommandHistory::s_Find(processData); try { @@ -481,9 +480,9 @@ size_t RetrieveNumberOfSpaces(_In_ SHORT sOriginalCursorPositionX, buffer.size_bytes(), // UserBufferSize reinterpret_cast(buffer.data()), // UserBuffer ctrlWakeupMask, // CtrlWakeupMask - pCommandHistory, // CommandHistory exeName, // exe name - initialData); + initialData, + reinterpret_cast(processData)); //pClientProcess gci.SetCookedReadData(cookedReadData.get()); bytesRead = buffer.size_bytes(); // This parameter on the way in is the size to read, on the way out, it will be updated to what is actually read. diff --git a/src/host/tracing.cpp b/src/host/tracing.cpp index 91efc57fc..ccd8716c7 100644 --- a/src/host/tracing.cpp +++ b/src/host/tracing.cpp @@ -406,42 +406,33 @@ void Tracing::s_TraceInputRecord(const INPUT_RECORD& inputRecord) } } -void Tracing::s_TraceCookedRead(_In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength) -{ - TraceLoggingWrite( - g_hConhostV2EventTraceProvider, - "CookedRead", - TraceLoggingCountedWideString(pwchCookedBuffer, cchCookedBufferLength, "ReadBuffer"), - TraceLoggingULong(cchCookedBufferLength, "ReadBufferLength"), - TraceLoggingKeyword(TIL_KEYWORD_TRACE), - TraceLoggingKeyword(TraceKeywords::CookedRead)); +void Tracing::s_TraceCookedRead(_In_ ConsoleProcessHandle* const pConsoleProcessHandle, _In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength) +{ + if (TraceLoggingProviderEnabled(g_hConhostV2EventTraceProvider, 0, TraceKeywords::CookedRead)) { + + TraceLoggingWrite( + g_hConhostV2EventTraceProvider, + "CookedRead", + TraceLoggingCountedWideString(pwchCookedBuffer, cchCookedBufferLength, "ReadBuffer"), + TraceLoggingULong(cchCookedBufferLength, "ReadBufferLength"), + TraceLoggingUInt32(pConsoleProcessHandle->dwProcessId, "AttachedProcessId"), + TraceLoggingUInt64(pConsoleProcessHandle->GetProcessCreationTime(), "AttachedProcessCreationTime"), + TraceLoggingKeyword(TIL_KEYWORD_TRACE), + TraceLoggingKeyword(TraceKeywords::CookedRead)); + } } -void Tracing::s_TraceConsoleAttachDetach(_In_ const ConsoleProcessHandle* pConsoleProcessHandle, _In_ bool bIsAttach) -{ - FILETIME ftCreationTime, ftDummyTime = { 0 }; - ULARGE_INTEGER creationTime = { 0 }; - - if (TraceLoggingProviderEnabled(g_hConhostV2EventTraceProvider, - WINEVENT_LEVEL_LOG_ALWAYS, - TraceKeywords::ConsoleAttachDetach)) { - - if (::GetProcessTimes(pConsoleProcessHandle->GetRawHandle(), - &ftCreationTime, - &ftDummyTime, - &ftDummyTime, - &ftDummyTime)) { - creationTime.HighPart = ftCreationTime.dwHighDateTime; - creationTime.LowPart = ftCreationTime.dwLowDateTime; - } - +void Tracing::s_TraceConsoleAttachDetach(_In_ ConsoleProcessHandle* const pConsoleProcessHandle, _In_ bool bIsAttach) +{ + if (TraceLoggingProviderEnabled(g_hConhostV2EventTraceProvider, 0, TraceKeywords::ConsoleAttachDetach)) { + bool bIsUserInteractive = Telemetry::Instance().IsUserInteractive(); TraceLoggingWrite( g_hConhostV2EventTraceProvider, "ConsoleAttachDetach", - TraceLoggingUInt32(pConsoleProcessHandle->dwProcessId, "ProcessId"), - TraceLoggingUInt64(creationTime.QuadPart, "ProcessCreationTime"), + TraceLoggingUInt32(pConsoleProcessHandle->dwProcessId, "AttachedProcessId"), + TraceLoggingUInt64(pConsoleProcessHandle->GetProcessCreationTime(), "AttachedProcessCreationTime"), TraceLoggingBool(bIsAttach, "IsAttach"), TraceLoggingBool(bIsUserInteractive, "IsUserInteractive"), TraceLoggingKeyword(TIL_KEYWORD_TRACE), diff --git a/src/host/tracing.hpp b/src/host/tracing.hpp index d2f8eb150..a32fc6c3f 100644 --- a/src/host/tracing.hpp +++ b/src/host/tracing.hpp @@ -62,8 +62,8 @@ public: static void s_TraceWindowMessage(const MSG& msg); static void s_TraceInputRecord(const INPUT_RECORD& inputRecord); - static void s_TraceCookedRead(_In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength); - static void s_TraceConsoleAttachDetach(_In_ const ConsoleProcessHandle* pConsoleProcessHandle, _In_ bool bIsAttach); + static void s_TraceCookedRead(_In_ ConsoleProcessHandle* const pConsoleProcessHandle, _In_reads_(cchCookedBufferLength) const wchar_t* pwchCookedBuffer, _In_ ULONG cchCookedBufferLength); + static void s_TraceConsoleAttachDetach(_In_ ConsoleProcessHandle* const pConsoleProcessHandle, _In_ bool bIsAttach); static void __stdcall TraceFailure(const wil::FailureInfo& failure) noexcept; diff --git a/src/inc/test/CommonState.hpp b/src/inc/test/CommonState.hpp index 80382837c..6319bb902 100644 --- a/src/inc/test/CommonState.hpp +++ b/src/inc/test/CommonState.hpp @@ -132,9 +132,9 @@ public: 0, nullptr, 0, - nullptr, L"", - initialData); + initialData, + nullptr); gci.SetCookedReadData(readData); } diff --git a/src/server/ApiDispatchers.cpp b/src/server/ApiDispatchers.cpp index 87e64b18f..fff5fc354 100644 --- a/src/server/ApiDispatchers.cpp +++ b/src/server/ApiDispatchers.cpp @@ -312,7 +312,7 @@ } CATCH_RETURN(); - // ReadConsole needs this to get the command history list associated with an attached process, but it can be an opaque value. + // ReadConsole needs this to get details associated with an attached process (such as the command history list, telemetry metadata). HANDLE const hConsoleClient = (HANDLE)m->GetProcessHandle(); // ReadConsole needs this to store context information across "processed reads" e.g. reads on the same handle diff --git a/src/server/ProcessHandle.cpp b/src/server/ProcessHandle.cpp index 1b5647bcd..f4d0a1d7f 100644 --- a/src/server/ProcessHandle.cpp +++ b/src/server/ProcessHandle.cpp @@ -27,7 +27,8 @@ ConsoleProcessHandle::ConsoleProcessHandle(const DWORD dwProcessId, FALSE, dwProcessId))), _policy(ConsoleProcessPolicy::s_CreateInstance(_hProcess.get())), - _shimPolicy(ConsoleShimPolicy::s_CreateInstance(_hProcess.get())) + _shimPolicy(ConsoleShimPolicy::s_CreateInstance(_hProcess.get())), + _processCreationTime(0) { if (nullptr != _hProcess.get()) { @@ -72,3 +73,28 @@ const HANDLE ConsoleProcessHandle::GetRawHandle() const { return _hProcess.get(); } + +// Routine Description: +// - Retrieves the process creation time (currently used in telemetry traces) +// - The creation time is lazily populated on first call +const ULONG64 ConsoleProcessHandle::GetProcessCreationTime() const +{ + if (_processCreationTime == 0 && _hProcess != nullptr) { + + FILETIME ftCreationTime, ftDummyTime = { 0 }; + ULARGE_INTEGER creationTime = { 0 }; + + if (::GetProcessTimes(_hProcess.get(), + &ftCreationTime, + &ftDummyTime, + &ftDummyTime, + &ftDummyTime)) { + creationTime.HighPart = ftCreationTime.dwHighDateTime; + creationTime.LowPart = ftCreationTime.dwLowDateTime; + } + + _processCreationTime = creationTime.QuadPart; + } + + return _processCreationTime; +} diff --git a/src/server/ProcessHandle.h b/src/server/ProcessHandle.h index e73706549..8f35935d9 100644 --- a/src/server/ProcessHandle.h +++ b/src/server/ProcessHandle.h @@ -44,6 +44,8 @@ public: CD_CONNECTION_INFORMATION GetConnectionInformation(IDeviceComm* deviceComm) const; + const ULONG64 GetProcessCreationTime() const; + private: ConsoleProcessHandle(const DWORD dwProcessId, const DWORD dwThreadId, @@ -58,6 +60,8 @@ private: ULONG const _ulProcessGroupId; wil::unique_handle const _hProcess; + mutable ULONG64 _processCreationTime; + const ConsoleProcessPolicy _policy; const ConsoleShimPolicy _shimPolicy; From 431d51de4c7219aa49e12f5468add009a517949b Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Tue, 21 Sep 2021 20:49:55 +0000 Subject: [PATCH 05/11] Merged PR 6286783: Release unneeded memory more eagerly from conhost This is equivalent to commit 8779249b1, but reflected from the OS repository. Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev a4d67e9b05039f365a1a0c58e9c63474c58073a1 Related work items: MSFT-34777060 --- src/inc/til/u8u16convert.h | 8 ++++++++ src/server/ApiMessage.cpp | 18 ++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/inc/til/u8u16convert.h b/src/inc/til/u8u16convert.h index b4d3cd9f8..af6e4f9b7 100644 --- a/src/inc/til/u8u16convert.h +++ b/src/inc/til/u8u16convert.h @@ -55,6 +55,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" RETURN_HR_IF(E_ABORT, !base::CheckAdd(in.length(), _partialsLen).AssignIfValid(&capacity)); _buffer.clear(); + + // If we were previously called with a huge buffer we have an equally large _buffer. + // We shouldn't just keep this huge buffer around, if no one needs it anymore. + if (_buffer.capacity() > 16 * 1024 && (_buffer.capacity() >> 1) > capacity) + { + _buffer.shrink_to_fit(); + } + _buffer.reserve(capacity); // copy UTF-8 code units that were remaining from the previous call (if any) diff --git a/src/server/ApiMessage.cpp b/src/server/ApiMessage.cpp index 598111c88..9287965d3 100644 --- a/src/server/ApiMessage.cpp +++ b/src/server/ApiMessage.cpp @@ -104,7 +104,14 @@ try { RETURN_HR_IF(E_FAIL, State.ReadOffset > Descriptor.InputSize); - ULONG const cbReadSize = Descriptor.InputSize - State.ReadOffset; + const ULONG cbReadSize = Descriptor.InputSize - State.ReadOffset; + + // If we were previously called with a huge buffer we have an equally large _inputBuffer. + // We shouldn't just keep this huge buffer around, if no one needs it anymore. + if (_inputBuffer.capacity() > 16 * 1024 && (_inputBuffer.capacity() >> 1) > cbReadSize) + { + _inputBuffer.shrink_to_fit(); + } _inputBuffer.resize(cbReadSize); @@ -145,10 +152,17 @@ try ULONG cbWriteSize = Descriptor.OutputSize - State.WriteOffset; RETURN_IF_FAILED(ULongMult(cbWriteSize, cbFactor, &cbWriteSize)); + // If we were previously called with a huge buffer we have an equally large _outputBuffer. + // We shouldn't just keep this huge buffer around, if no one needs it anymore. + if (_outputBuffer.capacity() > 16 * 1024 && (_outputBuffer.capacity() >> 1) > cbWriteSize) + { + _outputBuffer.shrink_to_fit(); + } + _outputBuffer.resize(cbWriteSize); // 0 it out. - std::fill(_outputBuffer.begin(), _outputBuffer.end(), (BYTE)0); + std::fill_n(_outputBuffer.data(), _outputBuffer.size(), BYTE(0)); State.OutputBuffer = _outputBuffer.data(); State.OutputBufferSize = cbWriteSize; From d26353bb32587a38d595ee75fa0b5a737dc3123d Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Tue, 21 Sep 2021 20:51:44 +0000 Subject: [PATCH 06/11] Merged PR 6303540: Prepare command history before COOKED_READ in tests [Git2Git] Merged PR 6303114: Prepare command history before COOKED_READ in tests PR !6278637 introduced a dependency from COOKED_READ_DATA on the ability to locate a command history for a process handle (here, `nullptr`). The tests were blowing up because no such history had been allocated. Closes MSFT-34812916 Closes MSFT-34813774 Closes MSFT-34815941 Closes MSFT-34817558 Closes MSFT-34817540 (Watson) Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev f7517e686447fc0469f6b83df19760dc3dafd577 --- src/host/ut_host/ApiRoutinesTests.cpp | 10 ++++++++++ src/host/ut_host/CommandLineTests.cpp | 3 ++- src/host/ut_host/CommandListPopupTests.cpp | 3 ++- src/host/ut_host/CommandNumberPopupTests.cpp | 3 ++- src/host/ut_host/CopyFromCharPopupTests.cpp | 9 +++++++++ src/host/ut_host/CopyToCharPopupTests.cpp | 3 ++- src/host/ut_host/SelectionTests.cpp | 9 +++++++++ 7 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/host/ut_host/ApiRoutinesTests.cpp b/src/host/ut_host/ApiRoutinesTests.cpp index bf0f7ed36..cf0e8cd18 100644 --- a/src/host/ut_host/ApiRoutinesTests.cpp +++ b/src/host/ut_host/ApiRoutinesTests.cpp @@ -27,6 +27,7 @@ class ApiRoutinesTests ApiRoutines _Routines; IApiRoutines* _pApiRoutines = &_Routines; + CommandHistory* m_pHistory; TEST_METHOD_SETUP(MethodSetup) { @@ -37,11 +38,20 @@ class ApiRoutinesTests m_state->PrepareGlobalInputBuffer(); + m_pHistory = CommandHistory::s_Allocate(L"cmd.exe", nullptr); + if (!m_pHistory) + { + return false; + } + // History must be prepared before COOKED_READ return true; } TEST_METHOD_CLEANUP(MethodCleanup) { + CommandHistory::s_Free(nullptr); + m_pHistory = nullptr; + m_state->CleanupGlobalInputBuffer(); m_state->CleanupGlobalScreenBuffer(); diff --git a/src/host/ut_host/CommandLineTests.cpp b/src/host/ut_host/CommandLineTests.cpp index c5988c8e9..78f253b1b 100644 --- a/src/host/ut_host/CommandLineTests.cpp +++ b/src/host/ut_host/CommandLineTests.cpp @@ -43,12 +43,13 @@ class CommandLineTests m_state->PrepareGlobalScreenBuffer(); m_state->PrepareGlobalInputBuffer(); m_state->PrepareReadHandle(); - m_state->PrepareCookedReadData(); m_pHistory = CommandHistory::s_Allocate(L"cmd.exe", nullptr); if (!m_pHistory) { return false; } + // History must be prepared before COOKED_READ (as it uses s_Find to get at it) + m_state->PrepareCookedReadData(); return true; } diff --git a/src/host/ut_host/CommandListPopupTests.cpp b/src/host/ut_host/CommandListPopupTests.cpp index 2bd19ebae..b0bfe7425 100644 --- a/src/host/ut_host/CommandListPopupTests.cpp +++ b/src/host/ut_host/CommandListPopupTests.cpp @@ -49,7 +49,6 @@ class CommandListPopupTests m_state->PrepareGlobalScreenBuffer(); m_state->PrepareGlobalInputBuffer(); m_state->PrepareReadHandle(); - m_state->PrepareCookedReadData(); m_pHistory = CommandHistory::s_Allocate(L"cmd.exe", nullptr); // resize command history storage to 50 items so that we don't cycle on accident // when PopupTestHelper::InitLongHistory() is called. @@ -58,6 +57,8 @@ class CommandListPopupTests { return false; } + // History must be prepared before COOKED_READ (as it uses s_Find to get at it) + m_state->PrepareCookedReadData(); return true; } diff --git a/src/host/ut_host/CommandNumberPopupTests.cpp b/src/host/ut_host/CommandNumberPopupTests.cpp index 822045223..871ed5f46 100644 --- a/src/host/ut_host/CommandNumberPopupTests.cpp +++ b/src/host/ut_host/CommandNumberPopupTests.cpp @@ -45,12 +45,13 @@ class CommandNumberPopupTests m_state->PrepareGlobalScreenBuffer(); m_state->PrepareGlobalInputBuffer(); m_state->PrepareReadHandle(); - m_state->PrepareCookedReadData(); m_pHistory = CommandHistory::s_Allocate(L"cmd.exe", nullptr); if (!m_pHistory) { return false; } + // History must be prepared before COOKED_READ (as it uses s_Find to get at it) + m_state->PrepareCookedReadData(); return true; } diff --git a/src/host/ut_host/CopyFromCharPopupTests.cpp b/src/host/ut_host/CopyFromCharPopupTests.cpp index 52b147535..e4a42ee5b 100644 --- a/src/host/ut_host/CopyFromCharPopupTests.cpp +++ b/src/host/ut_host/CopyFromCharPopupTests.cpp @@ -24,6 +24,7 @@ class CopyFromCharPopupTests TEST_CLASS(CopyFromCharPopupTests); std::unique_ptr m_state; + CommandHistory* m_pHistory; TEST_CLASS_SETUP(ClassSetup) { @@ -43,12 +44,20 @@ class CopyFromCharPopupTests m_state->PrepareGlobalScreenBuffer(); m_state->PrepareGlobalInputBuffer(); m_state->PrepareReadHandle(); + m_pHistory = CommandHistory::s_Allocate(L"cmd.exe", nullptr); + if (!m_pHistory) + { + return false; + } + // History must be prepared before COOKED_READ (as it uses s_Find to get at it) m_state->PrepareCookedReadData(); return true; } TEST_METHOD_CLEANUP(MethodCleanup) { + CommandHistory::s_Free(nullptr); + m_pHistory = nullptr; m_state->CleanupCookedReadData(); m_state->CleanupReadHandle(); m_state->CleanupGlobalInputBuffer(); diff --git a/src/host/ut_host/CopyToCharPopupTests.cpp b/src/host/ut_host/CopyToCharPopupTests.cpp index 750991074..752922a91 100644 --- a/src/host/ut_host/CopyToCharPopupTests.cpp +++ b/src/host/ut_host/CopyToCharPopupTests.cpp @@ -44,12 +44,13 @@ class CopyToCharPopupTests m_state->PrepareGlobalScreenBuffer(); m_state->PrepareGlobalInputBuffer(); m_state->PrepareReadHandle(); - m_state->PrepareCookedReadData(); m_pHistory = CommandHistory::s_Allocate(L"cmd.exe", nullptr); if (!m_pHistory) { return false; } + // History must be prepared before COOKED_READ (as it uses s_Find to get at it) + m_state->PrepareCookedReadData(); return true; } diff --git a/src/host/ut_host/SelectionTests.cpp b/src/host/ut_host/SelectionTests.cpp index 80515ceb6..9380990f2 100644 --- a/src/host/ut_host/SelectionTests.cpp +++ b/src/host/ut_host/SelectionTests.cpp @@ -412,6 +412,7 @@ class SelectionInputTests TEST_CLASS(SelectionInputTests); CommonState* m_state; + CommandHistory* m_pHistory; TEST_CLASS_SETUP(ClassSetup) { @@ -420,12 +421,20 @@ class SelectionInputTests m_state->PrepareGlobalFont(); m_state->PrepareGlobalScreenBuffer(); m_state->PrepareGlobalInputBuffer(); + m_pHistory = CommandHistory::s_Allocate(L"cmd.exe", nullptr); + if (!m_pHistory) + { + return false; + } + // History must be prepared before COOKED_READ (as it uses s_Find to get at it) return true; } TEST_CLASS_CLEANUP(ClassCleanup) { + CommandHistory::s_Free(nullptr); + m_pHistory = nullptr; m_state->CleanupGlobalScreenBuffer(); m_state->CleanupGlobalFont(); m_state->CleanupGlobalInputBuffer(); From a89b66f770ea877e7f9d00167d6b11b7f5e5866b Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Tue, 9 Nov 2021 19:21:35 +0000 Subject: [PATCH 07/11] Merged PR 6598109: [Git2Git] Pull Request 6508625: Update TAEF to vPack 10.63 (latest) A change required significant changes in TAEF published headers. This PR consumes those changes. Related work items: #20301352 --- src/host/ft_host/API_AliasTests.cpp | 2 +- src/host/ft_host/API_BufferTests.cpp | 2 +- src/host/ft_host/API_DimensionsTests.cpp | 2 +- src/host/ft_host/API_FileTests.cpp | 2 +- src/host/ft_host/API_FontTests.cpp | 2 +- src/host/ft_host/API_InputTests.cpp | 2 +- src/host/ft_host/API_OutputTests.cpp | 2 +- src/host/ft_host/API_PolicyTests.cpp | 2 +- src/host/ft_host/API_TitleTests.cpp | 2 +- src/host/ft_host/CJK_DbcsTests.cpp | 2 +- src/host/ft_host/CanaryTests.cpp | 2 +- src/host/ft_host/Common.cpp | 2 +- src/host/ft_host/Message_KeyPressTests.cpp | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/host/ft_host/API_AliasTests.cpp b/src/host/ft_host/API_AliasTests.cpp index ec274528a..d7a9d9ebd 100644 --- a/src/host/ft_host/API_AliasTests.cpp +++ b/src/host/ft_host/API_AliasTests.cpp @@ -30,7 +30,7 @@ // GetConsoleAlias using namespace WEX::TestExecution; using namespace WEX::Common; -using WEX::Logging::Log; +using namespace WEX::Logging; class AliasTests { diff --git a/src/host/ft_host/API_BufferTests.cpp b/src/host/ft_host/API_BufferTests.cpp index fc1a70b51..ff5e662a2 100644 --- a/src/host/ft_host/API_BufferTests.cpp +++ b/src/host/ft_host/API_BufferTests.cpp @@ -5,7 +5,7 @@ extern "C" IMAGE_DOS_HEADER __ImageBase; -using WEX::Logging::Log; +using namespace WEX::Logging; using namespace WEX::Common; // This class is intended to test boundary conditions for: diff --git a/src/host/ft_host/API_DimensionsTests.cpp b/src/host/ft_host/API_DimensionsTests.cpp index c41415c78..f8efba4d9 100644 --- a/src/host/ft_host/API_DimensionsTests.cpp +++ b/src/host/ft_host/API_DimensionsTests.cpp @@ -4,7 +4,7 @@ #include "precomp.h" using namespace WEX::TestExecution; -using WEX::Logging::Log; +using namespace WEX::Logging; using namespace WEX::Common; // This class is intended to test: diff --git a/src/host/ft_host/API_FileTests.cpp b/src/host/ft_host/API_FileTests.cpp index 1aaa790bd..ed0ed5afc 100644 --- a/src/host/ft_host/API_FileTests.cpp +++ b/src/host/ft_host/API_FileTests.cpp @@ -7,7 +7,7 @@ #include -using WEX::Logging::Log; +using namespace WEX::Logging; using WEX::TestExecution::TestData; using namespace WEX::Common; diff --git a/src/host/ft_host/API_FontTests.cpp b/src/host/ft_host/API_FontTests.cpp index 6222769a9..1c6f10f1d 100644 --- a/src/host/ft_host/API_FontTests.cpp +++ b/src/host/ft_host/API_FontTests.cpp @@ -5,7 +5,7 @@ using namespace WEX::TestExecution; using namespace WEX::Common; -using WEX::Logging::Log; +using namespace WEX::Logging; static const COORD c_coordZero = { 0, 0 }; diff --git a/src/host/ft_host/API_InputTests.cpp b/src/host/ft_host/API_InputTests.cpp index 6eed16bbc..c789de578 100644 --- a/src/host/ft_host/API_InputTests.cpp +++ b/src/host/ft_host/API_InputTests.cpp @@ -11,7 +11,7 @@ #define NUMBER_OF_SCENARIO_INPUTS 10 #define READ_BATCH 3 -using WEX::Logging::Log; +using namespace WEX::Logging; using namespace WEX::Common; // This class is intended to test: diff --git a/src/host/ft_host/API_OutputTests.cpp b/src/host/ft_host/API_OutputTests.cpp index e64517886..2453b71a4 100644 --- a/src/host/ft_host/API_OutputTests.cpp +++ b/src/host/ft_host/API_OutputTests.cpp @@ -11,7 +11,7 @@ using namespace Microsoft::Console::Types; using namespace WEX::TestExecution; -using WEX::Logging::Log; +using namespace WEX::Logging; using namespace WEX::Common; class OutputTests diff --git a/src/host/ft_host/API_PolicyTests.cpp b/src/host/ft_host/API_PolicyTests.cpp index 89f2bef1c..500ae1471 100644 --- a/src/host/ft_host/API_PolicyTests.cpp +++ b/src/host/ft_host/API_PolicyTests.cpp @@ -3,7 +3,7 @@ #include "precomp.h" -using WEX::Logging::Log; +using namespace WEX::Logging; // This class is intended to test restrictions placed on APIs from within a UWP application context class PolicyTests diff --git a/src/host/ft_host/API_TitleTests.cpp b/src/host/ft_host/API_TitleTests.cpp index b78f0e162..880b667ec 100644 --- a/src/host/ft_host/API_TitleTests.cpp +++ b/src/host/ft_host/API_TitleTests.cpp @@ -3,7 +3,7 @@ #include "precomp.h" -using WEX::Logging::Log; +using namespace WEX::Logging; using namespace WEX::Common; // This class is intended to test: diff --git a/src/host/ft_host/CJK_DbcsTests.cpp b/src/host/ft_host/CJK_DbcsTests.cpp index 4b8612004..499549944 100644 --- a/src/host/ft_host/CJK_DbcsTests.cpp +++ b/src/host/ft_host/CJK_DbcsTests.cpp @@ -10,7 +10,7 @@ #define ENGLISH_US_CP 437u #define JAPANESE_CP 932u -using WEX::Logging::Log; +using namespace WEX::Logging; using WEX::TestExecution::TestData; using namespace WEX::Common; diff --git a/src/host/ft_host/CanaryTests.cpp b/src/host/ft_host/CanaryTests.cpp index d174732c8..1c51fa49f 100644 --- a/src/host/ft_host/CanaryTests.cpp +++ b/src/host/ft_host/CanaryTests.cpp @@ -3,7 +3,7 @@ #include "precomp.h" -using WEX::Logging::Log; +using namespace WEX::Logging; using namespace WEX::Common; // This class is intended to provide a canary (simple launch test) diff --git a/src/host/ft_host/Common.cpp b/src/host/ft_host/Common.cpp index df34f101a..1250c5db2 100644 --- a/src/host/ft_host/Common.cpp +++ b/src/host/ft_host/Common.cpp @@ -3,7 +3,7 @@ #include "precomp.h" -using WEX::Logging::Log; +using namespace WEX::Logging; using namespace WEX::Common; HANDLE Common::_hConsole = INVALID_HANDLE_VALUE; diff --git a/src/host/ft_host/Message_KeyPressTests.cpp b/src/host/ft_host/Message_KeyPressTests.cpp index 3b2bb2174..2be78dee1 100644 --- a/src/host/ft_host/Message_KeyPressTests.cpp +++ b/src/host/ft_host/Message_KeyPressTests.cpp @@ -25,7 +25,7 @@ using namespace WEX::TestExecution; using namespace WEX::Common; -using WEX::Logging::Log; +using namespace WEX::Logging; class KeyPressTests { From 92643c1d34876cf0dd71b0eda89f1d1ea2bc67f8 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Tue, 9 Nov 2021 23:16:26 +0000 Subject: [PATCH 08/11] Merged PR 6654362: [Git2Git] OS build fixes for f9b97c488 Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 063b86ac10af16cade5c0754adcbf27e7e9ae266 Related work items: MSFT-34534216, MSFT-36986009, MSFT-36986203 --- src/project.inc | 1 + src/terminal/parser/ut_parser/Base64Test.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/src/project.inc b/src/project.inc index 4a0c674ab..38b99674e 100644 --- a/src/project.inc +++ b/src/project.inc @@ -49,6 +49,7 @@ INCLUDES= \ $(CONSOLE_SRC_PATH)\..\oss\fmt\include; \ $(CONSOLE_SRC_PATH)\..\oss\interval_tree; \ $(CONSOLE_SRC_PATH)\..\oss\boost\boost_1_73_0; \ + $(CONSOLE_SRC_PATH)\..\oss\pcg\include; \ $(MINWIN_INTERNAL_PRIV_SDK_INC_PATH_L); \ $(MINWIN_RESTRICTED_PRIV_SDK_INC_PATH_L); \ $(MINCORE_INTERNAL_PRIV_SDK_INC_PATH_L); \ diff --git a/src/terminal/parser/ut_parser/Base64Test.cpp b/src/terminal/parser/ut_parser/Base64Test.cpp index 5374a303a..137d0a7f4 100644 --- a/src/terminal/parser/ut_parser/Base64Test.cpp +++ b/src/terminal/parser/ut_parser/Base64Test.cpp @@ -5,6 +5,7 @@ #include "WexTestClass.h" #include +#include #include "base64.hpp" From 7db7ba1ac9c1ee65f7b389f1893f985f10e68c17 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Tue, 9 Nov 2021 17:22:55 -0600 Subject: [PATCH 09/11] ci: fix spelling for inbox merge --- .github/actions/spelling/allow/apis.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/actions/spelling/allow/apis.txt b/.github/actions/spelling/allow/apis.txt index b17163a05..45f4e02f1 100644 --- a/.github/actions/spelling/allow/apis.txt +++ b/.github/actions/spelling/allow/apis.txt @@ -17,6 +17,7 @@ COLORPROPERTY colspan COMDLG comparand +commandlinetoargv cstdint CXICON CYICON @@ -161,6 +162,7 @@ toupper TTask TVal UChar +ULARGE UPDATEINIFILE userenv wcsstr From 305255c6581a684940f58b3320b8e9c7c63dd41d Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 10 Nov 2021 22:03:47 +0100 Subject: [PATCH 10/11] Fix a conhost binary size regression due to fmt (#11727) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 6140fd9 causes a binary size regression in conhost. This PR fixes most if not all of the regression, by replacing `FMT_STRING` with `FMT_COMPILE` allowing us to drop most of the formatters built into fmt during linking (for instance floating point formatters). Additionally `std::wstring` was replaced with `fmt::basic_memory_buffer` in the same vein as was done for VtEngine. Stack is cheap and this prevents any unnecessary allocations. ## PR Checklist * [x] I work here * [x] Tests added/passed ## Validation Steps Performed * vttest 11.2.5.3.6.7 and .8 (DECSTBM and SGR) complete successfully ✅ --- src/terminal/adapter/adaptDispatch.cpp | 52 ++++++++++++++------------ 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 8ffe4c202..0553cc538 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -2509,53 +2509,55 @@ ITermDispatch::StringHandler AdaptDispatch::RequestSetting() // - None void AdaptDispatch::_ReportSGRSetting() const { + using namespace std::string_view_literals; + // A valid response always starts with DCS 1 $ r. // Then the '0' parameter is to reset the SGR attributes to the defaults. - std::wstring response = L"\033P1$r0"; + fmt::basic_memory_buffer response; + response.append(L"\033P1$r0"sv); TextAttribute attr; if (_pConApi->PrivateGetTextAttributes(attr)) { // For each boolean attribute that is set, we add the appropriate // parameter value to the response string. - const auto addAttribute = [&](const auto parameter, const auto enabled) { + const auto addAttribute = [&](const auto& parameter, const auto enabled) { if (enabled) { - response += parameter; + response.append(parameter); } }; - addAttribute(L";1", attr.IsBold()); - addAttribute(L";2", attr.IsFaint()); - addAttribute(L";3", attr.IsItalic()); - addAttribute(L";4", attr.IsUnderlined()); - addAttribute(L";5", attr.IsBlinking()); - addAttribute(L";7", attr.IsReverseVideo()); - addAttribute(L";8", attr.IsInvisible()); - addAttribute(L";9", attr.IsCrossedOut()); - addAttribute(L";21", attr.IsDoublyUnderlined()); - addAttribute(L";53", attr.IsOverlined()); + addAttribute(L";1"sv, attr.IsBold()); + addAttribute(L";2"sv, attr.IsFaint()); + addAttribute(L";3"sv, attr.IsItalic()); + addAttribute(L";4"sv, attr.IsUnderlined()); + addAttribute(L";5"sv, attr.IsBlinking()); + addAttribute(L";7"sv, attr.IsReverseVideo()); + addAttribute(L";8"sv, attr.IsInvisible()); + addAttribute(L";9"sv, attr.IsCrossedOut()); + addAttribute(L";21"sv, attr.IsDoublyUnderlined()); + addAttribute(L";53"sv, attr.IsOverlined()); // We also need to add the appropriate color encoding parameters for // both the foreground and background colors. const auto addColor = [&](const auto base, const auto color) { - const auto iterator = std::back_insert_iterator(response); if (color.IsIndex16()) { const auto index = color.GetIndex(); const auto colorParameter = base + (index >= 8 ? 60 : 0) + (index % 8); - fmt::format_to(iterator, FMT_STRING(L";{}"), colorParameter); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{}"), colorParameter); } else if (color.IsIndex256()) { const auto index = color.GetIndex(); - fmt::format_to(iterator, FMT_STRING(L";{};5;{}"), base + 8, index); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{};5;{}"), base + 8, index); } else if (color.IsRgb()) { const auto r = GetRValue(color.GetRGB()); const auto g = GetGValue(color.GetRGB()); const auto b = GetBValue(color.GetRGB()); - fmt::format_to(iterator, FMT_STRING(L";{};2;{};{};{}"), base + 8, r, g, b); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L";{};2;{};{};{}"), base + 8, r, g, b); } }; addColor(30, attr.GetForeground()); @@ -2563,8 +2565,8 @@ void AdaptDispatch::_ReportSGRSetting() const } // The 'm' indicates this is an SGR response, and ST ends the sequence. - response += L"m\033\\"; - _WriteResponse(response); + response.append(L"m\033\\"sv); + _WriteResponse({ response.data(), response.size() }); } // Method Description: @@ -2575,8 +2577,11 @@ void AdaptDispatch::_ReportSGRSetting() const // - None void AdaptDispatch::_ReportDECSTBMSetting() const { + using namespace std::string_view_literals; + // A valid response always starts with DCS 1 $ r. - std::wstring response = L"\033P1$r"; + fmt::basic_memory_buffer response; + response.append(L"\033P1$r"sv); CONSOLE_SCREEN_BUFFER_INFOEX csbiex = { 0 }; csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX); @@ -2592,11 +2597,10 @@ void AdaptDispatch::_ReportDECSTBMSetting() const marginTop = 1; marginBottom = csbiex.srWindow.Bottom - csbiex.srWindow.Top; } - const auto iterator = std::back_insert_iterator(response); - fmt::format_to(iterator, FMT_STRING(L"{};{}"), marginTop, marginBottom); + fmt::format_to(std::back_inserter(response), FMT_COMPILE(L"{};{}"), marginTop, marginBottom); } // The 'r' indicates this is an DECSTBM response, and ST ends the sequence. - response += L"r\033\\"; - _WriteResponse(response); + response.append(L"r\033\\"sv); + _WriteResponse({ response.data(), response.size() }); } From d5974f4c9124a12782d85849f8e1937e4dbaf9ae Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Wed, 10 Nov 2021 14:19:52 -0700 Subject: [PATCH 11/11] Automatically convert paths dropped on WSL instances (#11625) Drag and drop does not work for WSL because paths are pasted as windows paths having incorrect path separator and path root. This PR adds code to correct the path in TerminalControl before pasting to WSL terminals. One problem with this approach is that it assumes the default WSL automount root of "/mnt". It would be possible to add a setting like "WslDragAndDropMountRoot"... but I decided it if someone wants to change automount location it would be simple enough just to create the "/mnt" symlink in WSL. ## Validation Couldn't find an obvious place to add a test. Manually tested cut-n-paste from following paths: - "c:\" - "c:\subdir" - "c:\subdir\subdir" - "\\wsl.localhost\" - \\wsl.localhost\\subdir" Closes #331 --- .github/actions/spelling/allow/allow.txt | 1 + .../TerminalControl/IControlSettings.idl | 1 + src/cascadia/TerminalControl/TermControl.cpp | 37 +++++++++++++++++++ .../TerminalSettings.cpp | 1 + .../TerminalSettingsModel/TerminalSettings.h | 2 + .../UnitTests_Control/MockControlSettings.h | 1 + 6 files changed, 43 insertions(+) diff --git a/.github/actions/spelling/allow/allow.txt b/.github/actions/spelling/allow/allow.txt index 35742a611..085ff6682 100644 --- a/.github/actions/spelling/allow/allow.txt +++ b/.github/actions/spelling/allow/allow.txt @@ -41,6 +41,7 @@ Lmid Lorigin maxed mkmk +mnt mru noreply nje diff --git a/src/cascadia/TerminalControl/IControlSettings.idl b/src/cascadia/TerminalControl/IControlSettings.idl index 1b67df91f..2b16e7334 100644 --- a/src/cascadia/TerminalControl/IControlSettings.idl +++ b/src/cascadia/TerminalControl/IControlSettings.idl @@ -27,6 +27,7 @@ namespace Microsoft.Terminal.Control interface IControlSettings requires Microsoft.Terminal.Core.ICoreSettings, Microsoft.Terminal.Control.IControlAppearance { String ProfileName; + String ProfileSource; Boolean UseAcrylic; ScrollbarState ScrollState; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index afae49d9d..7da2257b4 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -2271,6 +2271,42 @@ namespace winrt::Microsoft::Terminal::Control::implementation } std::wstring fullPath{ item.Path() }; + + // Fix path for WSL + if (_settings.ProfileSource() == L"Windows.Terminal.Wsl") + { + std::replace(fullPath.begin(), fullPath.end(), L'\\', L'/'); + + if (fullPath.size() >= 2 && fullPath.at(1) == L':') + { + // C:/foo/bar -> Cc/foo/bar + fullPath.at(1) = til::tolower_ascii(fullPath.at(0)); + // Cc/foo/bar -> /mnt/c/foo/bar + fullPath.replace(0, 1, L"/mnt/"); + } + else + { + static constexpr std::wstring_view wslPathPrefixes[] = { L"//wsl.localhost/", L"//wsl$/" }; + for (auto prefix : wslPathPrefixes) + { + if (til::starts_with(fullPath, prefix)) + { + if (const auto idx = fullPath.find(L'/', prefix.size()); idx != std::wstring::npos) + { + // //wsl.localhost/Ubuntu-18.04/foo/bar -> /foo/bar + fullPath.erase(0, idx); + } + else + { + // //wsl.localhost/Ubuntu-18.04 -> / + fullPath = L"/"; + } + break; + } + } + } + } + const auto containsSpaces = std::find(fullPath.begin(), fullPath.end(), L' ') != fullPath.end(); @@ -2283,6 +2319,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation allPaths += fullPath; } + _core.PasteText(winrt::hstring{ allPaths }); } } diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp b/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp index 5e4d1eeea..ae9953986 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp @@ -275,6 +275,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // Fill in the remaining properties from the profile _ProfileName = profile.Name(); + _ProfileSource = profile.Source(); _UseAcrylic = profile.UseAcrylic(); _FontFace = profile.FontInfo().FontFace(); diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.h b/src/cascadia/TerminalSettingsModel/TerminalSettings.h index 90b7b60fe..096f18f79 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.h +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.h @@ -119,6 +119,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // ------------------------ End of Core Settings ----------------------- INHERITABLE_SETTING(Model::TerminalSettings, hstring, ProfileName); + INHERITABLE_SETTING(Model::TerminalSettings, hstring, ProfileSource); + INHERITABLE_SETTING(Model::TerminalSettings, bool, UseAcrylic, false); INHERITABLE_SETTING(Model::TerminalSettings, double, Opacity, UseAcrylic() ? 0.5 : 1.0); INHERITABLE_SETTING(Model::TerminalSettings, hstring, Padding, DEFAULT_PADDING); diff --git a/src/cascadia/UnitTests_Control/MockControlSettings.h b/src/cascadia/UnitTests_Control/MockControlSettings.h index 485f9b146..983d7a9d7 100644 --- a/src/cascadia/UnitTests_Control/MockControlSettings.h +++ b/src/cascadia/UnitTests_Control/MockControlSettings.h @@ -51,6 +51,7 @@ namespace ControlUnitTests // ------------------------ End of Core Settings ----------------------- WINRT_PROPERTY(winrt::hstring, ProfileName); + WINRT_PROPERTY(winrt::hstring, ProfileSource); WINRT_PROPERTY(bool, UseAcrylic, false); WINRT_PROPERTY(double, Opacity, .5); WINRT_PROPERTY(winrt::hstring, Padding, DEFAULT_PADDING);