From 3cfb5253aa4a9b4e5698e05982593f5effbee073 Mon Sep 17 00:00:00 2001 From: Adam Gashlin Date: Mon, 7 Dec 2020 23:20:53 +0000 Subject: [PATCH] Bug 1672957 Do not create or modify a shortcut for the WDBA. r=bytesized Fixes WinToast failing to find the shortcut to check its AUMI because: 1) the app name given to WinToast did not match the shortcut name, and 2) it wasn't looking in the system-wide Start Menu. Further, adds an option to fail if shortcut isn't found, rather than create it, because: a) the shortcut would be to the WDBA, which is not useful, and anyway b) we're showing the notification immediately after initializing WinToast, which is too soon for Windows to reliably pick up on it, so it usually doesn't have the desired effect of allowing the custom abstract model. Differential Revision: https://phabricator.services.mozilla.com/D98838 --- .../WinToast/moz-check-system-shortcut.patch | 81 +++++++++++++ .../moz-disable-create-shortcut.patch | 110 ++++++++++++++++++ third_party/WinToast/moz.yaml | 2 + third_party/WinToast/wintoastlib.cpp | 73 ++++++++---- third_party/WinToast/wintoastlib.h | 12 ++ toolkit/mozapps/defaultagent/Notification.cpp | 4 +- 6 files changed, 259 insertions(+), 23 deletions(-) create mode 100644 third_party/WinToast/moz-check-system-shortcut.patch create mode 100644 third_party/WinToast/moz-disable-create-shortcut.patch diff --git a/third_party/WinToast/moz-check-system-shortcut.patch b/third_party/WinToast/moz-check-system-shortcut.patch new file mode 100644 index 000000000000..84411ae7bc1b --- /dev/null +++ b/third_party/WinToast/moz-check-system-shortcut.patch @@ -0,0 +1,81 @@ +diff --git a/src/wintoastlib.cpp b/src/wintoastlib.cpp +index 0895ff7..ac8d5cf 100644 +--- a/src/wintoastlib.cpp ++++ b/src/wintoastlib.cpp +@@ -213,8 +213,8 @@ namespace Util { + } + + +- inline HRESULT defaultShellLinksDirectory(_In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { +- DWORD written = GetEnvironmentVariableW(L"APPDATA", path, nSize); ++ inline HRESULT commonShellLinksDirectory(_In_ const WCHAR* baseEnv, _In_ WCHAR* path, _In_ DWORD nSize) { ++ DWORD written = GetEnvironmentVariableW(baseEnv, path, nSize); + HRESULT hr = written > 0 ? S_OK : E_INVALIDARG; + if (SUCCEEDED(hr)) { + errno_t result = wcscat_s(path, nSize, DEFAULT_SHELL_LINKS_PATH); +@@ -224,8 +224,8 @@ namespace Util { + return hr; + } + +- inline HRESULT defaultShellLinkPath(const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { +- HRESULT hr = defaultShellLinksDirectory(path, nSize); ++ inline HRESULT commonShellLinkPath(_In_ const WCHAR* baseEnv, const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize) { ++ HRESULT hr = commonShellLinksDirectory(baseEnv, path, nSize); + if (SUCCEEDED(hr)) { + const std::wstring appLink(appname + DEFAULT_LINK_FORMAT); + errno_t result = wcscat_s(path, nSize, appLink.c_str()); +@@ -235,6 +235,13 @@ namespace Util { + return hr; + } + ++ inline HRESULT defaultUserShellLinkPath(const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { ++ return commonShellLinkPath(L"APPDATA", appname, path, nSize); ++ } ++ ++ inline HRESULT defaultSystemShellLinkPath(const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { ++ return commonShellLinkPath(L"PROGRAMDATA", appname, path, nSize); ++ } + + inline PCWSTR AsString(ComPtr &xmlDocument) { + HSTRING xml; +@@ -523,12 +530,19 @@ const std::wstring& WinToast::appUserModelId() const { + + HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { + WCHAR path[MAX_PATH] = { L'\0' }; +- Util::defaultShellLinkPath(_appName, path); ++ Util::defaultUserShellLinkPath(_appName, path); + // Check if the file exist + DWORD attr = GetFileAttributesW(path); + if (attr >= 0xFFFFFFF) { +- DEBUG_MSG("Error, shell link not found. Try to create a new one in: " << path); +- return E_FAIL; ++ // The shortcut may be in the system Start Menu. ++ WCHAR systemPath[MAX_PATH] = { L'\0' }; ++ Util::defaultSystemShellLinkPath(_appName, systemPath); ++ attr = GetFileAttributesW(systemPath); ++ if (attr >= 0xFFFFFFF) { ++ DEBUG_MSG("Error, shell link not found. Try to create a new one in: " << path); ++ return E_FAIL; ++ } ++ wcscpy(path, systemPath); + } + + // Let's load the file as shell link to validate. +@@ -543,7 +557,7 @@ HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { + ComPtr persistFile; + hr = shellLink.As(&persistFile); + if (SUCCEEDED(hr)) { +- hr = persistFile->Load(path, STGM_READWRITE); ++ hr = persistFile->Load(path, STGM_READ); + if (SUCCEEDED(hr)) { + ComPtr propertyStore; + hr = shellLink.As(&propertyStore); +@@ -583,7 +597,7 @@ HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { + HRESULT WinToast::createShellLinkHelper() { + WCHAR exePath[MAX_PATH]{L'\0'}; + WCHAR slPath[MAX_PATH]{L'\0'}; +- Util::defaultShellLinkPath(_appName, slPath); ++ Util::defaultUserShellLinkPath(_appName, slPath); + Util::defaultExecutablePath(exePath); + ComPtr shellLink; + HRESULT hr = CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&shellLink)); diff --git a/third_party/WinToast/moz-disable-create-shortcut.patch b/third_party/WinToast/moz-disable-create-shortcut.patch new file mode 100644 index 000000000000..96ccac27320a --- /dev/null +++ b/third_party/WinToast/moz-disable-create-shortcut.patch @@ -0,0 +1,110 @@ +diff --git a/src/wintoastlib.cpp b/src/wintoastlib.cpp +index 0895ff7..52de554 100644 +--- a/src/wintoastlib.cpp ++++ b/src/wintoastlib.cpp +@@ -391,6 +391,10 @@ void WinToast::setAppUserModelId(_In_ const std::wstring& aumi) { + DEBUG_MSG(L"Default App User Model Id: " << _aumi.c_str()); + } + ++void WinToast::setShortcutPolicy(_In_ ShortcutPolicy shortcutPolicy) { ++ _shortcutPolicy = shortcutPolicy; ++} ++ + bool WinToast::isCompatible() { + DllImporter::initialize(); + return !((DllImporter::SetCurrentProcessExplicitAppUserModelID == nullptr) +@@ -492,10 +496,12 @@ bool WinToast::initialize(_Out_ WinToastError* error) { + return false; + } + +- if (createShortcut() < 0) { +- setError(error, WinToastError::ShellLinkNotCreated); +- DEBUG_MSG(L"Error while attaching the AUMI to the current proccess =("); +- return false; ++ if (_shortcutPolicy != SHORTCUT_POLICY_IGNORE) { ++ if (createShortcut() < 0) { ++ setError(error, WinToastError::ShellLinkNotCreated); ++ DEBUG_MSG(L"Error while attaching the AUMI to the current proccess =("); ++ return false; ++ } + } + + if (FAILED(DllImporter::SetCurrentProcessExplicitAppUserModelID(_aumi.c_str()))) { +@@ -555,18 +561,23 @@ HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { + hr = DllImporter::PropVariantToString(appIdPropVar, AUMI, MAX_PATH); + wasChanged = false; + if (FAILED(hr) || _aumi != AUMI) { +- // AUMI Changed for the same app, let's update the current value! =) +- wasChanged = true; +- PropVariantClear(&appIdPropVar); +- hr = InitPropVariantFromString(_aumi.c_str(), &appIdPropVar); +- if (SUCCEEDED(hr)) { +- hr = propertyStore->SetValue(PKEY_AppUserModel_ID, appIdPropVar); ++ if (_shortcutPolicy == SHORTCUT_POLICY_REQUIRE_CREATE) { ++ // AUMI Changed for the same app, let's update the current value! =) ++ wasChanged = true; ++ PropVariantClear(&appIdPropVar); ++ hr = InitPropVariantFromString(_aumi.c_str(), &appIdPropVar); + if (SUCCEEDED(hr)) { +- hr = propertyStore->Commit(); +- if (SUCCEEDED(hr) && SUCCEEDED(persistFile->IsDirty())) { +- hr = persistFile->Save(path, TRUE); ++ hr = propertyStore->SetValue(PKEY_AppUserModel_ID, appIdPropVar); ++ if (SUCCEEDED(hr)) { ++ hr = propertyStore->Commit(); ++ if (SUCCEEDED(hr) && SUCCEEDED(persistFile->IsDirty())) { ++ hr = persistFile->Save(path, TRUE); ++ } + } + } ++ } else { ++ // Not allowed to touch the shortcut to fix the AUMI ++ hr = E_FAIL; + } + } + PropVariantClear(&appIdPropVar); +@@ -581,6 +592,10 @@ HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { + + + HRESULT WinToast::createShellLinkHelper() { ++ if (_shortcutPolicy != SHORTCUT_POLICY_REQUIRE_CREATE) { ++ return E_FAIL; ++ } ++ + WCHAR exePath[MAX_PATH]{L'\0'}; + WCHAR slPath[MAX_PATH]{L'\0'}; + Util::defaultShellLinkPath(_appName, slPath); +diff --git a/src/wintoastlib.h b/src/wintoastlib.h +index 68b1cb1..dc8d745 100644 +--- a/src/wintoastlib.h ++++ b/src/wintoastlib.h +@@ -173,6 +173,16 @@ namespace WinToastLib { + SHORTCUT_CREATE_FAILED = -4 + }; + ++ enum ShortcutPolicy { ++ /* Don't check, create, or modify a shortcut. */ ++ SHORTCUT_POLICY_IGNORE = 0, ++ /* Require a shortcut with matching AUMI, don't create or modify an existing one. */ ++ SHORTCUT_POLICY_REQUIRE_NO_CREATE = 1, ++ /* Require a shortcut with matching AUMI, create if missing, modify if not matching. ++ * This is the default. */ ++ SHORTCUT_POLICY_REQUIRE_CREATE = 2, ++ }; ++ + WinToast(void); + virtual ~WinToast(); + static WinToast* instance(); +@@ -194,10 +204,12 @@ namespace WinToastLib { + const std::wstring& appUserModelId() const; + void setAppUserModelId(_In_ const std::wstring& aumi); + void setAppName(_In_ const std::wstring& appName); ++ void setShortcutPolicy(_In_ ShortcutPolicy policy); + + protected: + bool _isInitialized{false}; + bool _hasCoInitialized{false}; ++ ShortcutPolicy _shortcutPolicy{SHORTCUT_POLICY_REQUIRE_CREATE}; + std::wstring _appName{}; + std::wstring _aumi{}; + std::map> _buffer{}; diff --git a/third_party/WinToast/moz.yaml b/third_party/WinToast/moz.yaml index 4faba12c99de..5cfbb804b6c7 100644 --- a/third_party/WinToast/moz.yaml +++ b/third_party/WinToast/moz.yaml @@ -5,6 +5,8 @@ schema: 1 # cd third_pary/WinToast # wget https://raw.githubusercontent.com/mohabouje/WinToast/master/src/wintoastlib.cpp # wget https://raw.githubusercontent.com/mohabouje/WinToast/master/src/wintoastlib.h +# patch -p2 < moz-check-system-shortcut.patch +# patch -p2 < moz-disable-create-shortcut.patch bugzilla: # Bugzilla product and component for this directory and subdirectories diff --git a/third_party/WinToast/wintoastlib.cpp b/third_party/WinToast/wintoastlib.cpp index 0895ff734353..7e8c307e91a4 100644 --- a/third_party/WinToast/wintoastlib.cpp +++ b/third_party/WinToast/wintoastlib.cpp @@ -213,8 +213,8 @@ namespace Util { } - inline HRESULT defaultShellLinksDirectory(_In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { - DWORD written = GetEnvironmentVariableW(L"APPDATA", path, nSize); + inline HRESULT commonShellLinksDirectory(_In_ const WCHAR* baseEnv, _In_ WCHAR* path, _In_ DWORD nSize) { + DWORD written = GetEnvironmentVariableW(baseEnv, path, nSize); HRESULT hr = written > 0 ? S_OK : E_INVALIDARG; if (SUCCEEDED(hr)) { errno_t result = wcscat_s(path, nSize, DEFAULT_SHELL_LINKS_PATH); @@ -224,8 +224,8 @@ namespace Util { return hr; } - inline HRESULT defaultShellLinkPath(const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { - HRESULT hr = defaultShellLinksDirectory(path, nSize); + inline HRESULT commonShellLinkPath(_In_ const WCHAR* baseEnv, const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize) { + HRESULT hr = commonShellLinksDirectory(baseEnv, path, nSize); if (SUCCEEDED(hr)) { const std::wstring appLink(appname + DEFAULT_LINK_FORMAT); errno_t result = wcscat_s(path, nSize, appLink.c_str()); @@ -235,6 +235,13 @@ namespace Util { return hr; } + inline HRESULT defaultUserShellLinkPath(const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { + return commonShellLinkPath(L"APPDATA", appname, path, nSize); + } + + inline HRESULT defaultSystemShellLinkPath(const std::wstring& appname, _In_ WCHAR* path, _In_ DWORD nSize = MAX_PATH) { + return commonShellLinkPath(L"PROGRAMDATA", appname, path, nSize); + } inline PCWSTR AsString(ComPtr &xmlDocument) { HSTRING xml; @@ -391,6 +398,10 @@ void WinToast::setAppUserModelId(_In_ const std::wstring& aumi) { DEBUG_MSG(L"Default App User Model Id: " << _aumi.c_str()); } +void WinToast::setShortcutPolicy(_In_ ShortcutPolicy shortcutPolicy) { + _shortcutPolicy = shortcutPolicy; +} + bool WinToast::isCompatible() { DllImporter::initialize(); return !((DllImporter::SetCurrentProcessExplicitAppUserModelID == nullptr) @@ -492,10 +503,12 @@ bool WinToast::initialize(_Out_ WinToastError* error) { return false; } - if (createShortcut() < 0) { - setError(error, WinToastError::ShellLinkNotCreated); - DEBUG_MSG(L"Error while attaching the AUMI to the current proccess =("); - return false; + if (_shortcutPolicy != SHORTCUT_POLICY_IGNORE) { + if (createShortcut() < 0) { + setError(error, WinToastError::ShellLinkNotCreated); + DEBUG_MSG(L"Error while attaching the AUMI to the current proccess =("); + return false; + } } if (FAILED(DllImporter::SetCurrentProcessExplicitAppUserModelID(_aumi.c_str()))) { @@ -523,12 +536,19 @@ const std::wstring& WinToast::appUserModelId() const { HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { WCHAR path[MAX_PATH] = { L'\0' }; - Util::defaultShellLinkPath(_appName, path); + Util::defaultUserShellLinkPath(_appName, path); // Check if the file exist DWORD attr = GetFileAttributesW(path); if (attr >= 0xFFFFFFF) { - DEBUG_MSG("Error, shell link not found. Try to create a new one in: " << path); - return E_FAIL; + // The shortcut may be in the system Start Menu. + WCHAR systemPath[MAX_PATH] = { L'\0' }; + Util::defaultSystemShellLinkPath(_appName, systemPath); + attr = GetFileAttributesW(systemPath); + if (attr >= 0xFFFFFFF) { + DEBUG_MSG("Error, shell link not found. Try to create a new one in: " << path); + return E_FAIL; + } + wcscpy(path, systemPath); } // Let's load the file as shell link to validate. @@ -543,7 +563,7 @@ HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { ComPtr persistFile; hr = shellLink.As(&persistFile); if (SUCCEEDED(hr)) { - hr = persistFile->Load(path, STGM_READWRITE); + hr = persistFile->Load(path, STGM_READ); if (SUCCEEDED(hr)) { ComPtr propertyStore; hr = shellLink.As(&propertyStore); @@ -555,18 +575,23 @@ HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { hr = DllImporter::PropVariantToString(appIdPropVar, AUMI, MAX_PATH); wasChanged = false; if (FAILED(hr) || _aumi != AUMI) { - // AUMI Changed for the same app, let's update the current value! =) - wasChanged = true; - PropVariantClear(&appIdPropVar); - hr = InitPropVariantFromString(_aumi.c_str(), &appIdPropVar); - if (SUCCEEDED(hr)) { - hr = propertyStore->SetValue(PKEY_AppUserModel_ID, appIdPropVar); + if (_shortcutPolicy == SHORTCUT_POLICY_REQUIRE_CREATE) { + // AUMI Changed for the same app, let's update the current value! =) + wasChanged = true; + PropVariantClear(&appIdPropVar); + hr = InitPropVariantFromString(_aumi.c_str(), &appIdPropVar); if (SUCCEEDED(hr)) { - hr = propertyStore->Commit(); - if (SUCCEEDED(hr) && SUCCEEDED(persistFile->IsDirty())) { - hr = persistFile->Save(path, TRUE); + hr = propertyStore->SetValue(PKEY_AppUserModel_ID, appIdPropVar); + if (SUCCEEDED(hr)) { + hr = propertyStore->Commit(); + if (SUCCEEDED(hr) && SUCCEEDED(persistFile->IsDirty())) { + hr = persistFile->Save(path, TRUE); + } } } + } else { + // Not allowed to touch the shortcut to fix the AUMI + hr = E_FAIL; } } PropVariantClear(&appIdPropVar); @@ -581,9 +606,13 @@ HRESULT WinToast::validateShellLinkHelper(_Out_ bool& wasChanged) { HRESULT WinToast::createShellLinkHelper() { + if (_shortcutPolicy != SHORTCUT_POLICY_REQUIRE_CREATE) { + return E_FAIL; + } + WCHAR exePath[MAX_PATH]{L'\0'}; WCHAR slPath[MAX_PATH]{L'\0'}; - Util::defaultShellLinkPath(_appName, slPath); + Util::defaultUserShellLinkPath(_appName, slPath); Util::defaultExecutablePath(exePath); ComPtr shellLink; HRESULT hr = CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&shellLink)); diff --git a/third_party/WinToast/wintoastlib.h b/third_party/WinToast/wintoastlib.h index 68b1cb1d5ce6..dc8d7456c2a1 100644 --- a/third_party/WinToast/wintoastlib.h +++ b/third_party/WinToast/wintoastlib.h @@ -173,6 +173,16 @@ namespace WinToastLib { SHORTCUT_CREATE_FAILED = -4 }; + enum ShortcutPolicy { + /* Don't check, create, or modify a shortcut. */ + SHORTCUT_POLICY_IGNORE = 0, + /* Require a shortcut with matching AUMI, don't create or modify an existing one. */ + SHORTCUT_POLICY_REQUIRE_NO_CREATE = 1, + /* Require a shortcut with matching AUMI, create if missing, modify if not matching. + * This is the default. */ + SHORTCUT_POLICY_REQUIRE_CREATE = 2, + }; + WinToast(void); virtual ~WinToast(); static WinToast* instance(); @@ -194,10 +204,12 @@ namespace WinToastLib { const std::wstring& appUserModelId() const; void setAppUserModelId(_In_ const std::wstring& aumi); void setAppName(_In_ const std::wstring& appName); + void setShortcutPolicy(_In_ ShortcutPolicy policy); protected: bool _isInitialized{false}; bool _hasCoInitialized{false}; + ShortcutPolicy _shortcutPolicy{SHORTCUT_POLICY_REQUIRE_CREATE}; std::wstring _appName{}; std::wstring _aumi{}; std::map> _buffer{}; diff --git a/toolkit/mozapps/defaultagent/Notification.cpp b/toolkit/mozapps/defaultagent/Notification.cpp index 9f53e8b6b9ba..3ca18a7c1d82 100644 --- a/toolkit/mozapps/defaultagent/Notification.cpp +++ b/toolkit/mozapps/defaultagent/Notification.cpp @@ -394,9 +394,11 @@ static NotificationActivities ShowNotification( return activitiesPerformed; } - WinToast::instance()->setAppName(L"" MOZ_APP_BASENAME); + WinToast::instance()->setAppName(L"" MOZ_APP_DISPLAYNAME); std::wstring aumiStr = aumi; WinToast::instance()->setAppUserModelId(aumiStr); + WinToast::instance()->setShortcutPolicy( + WinToastLib::WinToast::SHORTCUT_POLICY_REQUIRE_NO_CREATE); WinToast::WinToastError error; if (!WinToast::instance()->initialize(&error)) { LOG_ERROR_MESSAGE(WinToast::strerror(error).c_str());