From 113d8732bc5ae1f0acac810d31e42975ac2897bf Mon Sep 17 00:00:00 2001 From: Chris Tapley Date: Sun, 7 Feb 2021 19:39:08 +0800 Subject: [PATCH] implement env vars in settings (#2785) * use StringMap as a container * recursively resolve ${env:NAME} vars in values in the StringMap or in the process' environment * implement some tests --- .../LocalTests_SettingsModel/ProfileTests.cpp | 101 ++++++++++++++++++ src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- src/cascadia/TerminalApp/TerminalSettings.cpp | 5 + src/cascadia/TerminalApp/TerminalSettings.h | 5 +- .../TerminalControl/IControlSettings.idl | 2 +- .../TerminalSettingsEditor/Profiles.h | 1 + .../TerminalSettingsEditor/Profiles.idl | 1 + .../TerminalSettingsModel/Profile.cpp | 90 ++++++++++++++++ src/cascadia/TerminalSettingsModel/Profile.h | 5 + .../TerminalSettingsModel/Profile.idl | 4 + 10 files changed, 213 insertions(+), 3 deletions(-) diff --git a/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp b/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp index d157ad9cd8b..0775c22e321 100644 --- a/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp @@ -36,6 +36,11 @@ namespace SettingsModelLocalTests TEST_METHOD(LayerProfileProperties); TEST_METHOD(LayerProfileIcon); TEST_METHOD(LayerProfilesOnArray); + TEST_METHOD(ProfileWithEnvVarSelfRefKeyThrows); + TEST_METHOD(ProfileWithEnvVarCircularRefsThrows); + TEST_METHOD(ProfileWithEnvVarNoReferences); + TEST_METHOD(ProfileWithEnvVarProcessEnv); + TEST_METHOD(ProfileWithEnvVarComplex); TEST_CLASS_SETUP(ClassSetup) { @@ -307,4 +312,100 @@ namespace SettingsModelLocalTests VERIFY_ARE_EQUAL(L"profile4", settings->_allProfiles.GetAt(0).Name()); } + void ProfileTests::ProfileWithEnvVarSelfRefKeyThrows() + { + const std::string profileString{ R"({ + "name": "profile0", + "guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "environment": { + "VAR_1": "${env:VAR_1}" + } + })" }; + VERIFY_THROWS(implementation::Profile::FromJson(VerifyParseSucceeded(profileString)), winrt::hresult_error); + } + + void ProfileTests::ProfileWithEnvVarCircularRefsThrows() + { + const std::string profileString{ R"({ + "name": "profile0", + "guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "environment": { + "VAR_1": "${env:VAR_2}", + "VAR_2": "${env:VAR_3}", + "VAR_3": "${env:VAR_1}" + } + })" }; + VERIFY_THROWS(implementation::Profile::FromJson(VerifyParseSucceeded(profileString)), winrt::hresult_error); + } + + void ProfileTests::ProfileWithEnvVarNoReferences() + { + const std::string profileString{ R"({ + "name": "profile0", + "guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "environment": { + "VAR_1": "value1", + "VAR_2": "value2", + "VAR_3": "value3" + } + })" }; + const auto profile = implementation::Profile::FromJson(VerifyParseSucceeded(profileString)); + const auto envMap = profile->EnvironmentVariables(); + VERIFY_ARE_EQUAL(static_cast(3), envMap.Size()); + VERIFY_ARE_EQUAL(L"value1", envMap.Lookup(L"VAR_1")); + VERIFY_ARE_EQUAL(L"value2", envMap.Lookup(L"VAR_2")); + VERIFY_ARE_EQUAL(L"value3", envMap.Lookup(L"VAR_3")); + } + + void ProfileTests::ProfileWithEnvVarProcessEnv() + { + const std::string profileString{ R"({ + "name": "profile0", + "guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "environment": { + "VAR_1": "${env:SystemRoot}", + "VAR_2": "${env:___DOES_NOT_EXIST_IN_ENVIRONMENT___1234567890}" + } + })" }; + + std::wstring expectedSystemRoot(static_cast(MAX_PATH), L'\0'); + VERIFY_IS_TRUE(GetEnvironmentVariableW(L"SystemRoot", expectedSystemRoot.data(), MAX_PATH) > 0); + expectedSystemRoot.erase(std::find_if(expectedSystemRoot.rbegin(), expectedSystemRoot.rend(), [](wchar_t ch) { + return ch != L'\0'; + }).base(), + expectedSystemRoot.end()); + const auto profile = implementation::Profile::FromJson(VerifyParseSucceeded(profileString)); + const auto envMap = profile->EnvironmentVariables(); + VERIFY_ARE_EQUAL(static_cast(2), envMap.Size()); + VERIFY_ARE_EQUAL(expectedSystemRoot, envMap.Lookup(L"VAR_1")); + VERIFY_ARE_EQUAL(L"", envMap.Lookup(L"VAR_2")); + } + + void ProfileTests::ProfileWithEnvVarComplex() + { + const std::string profileString{ R"({ + "name": "profile0", + "guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}", + "environment": { + "TEST_HOME_DRIVE": "C:", + "TEST_HOME": "${env:TEST_HOME_DRIVE}${env:TEST_HOME_PATH}", + "TEST_HOME_PATH": "\\Users\\example", + "PARAM_START": "${env:TEST_HOME} abc", + "PARAM_MIDDLE": "abc ${env:TEST_HOME} abc", + "PARAM_END": "abc ${env:TEST_HOME}", + "PARAM_MULTIPLE": "${env:PARAM_START};${env:PARAM_MIDDLE};${env:PARAM_END}", + "PARAM_MULTIPLE_SAME": "${env:TEST_HOME_DRIVE};${env:TEST_HOME_DRIVE};${env:TEST_HOME_DRIVE};${env:TEST_HOME_DRIVE};${env:TEST_HOME_DRIVE};" + } + })" }; + + const auto profile = implementation::Profile::FromJson(VerifyParseSucceeded(profileString)); + const auto envMap = profile->EnvironmentVariables(); + VERIFY_ARE_EQUAL(static_cast(8), envMap.Size()); + VERIFY_ARE_EQUAL(L"C:\\Users\\example", envMap.Lookup(L"TEST_HOME")); + VERIFY_ARE_EQUAL(L"C:\\Users\\example abc", envMap.Lookup(L"PARAM_START")); + VERIFY_ARE_EQUAL(L"abc C:\\Users\\example abc", envMap.Lookup(L"PARAM_MIDDLE")); + VERIFY_ARE_EQUAL(L"abc C:\\Users\\example", envMap.Lookup(L"PARAM_END")); + VERIFY_ARE_EQUAL(L"C:\\Users\\example abc;abc C:\\Users\\example abc;abc C:\\Users\\example", envMap.Lookup(L"PARAM_MULTIPLE")); + VERIFY_ARE_EQUAL(L"C:;C:;C:;C:;C:;", envMap.Lookup(L"PARAM_MULTIPLE_SAME")); + } } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 22ec513eb95..5ad25e95a76 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -907,7 +907,7 @@ namespace winrt::TerminalApp::implementation { std::wstring guidWString = Utils::GuidToString(profileGuid); - StringMap envMap{}; + auto envMap = settings.EnvironmentVariables(); envMap.Insert(L"WT_PROFILE_ID", guidWString); envMap.Insert(L"WSLENV", L"WT_PROFILE_ID"); diff --git a/src/cascadia/TerminalApp/TerminalSettings.cpp b/src/cascadia/TerminalApp/TerminalSettings.cpp index 2f3cb99b0aa..c693160a94f 100644 --- a/src/cascadia/TerminalApp/TerminalSettings.cpp +++ b/src/cascadia/TerminalApp/TerminalSettings.cpp @@ -193,6 +193,11 @@ namespace winrt::TerminalApp::implementation const til::color colorRef{ profile.TabColor().Value() }; _TabColor = static_cast(colorRef); } + + if (profile.HasEnvironmentVariables()) + { + _EnvironmentVariables = profile.EnvironmentVariables(); + } } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalSettings.h b/src/cascadia/TerminalApp/TerminalSettings.h index 34d83fc28ab..70f4993b963 100644 --- a/src/cascadia/TerminalApp/TerminalSettings.h +++ b/src/cascadia/TerminalApp/TerminalSettings.h @@ -19,6 +19,9 @@ Author(s): #include "../inc/cppwinrt_utils.h" #include #include +#include + +using namespace winrt::Windows::Foundation::Collections; // fwdecl unittest classes namespace TerminalAppLocalTests @@ -108,7 +111,7 @@ namespace winrt::TerminalApp::implementation GETSET_SETTING(hstring, StartingDirectory); GETSET_SETTING(hstring, StartingTitle); GETSET_SETTING(bool, SuppressApplicationTitle); - GETSET_SETTING(hstring, EnvironmentVariables); + GETSET_SETTING(StringMap, EnvironmentVariables); GETSET_SETTING(Microsoft::Terminal::TerminalControl::ScrollbarState, ScrollState, Microsoft::Terminal::TerminalControl::ScrollbarState::Visible); diff --git a/src/cascadia/TerminalControl/IControlSettings.idl b/src/cascadia/TerminalControl/IControlSettings.idl index 0c80d68b1e3..a6a7e37d217 100644 --- a/src/cascadia/TerminalControl/IControlSettings.idl +++ b/src/cascadia/TerminalControl/IControlSettings.idl @@ -43,7 +43,7 @@ namespace Microsoft.Terminal.TerminalControl String Commandline; String StartingDirectory; - String EnvironmentVariables; + Windows.Foundation.Collections.StringMap EnvironmentVariables; String BackgroundImage; Double BackgroundImageOpacity; diff --git a/src/cascadia/TerminalSettingsEditor/Profiles.h b/src/cascadia/TerminalSettingsEditor/Profiles.h index cec7008778e..f34d44ccc2f 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles.h +++ b/src/cascadia/TerminalSettingsEditor/Profiles.h @@ -66,6 +66,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation OBSERVABLE_PROJECTED_SETTING(_profile, CursorShape); OBSERVABLE_PROJECTED_SETTING(_profile, CursorHeight); OBSERVABLE_PROJECTED_SETTING(_profile, BellStyle); + OBSERVABLE_PROJECTED_SETTING(_profile, EnvironmentVariables); private: Model::Profile _profile; diff --git a/src/cascadia/TerminalSettingsEditor/Profiles.idl b/src/cascadia/TerminalSettingsEditor/Profiles.idl index 906259f6e90..4fee4d04fb4 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles.idl +++ b/src/cascadia/TerminalSettingsEditor/Profiles.idl @@ -55,6 +55,7 @@ namespace Microsoft.Terminal.Settings.Editor OBSERVABLE_PROJECTED_SETTING(Microsoft.Terminal.TerminalControl.CursorStyle, CursorShape); OBSERVABLE_PROJECTED_SETTING(UInt32, CursorHeight); OBSERVABLE_PROJECTED_SETTING(Microsoft.Terminal.Settings.Model.BellStyle, BellStyle); + OBSERVABLE_PROJECTED_SETTING(Windows.Foundation.Collections.StringMap, EnvironmentVariables); } runtimeclass DeleteProfileEventArgs diff --git a/src/cascadia/TerminalSettingsModel/Profile.cpp b/src/cascadia/TerminalSettingsModel/Profile.cpp index 16fa759a95b..827e555018c 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.cpp +++ b/src/cascadia/TerminalSettingsModel/Profile.cpp @@ -59,6 +59,7 @@ static constexpr std::string_view AntialiasingModeKey{ "antialiasingMode" }; static constexpr std::string_view TabColorKey{ "tabColor" }; static constexpr std::string_view BellStyleKey{ "bellStyle" }; static constexpr std::string_view PixelShaderPathKey{ "experimental.pixelShaderPath" }; +static constexpr std::string_view EnvironmentKey{ "environment" }; static constexpr std::wstring_view DesktopWallpaperEnum{ L"desktopWallpaper" }; @@ -275,6 +276,75 @@ bool Profile::ShouldBeLayered(const Json::Value& json) const return sourceMatches; } +namespace +{ + std::wstring ResolveEnvironmentVariableValue(const std::wstring& key, StringMap& envMap, std::map seenKeys) + { + if (envMap.HasKey(key)) + { + if (seenKeys.find(key) != seenKeys.end()) + { + std::wstringstream error; + error << L"Self referencing keys in environment settings: "; + for (auto it : seenKeys) + { + error << L"'" << it.first << L"', "; + } + error << L"'" << key << L"'"; + throw winrt::hresult_error(WEB_E_INVALID_JSON_STRING, winrt::hstring(error.str())); + } + const std::wregex parameterRegex(L"\\$\\{env:(.*?)\\}"); + std::wstring value(envMap.Lookup(key)), resolvedValue; + auto textIter = value.cbegin(); + std::wsmatch regexMatch; + while (std::regex_search(textIter, value.cend(), regexMatch, parameterRegex)) + { + if (regexMatch.size() != 2) + { + std::wstringstream error; + error << L"Unexpected regex match count '" << regexMatch.size() + << L"' (expected '2') when matching '" << std::wstring(textIter, value.cend()) << L"'"; + throw winrt::hresult_error(WEB_E_INVALID_JSON_STRING, winrt::hstring(error.str())); + } + resolvedValue += std::wstring(textIter, regexMatch[0].first); + const auto referencedKey = regexMatch[1]; + if (referencedKey == key) + { + std::wstringstream error; + error << L"Self referencing key '" << key << L"' in environment settings"; + throw winrt::hresult_error(WEB_E_INVALID_JSON_STRING, winrt::hstring(error.str())); + } + seenKeys[key] = true; + resolvedValue += ResolveEnvironmentVariableValue(referencedKey, envMap, seenKeys); + textIter = regexMatch[0].second; + } + std::copy(textIter, value.cend(), std::back_inserter(resolvedValue)); + + return resolvedValue; + } + const auto size = GetEnvironmentVariableW(key.c_str(), nullptr, 0); + if (size > 0) + { + std::wstring value(static_cast(size), L'\0'); + if (GetEnvironmentVariableW(key.c_str(), value.data(), size) > 0) + { + // Documentation (https://docs.microsoft.com/en-us/windows/win32/api/processenv/nf-processenv-getenvironmentvariablew) + // says: "If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer, + // not including the terminating null character." + // However, my SystemDrive "C:" returns 3 and so without trimming we will end up with a stray NULL string terminator + // in the string. + value.erase(std::find_if(value.rbegin(), value.rend(), [](wchar_t ch) { + return ch != L'\0'; + }).base(), + value.end()); + + return value; + } + } + return {}; + } +} + // Method Description: // - Layer values from the given json object on top of the existing properties // of this object. For any keys we're expecting to be able to parse in the @@ -339,6 +409,26 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, TabColorKey, _TabColor); JsonUtils::GetValueForKey(json, BellStyleKey, _BellStyle); JsonUtils::GetValueForKey(json, PixelShaderPathKey, _PixelShaderPath); + + const auto environmentKey = std::string(EnvironmentKey); + if (json.isMember(environmentKey)) + { + auto environmentNode = json[environmentKey]; + StringMap rawEnvMap{}, resolvedEnvMap{}; + for (Json::Value::const_iterator it = environmentNode.begin(); it != environmentNode.end(); ++it) + { + rawEnvMap.Insert(JsonUtils::GetValue(it.key()), JsonUtils::GetValue(*it)); + } + + auto view = rawEnvMap.GetView(); + for (auto it = view.First(); it.HasCurrent(); it.MoveNext()) + { + const std::wstring key((*it).Key()); + resolvedEnvMap.Insert(key, ResolveEnvironmentVariableValue(key, rawEnvMap, {})); + } + + EnvironmentVariables(resolvedEnvMap); + } } // Method Description: diff --git a/src/cascadia/TerminalSettingsModel/Profile.h b/src/cascadia/TerminalSettingsModel/Profile.h index b7964abe516..a6008b5a93f 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.h +++ b/src/cascadia/TerminalSettingsModel/Profile.h @@ -21,6 +21,9 @@ Author(s): #include "../inc/cppwinrt_utils.h" #include "JsonUtils.h" #include +#include + +using namespace winrt::Windows::Foundation::Collections; // fwdecl unittest classes namespace SettingsModelLocalTests @@ -114,6 +117,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation GETSET_SETTING(Model::BellStyle, BellStyle, BellStyle::Audible); + GETSET_SETTING(StringMap, EnvironmentVariables); + private: static std::wstring EvaluateStartingDirectory(const std::wstring& directory); diff --git a/src/cascadia/TerminalSettingsModel/Profile.idl b/src/cascadia/TerminalSettingsModel/Profile.idl index f65b34e5c5a..019b39ed940 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.idl +++ b/src/cascadia/TerminalSettingsModel/Profile.idl @@ -190,5 +190,9 @@ namespace Microsoft.Terminal.Settings.Model Boolean HasBellStyle(); void ClearBellStyle(); BellStyle BellStyle; + + Boolean HasEnvironmentVariables(); + void ClearEnvironmentVariables(); + Windows.Foundation.Collections.StringMap EnvironmentVariables; } }