Fix WSLENV environment variable duplication in ConptyConnection (#19167)

This PR fixes issue #7130 where WT_SESSION and WT_PROFILE_ID environment
variables were being duplicated in the WSLENV environment variable when
multiple terminal sessions were created.

The previous implementation always appended WT_SESSION:WT_PROFILE_ID: to
WSLENV without checking if these variables already existed, causing
duplication.

Closes #7130

Co-authored-by: Leonard Hecker <[email protected]>
(cherry picked from commit 7d6e0c8)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgdDF2w
Service-Version: 1.23

Author: 2 people

src/cascadia/TerminalConnection/ConptyConnection.cpp

@@ -66,10 +66,41 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 
             // WSLENV is a colon-delimited list of environment variables (+flags) that should appear inside WSL
             // https://devblogs.microsoft.com/commandline/share-environment-vars-between-wsl-and-windows/
-            std::wstring wslEnv{ L"WT_SESSION:WT_PROFILE_ID:" };
+
+            // WSLENV.1: Get a handle to the WSLENV environment variable.
+            auto& wslEnv = environment.as_map()[L"WSLENV"];
+            std::wstring additionalWslEnv;
+
+            // WSLENV.2: Figure out what variables are already in WSLENV.
+            std::unordered_set<std::wstring_view> wslEnvVars;
+            for (const auto& part : til::split_iterator{ std::wstring_view{ wslEnv }, L':' })
+            {
+                // Each part may contain a variable name and flags (e.g., /p, /l, etc.)
+                // We only care about the variable name for WSLENV.
+                const auto key = til::safe_slice_len(part, 0, part.rfind(L'/'));
+                wslEnvVars.emplace(key);
+            }
+
+            // WSLENV.3: Add our terminal-specific environment variables to WSLENV.
+            static constexpr std::wstring_view builtinWslEnvVars[] = {
+                L"WT_SESSION",
+                L"WT_PROFILE_ID",
+            };
+            // Misdiagnosis in MSVC 14.44.35207. No pointer arithmetic in sight.
+#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1).
+            for (const auto& key : builtinWslEnvVars)
+            {
+                if (wslEnvVars.emplace(key).second)
+                {
+                    additionalWslEnv.append(key);
+                    additionalWslEnv.push_back(L':');
+                }
+            }
+
             if (_environment)
             {
                 // Order the environment variable names so that resolution order is consistent
+                // NOTE(lhecker): I'm like 99% sure that this is unnecessary.
                 std::set<std::wstring, til::env_key_sorter> keys{};
                 for (const auto item : _environment)
                 {
@@ -85,18 +116,39 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
                         const auto value = winrt::unbox_value<hstring>(_environment.Lookup(key));
 
                         environment.set_user_environment_var(key.c_str(), value.c_str());
-                        // For each environment variable added to the environment, also add it to WSLENV
-                        wslEnv += key + L":";
+
+                        // WSLENV.4: Add custom user environment variables to WSLENV.
+                        if (wslEnvVars.emplace(key).second)
+                        {
+                            additionalWslEnv.append(key);
+                            additionalWslEnv.push_back(L':');
+                        }
                     }
                     CATCH_LOG();
                 }
             }
 
-            // We want to prepend new environment variables to WSLENV - that way if a variable already
-            // exists in WSLENV but with a flag, the flag will be respected.
-            // (This behaviour was empirically observed)
-            wslEnv += environment.as_map()[L"WSLENV"];
-            environment.as_map().insert_or_assign(L"WSLENV", wslEnv);
+            if (!additionalWslEnv.empty())
+            {
+                // WSLENV.5: In the next step we'll prepend `additionalWslEnv` to `wslEnv`,
+                // so make sure that we have a single colon in between them.
+                const auto hasColon = additionalWslEnv.ends_with(L':');
+                const auto needsColon = !wslEnv.starts_with(L':');
+                if (hasColon != needsColon)
+                {
+                    if (hasColon)
+                    {
+                        additionalWslEnv.pop_back();
+                    }
+                    else
+                    {
+                        additionalWslEnv.push_back(L':');
+                    }
+                }
+
+                // WSLENV.6: Prepend our additional environment variables to WSLENV.
+                wslEnv.insert(0, additionalWslEnv);
+            }
         }
 
         auto newEnvVars = environment.to_string();

src/inc/til.h

@@ -55,20 +55,22 @@
 namespace til // Terminal Implementation Library. Also: "Today I Learned"
 {
     template<typename T>
-    as_view_t<T> safe_slice_abs(const T& view, size_t beg, size_t end)
+    as_view_t<T> safe_slice_abs(const T& view, size_t beg, size_t end) noexcept
     {
         const auto len = view.size();
         end = std::min(end, len);
         beg = std::min(beg, end);
+#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1).
         return { view.data() + beg, end - beg };
     }
 
     template<typename T>
-    as_view_t<T> safe_slice_len(const T& view, size_t start, size_t count)
+    as_view_t<T> safe_slice_len(const T& view, size_t start, size_t count) noexcept
     {
         const auto len = view.size();
         start = std::min(start, len);
         count = std::min(count, len - start);
+#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1).
         return { view.data() + start, count };
     }