From 89b4739b97433b3655c9c92fdaa20e9f5f765e3b Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 3 Jun 2022 22:11:17 +0200 Subject: [PATCH 1/4] Use feature markers during terminal-by-default handoff --- .github/actions/spelling/expect/expect.txt | 1 + .../CascadiaPackage/Package-Dev.appxmanifest | 1 + .../CascadiaPackage/Package-Pre.appxmanifest | 1 + .../CascadiaPackage/Package.appxmanifest | 1 + .../CascadiaSettings.cpp | 24 +- .../CascadiaSettingsSerialization.cpp | 4 +- .../TerminalSettingsModel/DefaultTerminal.cpp | 30 +- .../Resources/en-US/Resources.resw | 8 + src/host/exe/CConsoleHandoff.h | 2 +- src/host/exe/exemain.cpp | 3 +- src/host/globals.h | 6 +- src/host/proxy/IConsoleHandoff.idl | 6 + src/host/srvinit.cpp | 67 ++--- src/inc/conint.h | 4 +- src/internal/stubs.cpp | 10 +- src/propsheet/TerminalPropsheetPage.cpp | 23 +- src/propsheet/console.cpp | 6 +- src/propsheet/console.h | 3 +- src/propsheet/console.rc | 5 +- src/propsheet/globals.cpp | 1 - src/propsheet/globals.h | 1 - src/propslib/DelegationConfig.cpp | 196 +++++--------- src/propslib/DelegationConfig.hpp | 95 ++++--- src/server/IoDispatchers.cpp | 256 ++++++++++-------- 24 files changed, 379 insertions(+), 375 deletions(-) diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index d9c0dd008a9..dd4342d9d25 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -1082,6 +1082,7 @@ ICore IData IDCANCEL IDD +IDefault IDesktop IDevice IDictionary diff --git a/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest b/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest index 0627906f7c0..bc5ec672090 100644 --- a/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest +++ b/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest @@ -100,6 +100,7 @@ + diff --git a/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest b/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest index 06f3ec52d8f..eb968f870c6 100644 --- a/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest +++ b/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest @@ -189,6 +189,7 @@ + diff --git a/src/cascadia/CascadiaPackage/Package.appxmanifest b/src/cascadia/CascadiaPackage/Package.appxmanifest index 43e65c64220..fb494d98aff 100644 --- a/src/cascadia/CascadiaPackage/Package.appxmanifest +++ b/src/cascadia/CascadiaPackage/Package.appxmanifest @@ -189,6 +189,7 @@ + diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp index b34661a3cb6..8a5b98ea5c9 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp @@ -14,6 +14,7 @@ #include #include +#include using namespace winrt::Microsoft::Terminal; using namespace winrt::Microsoft::Terminal::Settings; @@ -1120,12 +1121,27 @@ void CascadiaSettings::CurrentDefaultTerminal(const Model::DefaultTerminal& term // but in the future it might be worthwhile to change the code to use list indices instead. void CascadiaSettings::_refreshDefaultTerminals() { - if (!_defaultTerminals) + if (_defaultTerminals) { - auto [defaultTerminals, defaultTerminal] = DefaultTerminal::Available(); - _defaultTerminals = winrt::single_threaded_observable_vector(std::move(defaultTerminals)); - _currentDefaultTerminal = std::move(defaultTerminal); + return; } + + // This is an extract of extractValueFromTaskWithoutMainThreadAwait + // as DefaultTerminal::Available creates the exact same issue. + std::pair, Model::DefaultTerminal> result{ {}, nullptr }; + til::latch latch{ 1 }; + + std::ignore = [&]() -> winrt::fire_and_forget { + const auto cleanup = wil::scope_exit([&]() { + latch.count_down(); + }); + co_await winrt::resume_background(); + result = DefaultTerminal::Available(); + }(); + + latch.wait(); + _defaultTerminals = winrt::single_threaded_observable_vector(std::move(result.first)); + _currentDefaultTerminal = std::move(result.second); } void CascadiaSettings::ExportFile(winrt::hstring path, winrt::hstring content) diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index 60bd730477a..a6206917d03 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -61,9 +61,11 @@ static auto extractValueFromTaskWithoutMainThreadAwait(TTask&& task) -> decltype til::latch latch{ 1 }; const auto _ = [&]() -> winrt::fire_and_forget { + const auto cleanup = wil::scope_exit([&]() { + latch.count_down(); + }); co_await winrt::resume_background(); finalVal.emplace(co_await task); - latch.count_down(); }(); latch.wait(); diff --git a/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp b/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp index 2f778bc6eb1..d34c6613fbc 100644 --- a/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp +++ b/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp @@ -17,13 +17,21 @@ DefaultTerminal::DefaultTerminal(DelegationConfig::DelegationPackage&& pkg) : winrt::hstring DefaultTerminal::Name() const { - return _pkg.terminal.name.empty() ? winrt::hstring{ RS_(L"InboxWindowsConsoleName") } : winrt::hstring{ _pkg.terminal.name }; + switch (_pkg.pair.kind) + { + case DelegationConfig::DelegationPairKind::Default: + return RS_(L"DefaultWindowsConsoleName"); + case DelegationConfig::DelegationPairKind::Conhost: + return RS_(L"InboxWindowsConsoleName"); + default: + return winrt::hstring{ _pkg.info.name }; + } } winrt::hstring DefaultTerminal::Version() const { // If there's no version information... return empty string instead. - const auto& version = _pkg.terminal.version; + const auto& version = _pkg.info.version; if (DelegationConfig::PkgVersion{} == version) { return winrt::hstring{}; @@ -36,12 +44,20 @@ winrt::hstring DefaultTerminal::Version() const winrt::hstring DefaultTerminal::Author() const { - return _pkg.terminal.author.empty() ? winrt::hstring{ RS_(L"InboxWindowsConsoleAuthor") } : winrt::hstring{ _pkg.terminal.author }; + switch (_pkg.pair.kind) + { + case DelegationConfig::DelegationPairKind::Default: + return RS_(L"DefaultWindowsConsoleAuthor"); + case DelegationConfig::DelegationPairKind::Conhost: + return RS_(L"InboxWindowsConsoleAuthor"); + default: + return winrt::hstring{ _pkg.info.author }; + } } winrt::hstring DefaultTerminal::Icon() const { - return _pkg.terminal.logo.empty() ? winrt::hstring{ L"\uE756" } : winrt::hstring{ _pkg.terminal.logo }; + return _pkg.info.logo.empty() ? winrt::hstring{ L"\uE756" } : winrt::hstring{ _pkg.info.logo }; } std::pair, Model::DefaultTerminal> DefaultTerminal::Available() @@ -76,11 +92,9 @@ std::pair, Model::DefaultTerminal> DefaultTe bool DefaultTerminal::HasCurrent() { std::vector allPackages; - DelegationConfig::DelegationPackage currentPackage; + DelegationConfig::DelegationPackage currentPackage{ DelegationConfig::DefaultDelegationPair }; LOG_IF_FAILED(DelegationConfig::s_GetAvailablePackages(allPackages, currentPackage)); - - // Good old conhost has a hardcoded GUID of {00000000-0000-0000-0000-000000000000}. - return currentPackage.terminal.clsid != CLSID{}; + return !currentPackage.pair.IsDefault(); } void DefaultTerminal::Current(const Model::DefaultTerminal& term) diff --git a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw index 7a23d7a111e..c0413ae8bba 100644 --- a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw @@ -493,6 +493,14 @@ Clear scrollback A command to clear the part of the buffer above the viewport + + Microsoft Corporation + Paired with `DefaultWindowsConsoleName`, this is the application author... which is us: Microsoft. + + + Let Windows decide + This is the default option if a user doesn't choose to override Microsoft's choice of a Terminal. + Microsoft Corporation Paired with `InboxWindowsConsoleName`, this is the application author... which is us: Microsoft. diff --git a/src/host/exe/CConsoleHandoff.h b/src/host/exe/CConsoleHandoff.h index 36ad564f620..67ee7bde11e 100644 --- a/src/host/exe/CConsoleHandoff.h +++ b/src/host/exe/CConsoleHandoff.h @@ -29,7 +29,7 @@ Author(s): using namespace Microsoft::WRL; struct __declspec(uuid(__CLSID_CConsoleHandoff)) - CConsoleHandoff : public RuntimeClass, IConsoleHandoff> + CConsoleHandoff : public RuntimeClass, IConsoleHandoff, IDefaultTerminalMarker> { #pragma region IConsoleHandoff STDMETHODIMP EstablishHandoff(HANDLE server, diff --git a/src/host/exe/exemain.cpp b/src/host/exe/exemain.cpp index b0c77ee5fc1..27e4b05cc14 100644 --- a/src/host/exe/exemain.cpp +++ b/src/host/exe/exemain.cpp @@ -265,8 +265,7 @@ int CALLBACK wWinMain( { // Only try to register as a handoff target if we are NOT a part of Windows. #if TIL_FEATURE_RECEIVEINCOMINGHANDOFF_ENABLED - auto defAppEnabled = false; - if (args.ShouldRunAsComServer() && SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(defAppEnabled)) && defAppEnabled) + if (args.ShouldRunAsComServer() && Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy()) { try { diff --git a/src/host/globals.h b/src/host/globals.h index 50e42c82d5c..319849f1030 100644 --- a/src/host/globals.h +++ b/src/host/globals.h @@ -22,8 +22,8 @@ Revision History: #include "ConsoleArguments.hpp" #include "ApiRoutines.h" +#include "../propslib/DelegationConfig.hpp" #include "../renderer/base/Renderer.hpp" - #include "../server/DeviceComm.h" #include "../server/ConDrvDeviceComm.h" @@ -71,10 +71,10 @@ class Globals bool handoffTarget = false; - std::optional handoffConsoleClsid; - std::optional handoffTerminalClsid; + DelegationConfig::DelegationPair delegationPair; wil::unique_hfile handoffInboxConsoleHandle; wil::unique_threadpool_wait handoffInboxConsoleExitWait; + bool defaultTerminalMarkerCheckRequired = false; #ifdef UNIT_TESTING void EnableConptyModeForTests(std::unique_ptr vtRenderEngine); diff --git a/src/host/proxy/IConsoleHandoff.idl b/src/host/proxy/IConsoleHandoff.idl index d7e8d63bc53..3a07dd84691 100644 --- a/src/host/proxy/IConsoleHandoff.idl +++ b/src/host/proxy/IConsoleHandoff.idl @@ -31,3 +31,9 @@ typedef const CONSOLE_PORTABLE_ATTACH_MSG* PCCONSOLE_PORTABLE_ATTACH_MSG; [out, system_handle(sh_process)] HANDLE* process); }; +[ + object, + uuid(746E6BC0-AB05-4E38-AB14-71E86763141F) +] interface IDefaultTerminalMarker : IUnknown +{ +}; diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index f963999341a..1b6d5b5179b 100644 --- a/src/host/srvinit.cpp +++ b/src/host/srvinit.cpp @@ -26,7 +26,6 @@ #include "../renderer/base/renderer.hpp" #include "../inc/conint.h" -#include "../propslib/DelegationConfig.hpp" #include "tracing.hpp" @@ -64,28 +63,25 @@ try // Check if this conhost is allowed to delegate its activities to another. // If so, look up the registered default console handler. - auto isEnabled = false; - if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(isEnabled)) && isEnabled) + if (Globals.delegationPair.IsUndecided() && Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy()) { - IID delegationClsid; - if (SUCCEEDED(DelegationConfig::s_GetDefaultConsoleId(delegationClsid))) - { - Globals.handoffConsoleClsid = delegationClsid; - TraceLoggingWrite(g_hConhostV2EventTraceProvider, - "SrvInit_FoundDelegationConsole", - TraceLoggingGuid(Globals.handoffConsoleClsid.value(), "ConsoleClsid"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - } - if (SUCCEEDED(DelegationConfig::s_GetDefaultTerminalId(delegationClsid))) - { - Globals.handoffTerminalClsid = delegationClsid; - TraceLoggingWrite(g_hConhostV2EventTraceProvider, - "SrvInit_FoundDelegationTerminal", - TraceLoggingGuid(Globals.handoffTerminalClsid.value(), "TerminalClsid"), - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - } + Globals.delegationPair = DelegationConfig::s_GetDelegationPair(); + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_FoundDelegationConsole", + TraceLoggingGuid(Globals.delegationPair.console, "ConsoleClsid"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_FoundDelegationTerminal", + TraceLoggingGuid(Globals.delegationPair.terminal, "TerminalClsid"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + } + if (Globals.delegationPair.IsDefault() && Microsoft::Console::Internal::DefaultApp::CheckShouldTerminalBeDefault()) + { + Globals.delegationPair = DelegationConfig::TerminalDelegationPair; + Globals.defaultTerminalMarkerCheckRequired = true; } // Create the accessibility notifier early in the startup process. @@ -427,28 +423,15 @@ try auto& g = ServiceLocator::LocateGlobals(); g.handoffTarget = true; - IID delegationClsid; - if (SUCCEEDED(DelegationConfig::s_GetDefaultConsoleId(delegationClsid))) - { - g.handoffConsoleClsid = delegationClsid; - } - if (SUCCEEDED(DelegationConfig::s_GetDefaultTerminalId(delegationClsid))) - { - g.handoffTerminalClsid = delegationClsid; - } - - if (!g.handoffTerminalClsid) + g.delegationPair = DelegationConfig::s_GetDelegationPair(); + if (!g.delegationPair.IsCustom()) { - TraceLoggingWrite(g_hConhostV2EventTraceProvider, - "SrvInit_ReceiveHandoff_NoTerminal", - TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), - TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - return E_NOT_SET; + g.delegationPair = DelegationConfig::TerminalDelegationPair; } TraceLoggingWrite(g_hConhostV2EventTraceProvider, "SrvInit_ReceiveHandoff", - TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"), + TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); @@ -509,14 +492,14 @@ try TraceLoggingWrite(g_hConhostV2EventTraceProvider, "SrvInit_PrepareToCreateDelegationTerminal", - TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"), + TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - RETURN_IF_FAILED(CoCreateInstance(g.handoffTerminalClsid.value(), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff))); + RETURN_IF_FAILED(CoCreateInstance(g.delegationPair.terminal, nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff))); TraceLoggingWrite(g_hConhostV2EventTraceProvider, "SrvInit_CreatedDelegationTerminal", - TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"), + TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); diff --git a/src/inc/conint.h b/src/inc/conint.h index 26a2a9e0107..2a3c8d0fd54 100644 --- a/src/inc/conint.h +++ b/src/inc/conint.h @@ -47,7 +47,7 @@ namespace Microsoft::Console::Internal namespace DefaultApp { - [[nodiscard]] HRESULT CheckDefaultAppPolicy(bool& isEnabled) noexcept; - [[nodiscard]] HRESULT CheckShouldTerminalBeDefault(bool& isEnabled) noexcept; + [[nodiscard]] bool CheckDefaultAppPolicy() noexcept; + [[nodiscard]] bool CheckShouldTerminalBeDefault() noexcept; } } diff --git a/src/internal/stubs.cpp b/src/internal/stubs.cpp index da6423d14ec..f0cf811b017 100644 --- a/src/internal/stubs.cpp +++ b/src/internal/stubs.cpp @@ -30,19 +30,17 @@ void EdpPolicy::AuditClipboard(const std::wstring_view /*destinationName*/) noex return S_FALSE; } -[[nodiscard]] HRESULT DefaultApp::CheckDefaultAppPolicy(bool& isEnabled) noexcept +[[nodiscard]] bool DefaultApp::CheckDefaultAppPolicy() noexcept { // True so propsheet will show configuration options but be sure that // the open one won't attempt handoff from double click of OpenConsole.exe - isEnabled = true; - return S_OK; + return true; } -[[nodiscard]] HRESULT DefaultApp::CheckShouldTerminalBeDefault(bool& isEnabled) noexcept +[[nodiscard]] bool DefaultApp::CheckShouldTerminalBeDefault() noexcept { // False since setting Terminal as the default app is an OS feature and probably // should not be done in the open source conhost. We can always decide to turn it // on in the future though. - isEnabled = false; - return S_OK; + return false; } diff --git a/src/propsheet/TerminalPropsheetPage.cpp b/src/propsheet/TerminalPropsheetPage.cpp index 5dcb6a6327e..36791ddedd4 100644 --- a/src/propsheet/TerminalPropsheetPage.cpp +++ b/src/propsheet/TerminalPropsheetPage.cpp @@ -97,26 +97,27 @@ void _PrepDefAppCombo(const HWND hDlg, const auto hCombo = GetDlgItem(hDlg, dlgItem); ComboBox_ResetContent(hCombo); + DWORD i = 0; DWORD selectedIndex = 0; - for (DWORD i = 0; i < gsl::narrow(list.size()); ++i) + for (const auto& item : list) { - auto& item = list[i]; - - // An empty CLSID is a sentinel for the inbox console. - if (item.terminal.clsid == CLSID{ 0 }) - { - const auto name = GetStringResource(IDS_TERMINAL_DEF_INBOX); - ComboBox_AddString(hCombo, name.c_str()); - } - else + switch (item.pair.kind) { - ComboBox_AddString(hCombo, item.terminal.name.c_str()); + case DelegationConfig::DelegationPairKind::Default: + ComboBox_AddString(hCombo, GetStringResource(IDS_TERMINAL_HANDOFF_DEFAULT).c_str()); + break; + case DelegationConfig::DelegationPairKind::Conhost: + ComboBox_AddString(hCombo, GetStringResource(IDS_TERMINAL_HANDOFF_CONHOST).c_str()); + break; + default: + ComboBox_AddString(hCombo, item.info.name.c_str()); } ComboBox_SetItemData(hCombo, i, &item); if (selected == item) { selectedIndex = i; } + ++i; } ComboBox_SetCurSel(hCombo, selectedIndex); diff --git a/src/propsheet/console.cpp b/src/propsheet/console.cpp index 60303430762..72bcbd53edf 100644 --- a/src/propsheet/console.cpp +++ b/src/propsheet/console.cpp @@ -96,7 +96,7 @@ void SaveConsoleSettingsIfNeeded(const HWND hwnd) gpStateInfo->FaceName[0] = TEXT('\0'); } - if (g_defAppEnabled) + if (Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy()) { LOG_IF_FAILED(DelegationConfig::s_SetDefaultByPackage(g_selectedPackage)); } @@ -552,7 +552,7 @@ BOOL PopulatePropSheetPageArray(_Out_writes_(cPsps) PROPSHEETPAGE* pPsp, const s { pTerminalPage->dwSize = sizeof(PROPSHEETPAGE); pTerminalPage->hInstance = ghInstance; - if (g_defAppEnabled) + if (Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy()) { pTerminalPage->pszTemplate = MAKEINTRESOURCE(DID_TERMINAL_WITH_DEFTERM); } @@ -629,7 +629,7 @@ INT_PTR ConsolePropertySheet(__in HWND hWnd, __in PCONSOLE_STATE_INFO pStateInfo // Find the available default console/terminal packages // - if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(g_defAppEnabled)) && g_defAppEnabled) + if (Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy()) { LOG_IF_FAILED(DelegationConfig::s_GetAvailablePackages(g_availablePackages, g_selectedPackage)); } diff --git a/src/propsheet/console.h b/src/propsheet/console.h index 5eb7e91a01f..dfd0efd87ed 100644 --- a/src/propsheet/console.h +++ b/src/propsheet/console.h @@ -58,7 +58,8 @@ Revision History: // unused 16 #define IDS_TOOLTIP_OPACITY 17 #define IDS_TOOLTIP_INTERCEPT_COPY_PASTE 18 -#define IDS_TERMINAL_DEF_INBOX 19 +#define IDS_TERMINAL_HANDOFF_DEFAULT 19 +#define IDS_TERMINAL_HANDOFF_CONHOST 20 // clang-format on void MakeAltRasterFont( diff --git a/src/propsheet/console.rc b/src/propsheet/console.rc index bf1c06e3f5b..18f3077d490 100644 --- a/src/propsheet/console.rc +++ b/src/propsheet/console.rc @@ -721,7 +721,7 @@ BEGIN AUTOCHECKBOX "Disable Scroll-Forward", IDD_DISABLE_SCROLLFORWARD, T_SCROLL_X+P_1, T_SCROLL_Y+P_2, T_SCROLL_W-P_4-P_4, 10 GROUPBOX "Default Terminal Application", -1, T_DEFAPP_X, T_DEFAPP_Y, T_DEFAPP_W, T_DEFAPP_H - COMBOBOX IDD_TERMINAL_COMBO_DEFTERM, T_DEFTERM_X+P_0, T_DEFTERM_Y, T_DEFTERM_W, 10, CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP + COMBOBOX IDD_TERMINAL_COMBO_DEFTERM, T_DEFTERM_X+P_0, T_DEFTERM_Y, T_DEFTERM_W, 10, CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP CONTROL "Find out more about experimental terminal settings", IDD_HELP_TERMINAL, "SysLink", WS_TABSTOP, 10, 225, 200, 10 @@ -749,7 +749,8 @@ BEGIN IDS_TOOLTIP_EDIT_KEYS, "Enable enhanced keyboard editing on command line." IDS_TOOLTIP_OPACITY, "Adjust the transparency of the console window." IDS_TOOLTIP_INTERCEPT_COPY_PASTE, "Use Ctrl+Shift+C/V as copy/paste shortcuts, regardless of input mode" - IDS_TERMINAL_DEF_INBOX, "Windows Console Host" + IDS_TERMINAL_HANDOFF_DEFAULT, "Let Windows decide" + IDS_TERMINAL_HANDOFF_CONHOST, "Windows Console Host" END diff --git a/src/propsheet/globals.cpp b/src/propsheet/globals.cpp index e42d0bbf4f6..b1b8baafb8e 100644 --- a/src/propsheet/globals.cpp +++ b/src/propsheet/globals.cpp @@ -60,6 +60,5 @@ COLORREF g_fakeCursorColor = RGB(242, 242, 242); // Default bright white HWND g_hTerminalDlg = static_cast(INVALID_HANDLE_VALUE); HWND g_hOptionsDlg = static_cast(INVALID_HANDLE_VALUE); -bool g_defAppEnabled = false; std::vector g_availablePackages; DelegationConfig::DelegationPackage g_selectedPackage; diff --git a/src/propsheet/globals.h b/src/propsheet/globals.h index 1ecfb407e31..3910c0300a7 100644 --- a/src/propsheet/globals.h +++ b/src/propsheet/globals.h @@ -56,6 +56,5 @@ extern COLORREF g_fakeCursorColor; extern HWND g_hTerminalDlg; extern HWND g_hOptionsDlg; -extern bool g_defAppEnabled; extern std::vector g_availablePackages; extern DelegationConfig::DelegationPackage g_selectedPackage; diff --git a/src/propslib/DelegationConfig.cpp b/src/propslib/DelegationConfig.cpp index 72f0519265d..2733094de24 100644 --- a/src/propslib/DelegationConfig.cpp +++ b/src/propslib/DelegationConfig.cpp @@ -29,27 +29,11 @@ using namespace ABI::Windows::ApplicationModel::AppExtensions; #define DELEGATION_CONSOLE_KEY_NAME L"DelegationConsole" #define DELEGATION_TERMINAL_KEY_NAME L"DelegationTerminal" -DEFINE_GUID(CLSID_SystemDelegationConsole, 0x2eaca947, 0x7f5f, 0x4cfa, 0xba, 0x87, 0x8f, 0x7f, 0xbe, 0xef, 0xbe, 0x69); -DEFINE_GUID(CLSID_SystemDelegationTerminal, 0xe12cff52, 0xa866, 0x4c77, 0x9a, 0x90, 0xf5, 0x70, 0xa7, 0xaa, 0x2c, 0x6b); - #define DELEGATION_CONSOLE_EXTENSION_NAME L"com.microsoft.windows.console.host" #define DELEGATION_TERMINAL_EXTENSION_NAME L"com.microsoft.windows.terminal.host" -template::value>::type* = nullptr> -HRESULT _lookupCatalog(PCWSTR extensionName, std::vector& vec) noexcept +static [[nodiscard]] HRESULT _lookupCatalog(PCWSTR extensionName, std::vector& vec) noexcept { - vec.clear(); - - T useInbox = { 0 }; - useInbox.clsid = { 0 }; - // CLSID of 0 will be sentinel to say "inbox console" or something. - // The UI displaying this information will have to go look up appropriate strings - // to convey that message. - - vec.push_back(useInbox); - - auto coinit = wil::CoInitializeEx(COINIT_APARTMENTTHREADED); - ComPtr catalogStatics; RETURN_IF_FAILED(Windows::Foundation::GetActivationFactory(HStringReference(RuntimeClass_Windows_ApplicationModel_AppExtensions_AppExtensionCatalog).Get(), &catalogStatics)); @@ -66,7 +50,7 @@ HRESULT _lookupCatalog(PCWSTR extensionName, std::vector& vec) noexcept RETURN_IF_FAILED(extensionList->get_Size(&extensionCount)); for (UINT index = 0; index < extensionCount; index++) { - T extensionMetadata; + DelegationConfig::PackageInfo extensionMetadata; ComPtr extension; RETURN_IF_FAILED(extensionList->GetAt(index, &extension)); @@ -165,55 +149,43 @@ HRESULT _lookupCatalog(PCWSTR extensionName, std::vector& vec) noexcept IID iid; RETURN_IF_FAILED(IIDFromString(value.GetRawBuffer(nullptr), &iid)); - extensionMetadata.clsid = iid; - - vec.emplace_back(std::move(extensionMetadata)); + vec.push_back({ iid, extensionMetadata }); } return S_OK; } -[[nodiscard]] HRESULT DelegationConfig::s_GetAvailableConsoles(std::vector& consoles) noexcept +[[nodiscard]] HRESULT DelegationConfig::s_GetAvailablePackages(std::vector& packages, DelegationPackage& def) noexcept try { - return _lookupCatalog(DELEGATION_CONSOLE_EXTENSION_NAME, consoles); -} -CATCH_RETURN() - -[[nodiscard]] HRESULT DelegationConfig::s_GetAvailableTerminals(std::vector& terminals) noexcept -try -{ - return _lookupCatalog(DELEGATION_TERMINAL_EXTENSION_NAME, terminals); -} -CATCH_RETURN() + auto coinit = wil::CoInitializeEx(COINIT_APARTMENTTHREADED); -[[nodiscard]] HRESULT DelegationConfig::s_GetAvailablePackages(std::vector& packages, DelegationPackage& defPackage) noexcept -try -{ packages.clear(); + packages.push_back({ DefaultDelegationPair }); + packages.push_back({ ConhostDelegationPair }); // Get consoles and terminals. // If we fail to look up any, we should still have ONE come back to us as the hardcoded default console host. // The errors aren't really useful except for debugging, so log only. - std::vector consoles; - LOG_IF_FAILED(s_GetAvailableConsoles(consoles)); + std::vector consoles; + LOG_IF_FAILED(_lookupCatalog(DELEGATION_CONSOLE_EXTENSION_NAME, consoles)); - std::vector terminals; - LOG_IF_FAILED(s_GetAvailableTerminals(terminals)); + std::vector terminals; + LOG_IF_FAILED(_lookupCatalog(DELEGATION_TERMINAL_EXTENSION_NAME, terminals)); // TODO: I hate this algorithm (it's bad performance), but I couldn't // find an AppModel interface that would let me look up all the extensions // in one package. - for (auto& term : terminals) + for (const auto& term : terminals) { - for (auto& con : consoles) + for (const auto& con : consoles) { - if (term.IsFromSamePackage(con)) + if (term.info.IsFromSamePackage(con.info)) { - DelegationPackage pkg; - pkg.terminal = term; - pkg.console = con; - packages.push_back(pkg); + DelegationPackage package; + package.pair = { DelegationPairKind::Custom, con.clsid, term.clsid }; + package.info = term.info; + packages.push_back(std::move(package)); break; } } @@ -222,29 +194,19 @@ try // We should find at least one package. RETURN_HR_IF(E_FAIL, packages.empty()); - // We also find the default here while we have the list of available ones so - // we can return the opaque structure instead of the raw IID. - IID defCon; - LOG_IF_FAILED(s_GetDefaultConsoleId(defCon)); - IID defTerm; - LOG_IF_FAILED(s_GetDefaultTerminalId(defTerm)); - - // The default one is the 0th one because that's supposed to be the inbox conhost one. - auto chosenPackage = packages.at(0); - - // Search through and find a package that matches. If we failed to match because - // it's torn across multiple or something not in the catalog, we'll offer the inbox conhost one. + // Search through and find a package that matches. + const auto delegationPair = s_GetDelegationPair(); for (auto& pkg : packages) { - if (pkg.console.clsid == defCon && pkg.terminal.clsid == defTerm) + if (pkg.pair == delegationPair) { - chosenPackage = pkg; - break; + def = pkg; + return S_OK; } } - defPackage = chosenPackage; - + // The default is DefaultDelegationPair ("Let Windows decide"). + def = packages.at(0); return S_OK; } CATCH_RETURN() @@ -261,90 +223,62 @@ CATCH_RETURN() [[nodiscard]] HRESULT DelegationConfig::s_SetDefaultByPackage(const DelegationPackage& package, const bool useRegExe) noexcept { - RETURN_IF_FAILED(s_SetDefaultConsoleById(package.console.clsid, useRegExe)); - RETURN_IF_FAILED(s_SetDefaultTerminalById(package.terminal.clsid, useRegExe)); + RETURN_IF_FAILED(s_SetDefaultConsoleById(package.pair.console, useRegExe)); + RETURN_IF_FAILED(s_SetDefaultTerminalById(package.pair.terminal, useRegExe)); return S_OK; } -[[nodiscard]] HRESULT DelegationConfig::s_GetDefaultConsoleId(IID& iid) noexcept +[[nodiscard]] DelegationConfig::DelegationPair DelegationConfig::s_GetDelegationPair() noexcept { - iid = { 0 }; + wil::unique_hkey currentUserKey; + wil::unique_hkey consoleKey; + wil::unique_hkey startupKey; + if (FAILED_NTSTATUS_LOG(RegistrySerialization::s_OpenConsoleKey(¤tUserKey, &consoleKey)) || + FAILED_NTSTATUS_LOG(RegistrySerialization::s_OpenKey(consoleKey.get(), L"%%Startup", &startupKey))) + { + return DefaultDelegationPair; + } - auto hr = s_Get(DELEGATION_CONSOLE_KEY_NAME, iid); + static constexpr const wchar_t* keys[2]{ DELEGATION_CONSOLE_KEY_NAME, DELEGATION_TERMINAL_KEY_NAME }; + IID values[2]{ CLSID_Default, CLSID_Default }; - auto defApp = false; - if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckShouldTerminalBeDefault(defApp)) && defApp) + for (size_t i = 0; i < 2; ++i) { - if (FAILED(hr)) + // The GUID is stored as: {00000000-0000-0000-0000-000000000000} + // = 38 characters + trailing null terminator. + wchar_t buffer[39]; + DWORD bytesUsed = 0; + const auto result = RegistrySerialization::s_QueryValue(startupKey.get(), keys[i], sizeof(buffer), REG_SZ, reinterpret_cast(&buffer[0]), &bytesUsed); + if (result != S_OK) { - // If we can't find a user-defined delegation console/terminal, use the hardcoded - // delegation console/terminal instead. - iid = CLSID_SystemDelegationConsole; - hr = S_OK; + // Don't log the more common key-not-found error. + if (result != NTSTATUS_FROM_WIN32(ERROR_FILE_NOT_FOUND)) + { + LOG_NTSTATUS(result); + } + continue; } - } - - return hr; -} - -[[nodiscard]] HRESULT DelegationConfig::s_GetDefaultTerminalId(IID& iid) noexcept -{ - iid = { 0 }; - - auto hr = s_Get(DELEGATION_TERMINAL_KEY_NAME, iid); - auto defApp = false; - if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckShouldTerminalBeDefault(defApp)) && defApp) - { - if (FAILED(hr)) + if (bytesUsed == sizeof(buffer)) { - // If we can't find a user-defined delegation console/terminal, use the hardcoded - // delegation console/terminal instead. - iid = CLSID_SystemDelegationTerminal; - hr = S_OK; + // RegQueryValueExW docs: + // If the data has the REG_SZ, REG_MULTI_SZ or REG_EXPAND_SZ type, the string may not have been stored with the + // proper terminating null characters. Therefore, even if the function returns ERROR_SUCCESS, the application + // should ensure that the string is properly terminated before using it; otherwise, it may overwrite a buffer. + buffer[std::size(buffer) - 1] = 0; + LOG_IF_FAILED(IIDFromString(&buffer[0], &values[i])); } } - return hr; -} - -[[nodiscard]] HRESULT DelegationConfig::s_Get(PCWSTR value, IID& iid) noexcept -{ - wil::unique_hkey currentUserKey; - wil::unique_hkey consoleKey; - - RETURN_IF_NTSTATUS_FAILED(RegistrySerialization::s_OpenConsoleKey(¤tUserKey, &consoleKey)); - - wil::unique_hkey startupKey; - RETURN_IF_NTSTATUS_FAILED(RegistrySerialization::s_OpenKey(consoleKey.get(), L"%%Startup", &startupKey)); - - DWORD bytesNeeded = 0; - auto result = RegistrySerialization::s_QueryValue(startupKey.get(), - value, - 0, - REG_SZ, - nullptr, - &bytesNeeded); - - if (NTSTATUS_FROM_WIN32(ERROR_SUCCESS) != result) + if (values[0] == CLSID_Default || values[1] == CLSID_Default) { - RETURN_NTSTATUS(result); + return DefaultDelegationPair; } - - auto buffer = std::make_unique(bytesNeeded / sizeof(wchar_t) + 1); - - DWORD bytesUsed = 0; - - RETURN_IF_NTSTATUS_FAILED(RegistrySerialization::s_QueryValue(startupKey.get(), - value, - bytesNeeded, - REG_SZ, - reinterpret_cast(buffer.get()), - &bytesUsed)); - - RETURN_IF_FAILED(IIDFromString(buffer.get(), &iid)); - - return S_OK; + if (values[0] == CLSID_Conhost || values[1] == CLSID_Conhost) + { + return ConhostDelegationPair; + } + return { DelegationPairKind::Custom, values[0], values[1] }; } [[nodiscard]] HRESULT DelegationConfig::s_Set(PCWSTR value, const CLSID clsid, const bool useRegExe) noexcept diff --git a/src/propslib/DelegationConfig.hpp b/src/propslib/DelegationConfig.hpp index 8590333d06d..70e3d6cb505 100644 --- a/src/propslib/DelegationConfig.hpp +++ b/src/propslib/DelegationConfig.hpp @@ -18,66 +18,82 @@ Author(s): class DelegationConfig { public: + enum class DelegationPairKind : uint32_t + { + Undecided, + Default, + Conhost, + Custom, + }; + + struct DelegationPair + { + // state contains a "pre parsed" idea of what console/terminal mean. + // This might help make some code more readable. + // If either are CLSID_Default, state will be Default. + // If either are CLSID_Conhost, state will be Conhost. + // Otherwise it'll be Custom. + DelegationPairKind kind = DelegationPairKind::Undecided; + CLSID console{}; + CLSID terminal{}; + + constexpr bool IsUndecided() const noexcept { return kind == DelegationPairKind::Undecided; } + constexpr bool IsDefault() const noexcept { return kind == DelegationPairKind::Default; } + constexpr bool IsConhost() const noexcept { return kind == DelegationPairKind::Conhost; } + constexpr bool IsCustom() const noexcept { return kind == DelegationPairKind::Custom; } + + constexpr bool operator==(const DelegationPair& other) const noexcept + { + static_assert(std::has_unique_object_representations_v); + return __builtin_memcmp(this, &other, sizeof(*this)) == 0; + } + }; + struct PkgVersion { - unsigned short major; - unsigned short minor; - unsigned short build; - unsigned short revision; + unsigned short major = 0; + unsigned short minor = 0; + unsigned short build = 0; + unsigned short revision = 0; - bool operator==(const PkgVersion& other) const + constexpr bool operator==(const PkgVersion& other) const noexcept { - return major == other.major && - minor == other.minor && - build == other.build && - revision == other.revision; + static_assert(std::has_unique_object_representations_v); + return __builtin_memcmp(this, &other, sizeof(*this)) == 0; } }; - struct DelegationBase + struct PackageInfo { - CLSID clsid; std::wstring name; std::wstring author; std::wstring pfn; std::wstring logo; PkgVersion version; - bool IsFromSamePackage(const DelegationBase& other) const + bool IsFromSamePackage(const PackageInfo& other) const noexcept { return name == other.name && author == other.author && pfn == other.pfn && version == other.version; } - - bool operator==(const DelegationBase& other) const - { - return clsid == other.clsid && - name == other.name && - author == other.author && - pfn == other.pfn && - version == other.version; - } - }; - - struct DelegationConsole : public DelegationBase - { }; - struct DelegationTerminal : public DelegationBase + struct DelegationBase { + CLSID clsid{}; + PackageInfo info; }; struct DelegationPackage { - DelegationConsole console; - DelegationTerminal terminal; + DelegationPair pair; + PackageInfo info; - bool operator==(const DelegationPackage& other) const + bool operator==(const DelegationPackage& other) const noexcept { - return console == other.console && - terminal == other.terminal; + return pair == other.pair; } }; @@ -85,16 +101,21 @@ class DelegationConfig [[nodiscard]] static HRESULT s_SetDefaultByPackage(const DelegationPackage& pkg, const bool useRegExe = false) noexcept; - [[nodiscard]] static HRESULT s_GetDefaultConsoleId(IID& iid) noexcept; - [[nodiscard]] static HRESULT s_GetDefaultTerminalId(IID& iid) noexcept; + static constexpr CLSID CLSID_Default{}; + static constexpr CLSID CLSID_Conhost{ 0xb23d10c0, 0xe52e, 0x411e, { 0x9d, 0x5b, 0xc0, 0x9f, 0xdf, 0x70, 0x9c, 0x7d } }; + static constexpr CLSID CLSID_WindowsTerminalConsole{ 0x2eaca947, 0x7f5f, 0x4cfa, { 0xba, 0x87, 0x8f, 0x7f, 0xbe, 0xef, 0xbe, 0x69 } }; + static constexpr CLSID CLSID_WindowsTerminalTerminal{ 0xe12cff52, 0xa866, 0x4c77, { 0x9a, 0x90, 0xf5, 0x70, 0xa7, 0xaa, 0x2c, 0x6b } }; + static constexpr CLSID CLSID_WindowsTerminalConsoleDev{ 0x1f9f2bf5, 0x5bc3, 0x4f17, { 0xb0, 0xe6, 0x91, 0x24, 0x13, 0xf1, 0xf4, 0x51 } }; + static constexpr CLSID CLSID_WindowsTerminalTerminalDev{ 0x051f34ee, 0xc1fd, 0x4b19, { 0xaf, 0x75, 0x9b, 0xa5, 0x46, 0x48, 0x43, 0x4c } }; + static constexpr DelegationPair DefaultDelegationPair{ DelegationPairKind::Default, CLSID_Default, CLSID_Default }; + static constexpr DelegationPair ConhostDelegationPair{ DelegationPairKind::Conhost, CLSID_Conhost, CLSID_Conhost }; + static constexpr DelegationPair TerminalDelegationPair{ DelegationPairKind::Custom, CLSID_WindowsTerminalConsole, CLSID_WindowsTerminalTerminal }; -private: - [[nodiscard]] static HRESULT s_GetAvailableConsoles(std::vector& consoles) noexcept; - [[nodiscard]] static HRESULT s_GetAvailableTerminals(std::vector& terminals) noexcept; + [[nodiscard]] static DelegationPair s_GetDelegationPair() noexcept; +private: [[nodiscard]] static HRESULT s_SetDefaultConsoleById(const IID& iid, const bool useRegExe) noexcept; [[nodiscard]] static HRESULT s_SetDefaultTerminalById(const IID& iid, const bool useRegExe) noexcept; - [[nodiscard]] static HRESULT s_Get(PCWSTR value, IID& iid) noexcept; [[nodiscard]] static HRESULT s_Set(PCWSTR value, const CLSID clsid, const bool useRegExe) noexcept; }; diff --git a/src/server/IoDispatchers.cpp b/src/server/IoDispatchers.cpp index 10363449230..f08e840fd1d 100644 --- a/src/server/IoDispatchers.cpp +++ b/src/server/IoDispatchers.cpp @@ -178,27 +178,15 @@ static bool _shouldAttemptHandoff(const Globals& globals, #else - // Service desktops and non-interactive sessions should not - // try to hand off -- they probably don't have any terminals - // installed, and we don't want to risk breaking a service if - // they *do*. - if (!_isInteractiveUserSession()) - { - return false; - } - - // This console was started with a command line argument to - // specifically block handoff to another console. We presume - // this was for good reason (compatibility) and give up here. - if (globals.launchArgs.GetForceNoHandoff()) + // If we do not have a registered handoff, do not attempt. + if (!globals.delegationPair.IsCustom()) { return false; } - // Someone double clicked this console or explicitly tried - // to use it to launch a child process. Host it within this one - // and do not hand off. - if (globals.launchArgs.ShouldCreateServerHandle()) + // If we're already a target for receiving another handoff, + // do not chain. + if (globals.handoffTarget) { return false; } @@ -220,21 +208,33 @@ static bool _shouldAttemptHandoff(const Globals& globals, return false; } - // If it is a PTY session, do not attempt handoff. - if (globals.launchArgs.IsHeadless()) + // This console was started with a command line argument to + // specifically block handoff to another console. We presume + // this was for good reason (compatibility) and give up here. + if (globals.launchArgs.GetForceNoHandoff()) { return false; } - // If we do not have a registered handoff, do not attempt. - if (!globals.handoffConsoleClsid) + // Someone double clicked this console or explicitly tried + // to use it to launch a child process. Host it within this one + // and do not hand off. + if (globals.launchArgs.ShouldCreateServerHandle()) { return false; } - // If we're already a target for receiving another handoff, - // do not chain. - if (globals.handoffTarget) + // If it is a PTY session, do not attempt handoff. + if (globals.launchArgs.IsHeadless()) + { + return false; + } + + // Service desktops and non-interactive sessions should not + // try to hand off -- they probably don't have any terminals + // installed, and we don't want to risk breaking a service if + // they *do*. + if (!_isInteractiveUserSession()) { return false; } @@ -269,6 +269,118 @@ static bool _shouldAttemptHandoff(const Globals& globals, #endif } +static void attemptHandoff(Globals& Globals, const CONSOLE_INFORMATION& gci, CONSOLE_API_CONNECTINFO& cac, PCONSOLE_API_MSG pReceiveMsg) +{ + if (!_shouldAttemptHandoff(Globals, gci, cac)) + { + return; + } + + try + { + // Go get ourselves some COM. + auto coinit = wil::CoInitializeEx(COINIT_MULTITHREADED); + + // Get the class/interface to the handoff handler. Local machine only. + ::Microsoft::WRL::ComPtr handoff; + THROW_IF_FAILED(CoCreateInstance(Globals.delegationPair.console, nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff))); + + if (Globals.defaultTerminalMarkerCheckRequired) + { + ::Microsoft::WRL::ComPtr marker; + if (FAILED(handoff.As(&marker))) + { + Globals.delegationPair = DelegationConfig::ConhostDelegationPair; + Globals.defaultTerminalMarkerCheckRequired = false; + return; + } + } + + // Pack up just enough of the attach message for the other console to process it. + // NOTE: It can and will pick up the size/title/etc parameters from the driver again. + CONSOLE_PORTABLE_ATTACH_MSG msg{}; + msg.IdHighPart = pReceiveMsg->Descriptor.Identifier.HighPart; + msg.IdLowPart = pReceiveMsg->Descriptor.Identifier.LowPart; + msg.Process = pReceiveMsg->Descriptor.Process; + msg.Object = pReceiveMsg->Descriptor.Object; + msg.Function = pReceiveMsg->Descriptor.Function; + msg.InputSize = pReceiveMsg->Descriptor.InputSize; + msg.OutputSize = pReceiveMsg->Descriptor.OutputSize; + + // Attempt to get server handle out of our own communication stack to pass it on. + HANDLE serverHandle; + THROW_IF_FAILED(Globals.pDeviceComm->GetServerHandle(&serverHandle)); + + wil::unique_hfile signalPipeTheirSide; + wil::unique_hfile signalPipeOurSide; + + THROW_IF_WIN32_BOOL_FALSE(CreatePipe(signalPipeOurSide.addressof(), signalPipeTheirSide.addressof(), nullptr, 0)); + + // Give a copy of our own process handle to be tracked. + wil::unique_process_handle ourProcess; + THROW_IF_WIN32_BOOL_FALSE(DuplicateHandle(GetCurrentProcess(), + GetCurrentProcess(), + GetCurrentProcess(), + ourProcess.addressof(), + SYNCHRONIZE, + FALSE, + 0)); + + wil::unique_process_handle clientProcess; + + // Okay, moment of truth! If they say they successfully took it over, we're going to clean up. + // If they fail, we'll throw here and it'll log and we'll just start normally. + THROW_IF_FAILED(handoff->EstablishHandoff(serverHandle, + Globals.hInputEvent.get(), + &msg, + signalPipeTheirSide.get(), + ourProcess.get(), + &clientProcess)); + + // Close handles for the things we gave to them + signalPipeTheirSide.reset(); + ourProcess.reset(); + Globals.hInputEvent.reset(); + + // Start a thread to listen for signals from their side that we must relay to the OS. + auto hostSignalThread = std::make_unique(std::move(signalPipeOurSide)); + + // Start it if it was successfully created. + THROW_IF_FAILED(hostSignalThread->Start()); + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "ConsoleHandoffSucceeded", + TraceLoggingDescription("successfully handed off console connection"), + TraceLoggingGuid(Globals.delegationPair.console, "handoffCLSID"), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + + // Unlock in case anything tries to spool down as we exit. + UnlockConsole(); + + // We've handed off responsibility. Wait for child process to exit so we can maintain PID continuity for some clients. + WaitForSingleObject(clientProcess.get(), INFINITE); + + // Exit process to clean up any outstanding things we have open. + ExitProcess(S_OK); + } + catch (...) + { + const auto hr = wil::ResultFromCaughtException(); + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "ConsoleHandoffFailed", + TraceLoggingDescription("failed while attempting handoff"), + TraceLoggingGuid(Globals.delegationPair.console, "handoffCLSID"), + TraceLoggingHResult(hr), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + + // Just log, don't do anything more. We'll move on to launching normally on failure. + LOG_CAUGHT_EXCEPTION(); + } +} + // Routine Description: // - Used when a client application establishes an initial connection to this console server. // - This is supposed to represent accounting for the process, making the appropriate handles, etc. @@ -314,101 +426,7 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API // If we pass the tests... // then attempt to delegate the startup to the registered replacement. - if (_shouldAttemptHandoff(Globals, gci, Cac)) - { - try - { - // Go get ourselves some COM. - auto coinit = wil::CoInitializeEx(COINIT_MULTITHREADED); - - // Get the class/interface to the handoff handler. Local machine only. - ::Microsoft::WRL::ComPtr handoff; - THROW_IF_FAILED(CoCreateInstance(Globals.handoffConsoleClsid.value(), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff))); - - // Pack up just enough of the attach message for the other console to process it. - // NOTE: It can and will pick up the size/title/etc parameters from the driver again. - CONSOLE_PORTABLE_ATTACH_MSG msg{}; - msg.IdHighPart = pReceiveMsg->Descriptor.Identifier.HighPart; - msg.IdLowPart = pReceiveMsg->Descriptor.Identifier.LowPart; - msg.Process = pReceiveMsg->Descriptor.Process; - msg.Object = pReceiveMsg->Descriptor.Object; - msg.Function = pReceiveMsg->Descriptor.Function; - msg.InputSize = pReceiveMsg->Descriptor.InputSize; - msg.OutputSize = pReceiveMsg->Descriptor.OutputSize; - - // Attempt to get server handle out of our own communication stack to pass it on. - HANDLE serverHandle; - THROW_IF_FAILED(Globals.pDeviceComm->GetServerHandle(&serverHandle)); - - wil::unique_hfile signalPipeTheirSide; - wil::unique_hfile signalPipeOurSide; - - THROW_IF_WIN32_BOOL_FALSE(CreatePipe(signalPipeOurSide.addressof(), signalPipeTheirSide.addressof(), nullptr, 0)); - - // Give a copy of our own process handle to be tracked. - wil::unique_process_handle ourProcess; - THROW_IF_WIN32_BOOL_FALSE(DuplicateHandle(GetCurrentProcess(), - GetCurrentProcess(), - GetCurrentProcess(), - ourProcess.addressof(), - SYNCHRONIZE, - FALSE, - 0)); - - wil::unique_process_handle clientProcess; - - // Okay, moment of truth! If they say they successfully took it over, we're going to clean up. - // If they fail, we'll throw here and it'll log and we'll just start normally. - THROW_IF_FAILED(handoff->EstablishHandoff(serverHandle, - Globals.hInputEvent.get(), - &msg, - signalPipeTheirSide.get(), - ourProcess.get(), - &clientProcess)); - - // Close handles for the things we gave to them - signalPipeTheirSide.reset(); - ourProcess.reset(); - Globals.hInputEvent.reset(); - - // Start a thread to listen for signals from their side that we must relay to the OS. - auto hostSignalThread = std::make_unique(std::move(signalPipeOurSide)); - - // Start it if it was successfully created. - THROW_IF_FAILED(hostSignalThread->Start()); - - TraceLoggingWrite(g_hConhostV2EventTraceProvider, - "ConsoleHandoffSucceeded", - TraceLoggingDescription("successfully handed off console connection"), - TraceLoggingGuid(Globals.handoffConsoleClsid.value(), "handoffCLSID"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - - // Unlock in case anything tries to spool down as we exit. - UnlockConsole(); - - // We've handed off responsibility. Wait for child process to exit so we can maintain PID continuity for some clients. - WaitForSingleObject(clientProcess.get(), INFINITE); - - // Exit process to clean up any outstanding things we have open. - ExitProcess(S_OK); - } - catch (...) - { - const auto hr = wil::ResultFromCaughtException(); - - TraceLoggingWrite(g_hConhostV2EventTraceProvider, - "ConsoleHandoffFailed", - TraceLoggingDescription("failed while attempting handoff"), - TraceLoggingGuid(Globals.handoffConsoleClsid.value(), "handoffCLSID"), - TraceLoggingHResult(hr), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - - // Just log, don't do anything more. We'll move on to launching normally on failure. - LOG_CAUGHT_EXCEPTION(); - } - } + attemptHandoff(Globals, gci, Cac, pReceiveMsg); Status = NTSTATUS_FROM_HRESULT(gci.ProcessHandleList.AllocProcessData(dwProcessId, dwThreadId, From 33a69fe62be287e22b97b60d73f7eefa3f101004 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 6 Jun 2022 17:10:46 +0200 Subject: [PATCH 2/4] Improve layout of xaml dropdown --- src/cascadia/TerminalSettingsEditor/Launch.xaml | 11 ++++++++--- .../Resources/en-US/Resources.resw | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalSettingsEditor/Launch.xaml b/src/cascadia/TerminalSettingsEditor/Launch.xaml index fe0f5607f9e..3f3f653ade0 100644 --- a/src/cascadia/TerminalSettingsEditor/Launch.xaml +++ b/src/cascadia/TerminalSettingsEditor/Launch.xaml @@ -93,7 +93,7 @@ - + @@ -109,20 +109,25 @@ + Text="{x:Bind Author}" + Visibility="{x:Bind local:Converters.StringNotEmptyToVisibility(Author)}" /> + Text="{x:Bind Version}" + Visibility="{x:Bind local:Converters.StringNotEmptyToVisibility(Version)}" /> diff --git a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw index c0413ae8bba..ecae488bcc4 100644 --- a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw @@ -494,7 +494,7 @@ A command to clear the part of the buffer above the viewport - Microsoft Corporation + Paired with `DefaultWindowsConsoleName`, this is the application author... which is us: Microsoft. From 23c1b9601401c2911f861289658e31d6ef0bb260 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 7 Jun 2022 20:53:04 +0200 Subject: [PATCH 3/4] Add comments --- src/host/srvinit.cpp | 6 ++++++ src/propslib/DelegationConfig.cpp | 6 +++++- src/server/IoDispatchers.cpp | 4 ++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index 1b6d5b5179b..e3c52a999f0 100644 --- a/src/host/srvinit.cpp +++ b/src/host/srvinit.cpp @@ -78,6 +78,9 @@ try TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); } + // If we looked up the registered defterm pair, and it was left as the default (missing or {0}), + // AND velocity is enabled for DxD, then we switch the delegation pair to Terminal and + // mark that we should check that class for the marker interface later. if (Globals.delegationPair.IsDefault() && Microsoft::Console::Internal::DefaultApp::CheckShouldTerminalBeDefault()) { Globals.delegationPair = DelegationConfig::TerminalDelegationPair; @@ -424,6 +427,9 @@ try g.handoffTarget = true; g.delegationPair = DelegationConfig::s_GetDelegationPair(); + // We've been handed off to (we're OpenConsole, not conhost). + // If we get here and there's not a custom defterm set, then it must be because + // conhost defaulted to us for DxD. Set up Terminal as the thing to handoff too. if (!g.delegationPair.IsCustom()) { g.delegationPair = DelegationConfig::TerminalDelegationPair; diff --git a/src/propslib/DelegationConfig.cpp b/src/propslib/DelegationConfig.cpp index 2733094de24..2f3074894b7 100644 --- a/src/propslib/DelegationConfig.cpp +++ b/src/propslib/DelegationConfig.cpp @@ -194,7 +194,9 @@ try // We should find at least one package. RETURN_HR_IF(E_FAIL, packages.empty()); - // Search through and find a package that matches. + // Get the currently set default console/terminal. + // Then, search through and find a package that matches. + // If we find one, then return it. const auto delegationPair = s_GetDelegationPair(); for (auto& pkg : packages) { @@ -240,6 +242,8 @@ CATCH_RETURN() } static constexpr const wchar_t* keys[2]{ DELEGATION_CONSOLE_KEY_NAME, DELEGATION_TERMINAL_KEY_NAME }; + // values[0]/[1] will contain the delegation console/terminal + // respectively if set to a valid value within the registry. IID values[2]{ CLSID_Default, CLSID_Default }; for (size_t i = 0; i < 2; ++i) diff --git a/src/server/IoDispatchers.cpp b/src/server/IoDispatchers.cpp index f08e840fd1d..8c032a90b24 100644 --- a/src/server/IoDispatchers.cpp +++ b/src/server/IoDispatchers.cpp @@ -285,6 +285,10 @@ static void attemptHandoff(Globals& Globals, const CONSOLE_INFORMATION& gci, CON ::Microsoft::WRL::ComPtr handoff; THROW_IF_FAILED(CoCreateInstance(Globals.delegationPair.console, nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff))); + // If we looked up the registered defterm pair, and it was left as the default (missing or {0}), + // AND velocity is enabled for DxD, then we switched the delegation pair to Terminal, with a notice + // that we still need to check whether Terminal actually wants to be the default Terminal. + // See ConsoleServerInitialization. if (Globals.defaultTerminalMarkerCheckRequired) { ::Microsoft::WRL::ComPtr marker; From 8a80ac1219265e803c1c8be90848d43ffec4907a Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 7 Jun 2022 20:56:53 +0200 Subject: [PATCH 4/4] Address feedback --- src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp | 2 +- .../TerminalSettingsModel/Resources/en-US/Resources.resw | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp b/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp index d34c6613fbc..0ce17eb725d 100644 --- a/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp +++ b/src/cascadia/TerminalSettingsModel/DefaultTerminal.cpp @@ -47,7 +47,7 @@ winrt::hstring DefaultTerminal::Author() const switch (_pkg.pair.kind) { case DelegationConfig::DelegationPairKind::Default: - return RS_(L"DefaultWindowsConsoleAuthor"); + return {}; // The "Let Windows decide" option has no author. case DelegationConfig::DelegationPairKind::Conhost: return RS_(L"InboxWindowsConsoleAuthor"); default: diff --git a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw index ecae488bcc4..9346dd95036 100644 --- a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw @@ -493,10 +493,6 @@ Clear scrollback A command to clear the part of the buffer above the viewport - - - Paired with `DefaultWindowsConsoleName`, this is the application author... which is us: Microsoft. - Let Windows decide This is the default option if a user doesn't choose to override Microsoft's choice of a Terminal.