Repository: microsoft/terminal (language: cpp)

Issue:
v1.18.2681.0 breaks Powershell/WSL2/Commandline Shell (only tested on 23H2 (10.0.22631.2361)) (error 2147942487 (0x80070057))
<hr>

> [!IMPORTANT]  
> 📌 Pinned comment: **https://github.com/microsoft/terminal/issues/16051#issuecomment-1764546234**
> Are you seeing this issue? If you are, please go try the above comment to help debug. Thanks!

<hr>


### Windows Terminal version

1.18.2681.0

### Windows build number

10.0.22631.2361

### Other Software

N/A

### Steps to reproduce

Starting Terminal or Open Tabs with Powershell, WSL2 or Commandline.


### Expected Behavior

Starting the corosponding shell.

### Actual Behavior

Starting windows-terminal with powershell end in "error 8009001d loading managed Powershell" (translated from german error message).

Starting WSL tab end in error in RPC Call Error code: Wsl/Service/0x8007072c

Starting Commandline in terminal works, but has a PATH set to (NULL).

Deinstallation of Windows Terminal and downgrade to v1.17.11461.0 fixes this issues. The same happens with latest Preview version i tested after this issue arise.


Discussion / hints:
Does the issue happen if you use our portable version from our release page (the .zip files)? https://github.com/microsoft/terminal/releases/tag/v1.18.2681.0
@lhecker i uninstall windows-terminal and downloaded and unpacked https://github.com/microsoft/terminal/releases/download/v1.18.2681.0/Microsoft.WindowsTerminal_1.18.2681.0_x64.zip and started WindowsTerminal.exe

Same issue:

Powershell:
Interner Windows PowerShell-Fehler. Fehler 8009001d beim Laden der verwalteten Windows PowerShell.
[Verarbeitung des Prozesses mit Code 4294901760 (0xffff0000) beendet]

WSL:
Der RPC-Aufruf enthält ein Handle, das sich vom deklarierten Handletyp unterscheidet.
Error code: Wsl/Service/0x8007072c

Commandline:
Still missing a hugh amount of environment variables in comparison to a started cmd.exe


Ah I think I have an idea what's going on. Your environment variables are broken.

Can you launch `regedit`, navigate to `HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment` and make a screenshot of what you see?
> Ah I think I have an idea what's going on. Your environment variables are broken.
> 
> Can you launch `regedit`, navigate to `HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment` and make a screenshot of what you see?

<img width="1124" alt="image" src="https://github.com/microsoft/terminal/assets/8289580/73c8e9ea-0f76-4219-ac50-329833139cff">

Path is incomplete in the screenshot, it is: C:\Program Files\Amazon Corretto\jdk17.0.8_8\bin;C:\Program Files (x86)\Microsoft SDKs\Azure\CLI2\wbin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\WINDOWS\System32\OpenSSH\;C:\Program Files\WindowsPowerShell\Scripts;C:\Program Files\VSCodium\bin;C:\Program Files\dotnet\;C:\Program Files\Tailscale\

And what about the contents of `HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion`? Does it look like this?
![image](https://github.com/microsoft/terminal/assets/2256941/000440db-a9dc-474f-aff5-a87af3cd664e)

When you open the permissions dialog here:
![image](https://github.com/microsoft/terminal/assets/2256941/51edb1fb-09e1-4b07-8273-7b4e02c462b2)

and you select "Users" does it look like this?
![image](https://github.com/microsoft/terminal/assets/2256941/4d42d043-b539-4124-b75c-72765de53d34)
<img width="383" alt="image" src="https://github.com/microsoft/terminal/assets/8289580/2f92b1f5-82d7-440f-a33b-b150674257e3">

![image](https://github.com/microsoft/terminal/assets/8289580/978590c4-e9eb-400c-92c5-dbe4053f27d7)


So far it looks similar to your settings.

I have the same issues with the recent release. PATH is Null for CMD and PSCore. PowerShell 5.1 won't start with the aformentioned error. Terminal's completely unusable.

Windows 10 22H2, set to german.
Does one of you feel comfortable to run a piece of C++ code under a debugger on your system?
> Does one of you feel comfortable to run a piece of C++ code under a debugger on your system?

MDE is a bit picky with unsigned code, but i can try.
My private machine show the same issue after the update, another machine with 23h2.
I am facing similar issue with preview version 1.19.2682.0, windows 25951.1010. Any idea on what's going on?
> Analyzing a bit, it seems to be that OpenConsole.exe never gets around to trying to start the exe (in procmon, there is no "pwsh" in the entire capture from OpenConsole.exe on a machine where this is broken).
> 
> Note that e.g. `./OpenConsole.exe pwsh.exe` or similar works ok.

From #16087
Here's a theory - y'all might be able to get unblocked by setting:

```json
"profiles":
{
    "defaults": 
    {
        "compatibility.reloadEnvironmentVariables": false
    },
    "list":
    [
        // ...
    ]
}
```

in your settings.json. Now, why that regressed, and seemingly only in `10.0.22631.2361` sure is curious.

If you're seeing this, is your `Path` under `HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment` a `REG_SZ` or a `REG_EXPAND_SZ`?
> If you're seeing this, is your `Path` under `HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment` a `REG_SZ` or a `REG_EXPAND_SZ`?

@zadjii-msft Path is a REG_SZ

**Setting "compatibility.reloadEnvironmentVariables": false** in settings.json has no effect.

Edit: i have to correct myself, Commandshell and WSL works, Powershell still fails with 8009001d.
@zadjii-msft, the addition of `"compatibility.reloadEnvironmentVariables": false` has the effect that now every _new_ tab you open works fine, just not the initial one (which fails like before). So PowerShell also works, if you just press + after the initial one has failed. All of them do.

As for the registry, it's a REG_EXPAND_SZ key for the affected machine.
> @zadjii-msft, the addition of `"compatibility.reloadEnvironmentVariables": false` has the effect that now every _new_ tab you open works fine, just not the initial one (which fails like before). So PowerShell also works, if you just press + after the initial one has failed. All of them do.

you are right, opening another powershell tab works.
Sorry for the delay, here's the C++ code: https://gist.github.com/lhecker/f0bd197c8aa8ecf1aeb6505115bb7646
It would be very helpful if someone could try to compile and run it under a debugger. It should pause at some point during the execution. A stack trace and maybe a screenshot of the local variables would be nice. 🙂

But yesterday we realized we can also just use a WPR trace ([see steps here](https://github.com/microsoft/terminal/wiki/Toubleshooting-Tips#capturing-a-debug-etl-trace)) to get a hold of the WIL error logs, so if the above doesn't work out (because it's rather cumbersome), there's still another way to go about this.
Also, squinting at the code:

https://github.com/microsoft/terminal/blob/6489f6b39daaef634dd0e56e50afa8866f0103c7/src/inc/til/env.h#L226-L232

That calls the string version of `wil::TryGetEnvironmentVariableW`

https://github.com/microsoft/wil/blob/be13859134711a1b1206da484f2f6b8b41feb883/include/wil/win32_helpers.h#L604-L609
```c++
    string_type TryGetEnvironmentVariableW(_In_ PCWSTR key)
    {
        string_type result{};
        THROW_IF_FAILED((wil::TryGetEnvironmentVariableW<string_type, initialBufferLength>(key, result)));
        return result;
    }
```

So that can throw if we don't find the env var. Then in `regenerate`, we do:

https://github.com/microsoft/terminal/blob/6489f6b39daaef634dd0e56e50afa8866f0103c7/src/inc/til/env.h#L537-L560

Huh, looks like we `get` a bunch of variables. I can't help but think one of those blows up in this scenario. The [aformentioned](https://github.com/microsoft/terminal/issues/16051#issuecomment-1744790570) script under a debugger might tell us which. But maybe all those need to be `try_get` or something. 
@lhecker, unfortunately I can't compile the cpp code. I never do c++ these days (and never did on Windows), and my installed compiler is from VS 2017, which isn't new enough evidently. I need some assistance, at least, in how to do it.

I did get an ETL trace; not sure if it's useful. I enabled tracing (from admin cmd), started WindowsTerminal.exe (from explorer, non-admin), saw the error (PowerShell did start, but with broken environment), stopped trace and then stopped terminal.

ETL file (zipped, since otherwise it doesn't work): [terminal-trace.zip](https://github.com/microsoft/terminal/files/12854817/terminal-trace.zip)

Meanwhile this problem hit another few co-workers. This time it behaved a bit differently, where a large number, but not all, environment variables were lost, including USERPROFILE. This meant that ~ (for home directory) didn't work, which failed a startup script we have. The workaround helped as before. The same behavior was seen by me when doing the trace above.
Unfortunately, the trace doesn't contain any relevant errors.

Fortunately however, the absence of any errors is a pretty good indication for what's going on here. I believe the issue is that you (the affected users) have null bytes in your registry values. In particular, this code is wrong:
https://github.com/microsoft/terminal/blob/f2c3ddd10523c146260a259a1c2ee2a85b264be8/src/inc/til/env.h#L340-L343

It makes 2 mistakes:
* `strip_trailing_null` only strips 1 trailing null byte, but there can be arbitrarily many
* Embedded null bytes are not removed

Because of this, the environment string (double null byte terminated) can be malformed (for instance if a section between two null bytes isn't of the form `key=value`), or it can end up containing two or more null bytes in the middle due to which `CreateProcess` thinks the environment string only contains N items, even though it contains way more than that. This observation somewhat supports my hypothesis:
> This time it behaved a bit differently, where a large number, but not all, environment variables were lost, including USERPROFILE. 
Based on this idea I've written this PowerShell script: https://gist.github.com/lhecker/393252fb1e95b3bc118d40ace802a505
If you run it, it should list all environment strings that contain embedded null bytes/characters.
I just run it and this is the result:

# .\Get-EnvsWithEmbeddedNull.ps1
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment: Path
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment: PSModulePath
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment: PROCESSOR_REVISION
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment: JAVA_HOME

I dont know what is wrong with this. I removeed PROCESSOR_REVISION and recreated it with the same value, it still show up in your script.


Nice! That means my assumption is most likely correct.

> I removeed PROCESSOR_REVISION and recreated it with the same value, it still show up in your script.

How did you edit the value? If you haven't already, try using `regedit`. In case of something like `PROCESSOR_REVISION` try typing the `REG_SZ` value by hand (since it's just 4 characters or something) to ensure that it truly doesn't contain any null characters.
> Nice! That means my assumption is most likely correct.
> 
> > I removeed PROCESSOR_REVISION and recreated it with the same value, it still show up in your script.
> 
> How did you edit the value? If you haven't already, try using `regedit`. In case of something like `PROCESSOR_REVISION` try typing the `REG_SZ` value by hand (since it's just 4 characters or something) to ensure that it truly doesn't contain any null characters.

PATH
C:\Program Files\Amazon Corretto\jdk17.0.8_8\bin;C:\Program Files (x86)\Microsoft SDKs\Azure\CLI2\wbin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\WINDOWS\System32\OpenSSH\;C:\Program Files\WindowsPowerShell\Scripts;C:\Program Files\VSCodium\bin;C:\Program Files\dotnet\;C:\Program Files\Git\cmd;C:\Program Files\Tailscale\;C:\Program Files\PuTTY\

PSModulePath
C:\Program Files\WindowsPowerShell\Modules;C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules;C:\Program Files\MicrosoftManagedDesktop\Powershell

PROCESSOR_REVISION
4401

Your script triggers on every zero in finds, if i replace the revision from 4401 to 4411, your script no longer show the reg.

Oh I see... That's because I tested my script on PowerShell 7 (`pwsh.exe`) and you probably use PowerShell 5 (`powershell.exe`). The fix is to use ``Contains("`0")`` instead of `Contains(0)`. I've updated the script above with the fix. Can you test it again?
(I've marked our comments as resolved to make this issue more compact.)
@lhecker bingo! That's the problem. The "lovely" tool Snow Inventory Manager, that my company uses, has a NUL:

    C:\src # ~\check.ps1
    HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\Environment: SNOW_INVENTORY_HOME

With the value:

    C:\Program Files\INVENTORYCLIENT믃蔟℀退幠˕幠˕ etc. etc. 

Obviously only the part before the Hanzi makes it into the environment variable itself.
Thanks for confirming this! I've been rewriting the environment-reload code the last few days (twice as fast, half the code 🎉) and I've got a patch ready to hotfix it in the already released 1.18 branch.

Fix patch:
diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp
index e086c9b2cfc..5b20f0d6ef1 100644
--- a/src/cascadia/TerminalConnection/ConptyConnection.cpp
+++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp
@@ -82,14 +82,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
                                                              nullptr));
 
         auto cmdline{ wil::ExpandEnvironmentStringsW<std::wstring>(_commandline.c_str()) }; // mutable copy -- required for CreateProcessW
-
-        til::env environment;
-        auto zeroEnvMap = wil::scope_exit([&]() noexcept {
-            environment.clear();
-        });
-
-        // Populate the environment map with the current environment.
-        environment = _initialEnv;
+        auto environment = _initialEnv;
 
         {
             // Convert connection Guid to string and ignore the enclosing '{}'.
@@ -140,15 +133,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
             environment.as_map().insert_or_assign(L"WSLENV", wslEnv);
         }
 
-        std::vector<wchar_t> newEnvVars;
-        auto zeroNewEnv = wil::scope_exit([&]() noexcept {
-            ::SecureZeroMemory(newEnvVars.data(),
-                               newEnvVars.size() * sizeof(decltype(newEnvVars.begin())::value_type));
-        });
-
-        RETURN_IF_FAILED(environment.to_environment_strings_w(newEnvVars));
-
-        auto lpEnvironment = newEnvVars.empty() ? nullptr : newEnvVars.data();
+        auto newEnvVars = environment.to_string();
+        const auto lpEnvironment = newEnvVars.empty() ? nullptr : newEnvVars.data();
 
         // If we have a startingTitle, create a mutable character buffer to add
         // it to the STARTUPINFO.
diff --git a/src/inc/til/env.h b/src/inc/til/env.h
index 4d95f9f2f98..1546a83afb6 100644
--- a/src/inc/til/env.h
+++ b/src/inc/til/env.h
@@ -76,65 +76,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
 
         namespace wil_env
         {
-            /** Looks up the computer name and fails if it is not found. */
-            template<typename string_type, size_t initialBufferLength = MAX_COMPUTERNAME_LENGTH + 1>
-            HRESULT GetComputerNameW(string_type& result) WI_NOEXCEPT
-            {
-                return wil::AdaptFixedSizeToAllocatedResult<string_type, initialBufferLength>(result, [&](_Out_writes_(valueLength) PWSTR value, size_t valueLength, _Out_ size_t* valueLengthNeededWithNul) -> HRESULT {
-                    // 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.
-                    //
-                    // If lpBuffer is not large enough to hold the data, the return value is the buffer size, in
-                    // characters, required to hold the string and its terminating null character and the contents of
-                    // lpBuffer are undefined.
-                    //
-                    // If the function fails, the return value is zero. If the specified environment variable was not
-                    // found in the environment block, GetLastError returns ERROR_ENVVAR_NOT_FOUND.
-
-                    ::SetLastError(ERROR_SUCCESS);
-
-                    DWORD length = static_cast<DWORD>(valueLength);
-
-                    const auto result = ::GetComputerNameW(value, &length);
-                    *valueLengthNeededWithNul = length;
-                    RETURN_IF_WIN32_BOOL_FALSE_EXPECTED(result);
-                    if (*valueLengthNeededWithNul < valueLength)
-                    {
-                        (*valueLengthNeededWithNul)++; // It fit, account for the null.
-                    }
-                    return S_OK;
-                });
-            }
-
-            /** Looks up the computer name and returns null if it is not found. */
-            template<typename string_type, size_t initialBufferLength = MAX_COMPUTERNAME_LENGTH + 1>
-            HRESULT TryGetComputerNameW(string_type& result) WI_NOEXCEPT
-            {
-                const auto hr = wil_env::GetComputerNameW<string_type, initialBufferLength>(result);
-                RETURN_HR_IF(hr, FAILED(hr) && (hr != HRESULT_FROM_WIN32(ERROR_ENVVAR_NOT_FOUND)));
-                return S_OK;
-            }
-
-#ifdef WIL_ENABLE_EXCEPTIONS
-            /** Looks up the computer name and fails if it is not found. */
-            template<typename string_type = wil::unique_cotaskmem_string, size_t initialBufferLength = MAX_COMPUTERNAME_LENGTH + 1>
-            string_type GetComputerNameW()
-            {
-                string_type result;
-                THROW_IF_FAILED((wil_env::GetComputerNameW<string_type, initialBufferLength>(result)));
-                return result;
-            }
-
-            /** Looks up the computer name and returns null if it is not found. */
-            template<typename string_type = wil::unique_cotaskmem_string, size_t initialBufferLength = MAX_COMPUTERNAME_LENGTH + 1>
-            string_type TryGetComputerNameW()
-            {
-                string_type result;
-                THROW_IF_FAILED((wil_env::TryGetComputerNameW<string_type, initialBufferLength>(result)));
-                return result;
-            }
-#endif
-
             /** Looks up a registry value from 'key' and fails if it is not found. */
             template<typename string_type, size_t initialBufferLength = 256>
             HRESULT RegQueryValueExW(HKEY key, PCWSTR valueName, string_type& result) WI_NOEXCEPT
@@ -231,41 +172,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
             }
         }
 
-        void get_computer_name()
-        {
-            if (auto value = til::details::wil_env::TryGetComputerNameW<std::wstring>(); !value.empty())
-            {
-                save_to_map(std::wstring{ til::details::vars::computer_name }, std::move(value));
-            }
-        }
-
-        void get_user_name_and_domain()
-        try
-        {
-            const auto token = wil::open_current_access_token();
-            const auto user = wil::get_token_information<TOKEN_USER>(token.get());
-
-            DWORD accountNameSize = 0, userDomainSize = 0;
-            SID_NAME_USE sidNameUse;
-            SetLastError(ERROR_SUCCESS);
-            if (LookupAccountSidW(nullptr, user.get()->User.Sid, nullptr, &accountNameSize, nullptr, &userDomainSize, &sidNameUse) || GetLastError() == ERROR_INSUFFICIENT_BUFFER)
-            {
-                std::wstring accountName, userDomain;
-                accountName.resize(accountNameSize);
-                userDomain.resize(userDomainSize);
-
-                SetLastError(ERROR_SUCCESS);
-                if (LookupAccountSidW(nullptr, user.get()->User.Sid, accountName.data(), &accountNameSize, userDomain.data(), &userDomainSize, &sidNameUse))
-                {
-                    strip_trailing_null(accountName);
-                    strip_trailing_null(userDomain);
-                    save_to_map(std::wstring{ til::details::vars::user_name }, std::move(accountName));
-                    save_to_map(std::wstring{ til::details::vars::user_domain }, std::move(userDomain));
-                }
-            }
-        }
-        CATCH_LOG()
-
         void get_program_files()
         {
             wil::unique_hkey key;
@@ -325,23 +231,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
                                 std::wstring data;
                                 if (pass == 0 && (type == REG_SZ) && valueDataSize >= sizeof(wchar_t))
                                 {
-                                    data = {
-                                        reinterpret_cast<wchar_t*>(valueData.data()), valueData.size() / sizeof(wchar_t)
-                                    };
+                                    const auto p = reinterpret_cast<const wchar_t*>(valueData.data());
+                                    const auto l = wcsnlen(p, valueData.size() / sizeof(wchar_t));
+                                    data.assign(p, l);
                                 }
                                 else if (pass == 1 && (type == REG_EXPAND_SZ) && valueDataSize >= sizeof(wchar_t))
                                 {
-                                    data = {
-                                        reinterpret_cast<wchar_t*>(valueData.data()), valueData.size() / sizeof(wchar_t)
-                                    };
+                                    const auto p = reinterpret_cast<const wchar_t*>(valueData.data());
+                                    const auto l = wcsnlen(p, valueData.size() / sizeof(wchar_t));
+                                    data.assign(p, l);
                                     data = expand_environment_strings(data.data());
                                 }
 
-                                // Because Registry data may or may not be null terminated... check if we've managed
-                                // to store an extra null in the wstring by telling it to create itself from pointer and size.
-                                // If we did, pull it off.
-                                strip_trailing_null(data);
-
                                 if (!data.empty())
                                 {
                                     if (isPathVar)
@@ -468,14 +369,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
                    til::compare_string_ordinal(input, os2LibPath) == CSTR_EQUAL;
         }
 
-        static void strip_trailing_null(std::wstring& str) noexcept
-        {
-            if (!str.empty() && str.back() == L'\0')
-            {
-                str.pop_back();
-            }
-        }
-
         void parse(const wchar_t* lastCh)
         {
             for (; *lastCh != '\0'; ++lastCh)
@@ -537,13 +430,20 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
         void regenerate()
         {
             // Generally replicates the behavior of shell32!RegenerateUserEnvironment
+            // The difference is that we don't
+            // * handle autoexec.bat (intentionally unsupported).
+            // * call LookupAccountSidW to get the USERNAME/USERDOMAIN variables.
+            //   Windows sets these as values of the "Volatile Environment" key at each login.
+            //   We don't expect our process to impersonate another user so we can get them from the PEB.
+            // * call GetComputerNameW to get the COMPUTERNAME variable, for the same reason.
             get(til::details::vars::system_root);
             get(til::details::vars::system_drive);
             get(til::details::vars::all_users_profile);
             get(til::details::vars::public_var);
             get(til::details::vars::program_data);
-            get_computer_name();
-            get_user_name_and_domain();
+            get(til::details::vars::computer_name);
+            get(til::details::vars::user_name);
+            get(til::details::vars::user_domain);
             get(til::details::vars::user_dns_domain);
             get(til::details::vars::home_drive);
             get(til::details::vars::home_share);
@@ -562,104 +462,17 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
         std::wstring to_string() const
         {
             std::wstring result;
-            for (const auto& [name, value] : _envMap)
+            for (const auto& [k, v] : _envMap)
             {
-                result += name;
-                result += L"=";
-                result += value;
-                result.append(L"\0", 1); // Override string's natural propensity to stop at \0
+                result.append(k);
+                result.push_back(L'=');
+                result.append(v);
+                result.push_back(L'\0');
             }
-            result.append(L"\0", 1);
+            result.push_back(L'\0');
             return result;
         }
 
-        void clear() noexcept
-        {
-            // Can't zero the keys, but at least we can zero the values.
-            for (auto& [name, value] : _envMap)
-            {
-                ::SecureZeroMemory(value.data(), value.size() * sizeof(decltype(value.begin())::value_type));
-            }
-
-            _envMap.clear();
-        }
-
-        // Function Description:
-        // - Creates a new environment block using the provided vector as appropriate
-        //   (resizing if needed) based on the current environment variable map
-        //   matching the format of GetEnvironmentStringsW.
-        // Arguments:
-        // - newEnvVars: The vector that will be used to create the new environment block.
-        // Return Value:
-        // - S_OK if we succeeded, or an appropriate HRESULT for failing
-        HRESULT to_environment_strings_w(std::vector<wchar_t>& newEnvVars)
-        try
-        {
-            // Clear environment block before use.
-            constexpr auto cbChar{ sizeof(decltype(newEnvVars.begin())::value_type) };
-
-            if (!newEnvVars.empty())
-            {
-                ::SecureZeroMemory(newEnvVars.data(), newEnvVars.size() * cbChar);
-            }
-
-            // Resize environment block to fit map.
-            size_t cchEnv{ 2 }; // For the block's double NULL-terminator.
-            for (const auto& [name, value] : _envMap)
-            {
-                // Final form of "name=value\0".
-                cchEnv += name.size() + 1 + value.size() + 1;
-            }
-            newEnvVars.resize(cchEnv);
-
-            // Ensure new block is wiped if we exit due to failure.
-            auto zeroNewEnv = wil::scope_exit([&]() noexcept {
-                ::SecureZeroMemory(newEnvVars.data(), newEnvVars.size() * cbChar);
-            });
-
-            // Transform each map entry and copy it into the new environment block.
-            auto pEnvVars{ newEnvVars.data() };
-            auto cbRemaining{ cchEnv * cbChar };
-            for (const auto& [name, value] : _envMap)
-            {
-                // Final form of "name=value\0" for every entry.
-                {
-                    const auto cchSrc{ name.size() };
-                    const auto cbSrc{ cchSrc * cbChar };
-                    RETURN_HR_IF(E_OUTOFMEMORY, memcpy_s(pEnvVars, cbRemaining, name.c_str(), cbSrc) != 0);
-                    pEnvVars += cchSrc;
-                    cbRemaining -= cbSrc;
-                }
-
-                RETURN_HR_IF(E_OUTOFMEMORY, memcpy_s(pEnvVars, cbRemaining, L"=", cbChar) != 0);
-                ++pEnvVars;
-                cbRemaining -= cbChar;
-
-                {
-                    const auto cchSrc{ value.size() };
-                    const auto cbSrc{ cchSrc * cbChar };
-                    RETURN_HR_IF(E_OUTOFMEMORY, memcpy_s(pEnvVars, cbRemaining, value.c_str(), cbSrc) != 0);
-                    pEnvVars += cchSrc;
-                    cbRemaining -= cbSrc;
-                }
-
-                RETURN_HR_IF(E_OUTOFMEMORY, memcpy_s(pEnvVars, cbRemaining, L"\0", cbChar) != 0);
-                ++pEnvVars;
-                cbRemaining -= cbChar;
-            }
-
-            // Environment block only has to be NULL-terminated, but double NULL-terminate anyway.
-            RETURN_HR_IF(E_OUTOFMEMORY, memcpy_s(pEnvVars, cbRemaining, L"\0\0", cbChar * 2) != 0);
-            cbRemaining -= cbChar * 2;
-
-            RETURN_HR_IF(E_UNEXPECTED, cbRemaining != 0);
-
-            zeroNewEnv.release(); // success; don't wipe new environment block on exit
-
-            return S_OK;
-        }
-        CATCH_RETURN();
-
         auto& as_map() noexcept
         {
             return _envMap;
diff --git a/src/til/ut_til/EnvTests.cpp b/src/til/ut_til/EnvTests.cpp
index 1a9fd53c603..f0d2e97cd95 100644
--- a/src/til/ut_til/EnvTests.cpp
+++ b/src/til/ut_til/EnvTests.cpp
@@ -9,7 +9,7 @@ using namespace WEX::Common;
 using namespace WEX::Logging;
 using namespace WEX::TestExecution;
 
-BOOL APIENTRY RegenerateUserEnvironment(void** pNewEnv, BOOL bSetCurrentEnv);
+BOOL APIENTRY RegenerateUserEnvironment(wchar_t** pNewEnv, BOOL bSetCurrentEnv);
 
 class EnvTests
 {
@@ -26,9 +26,11 @@ class EnvTests
         wil::unique_hmodule shell32{ LoadLibraryW(L"shell32.dll") };
         auto func = GetProcAddressByFunctionDeclaration(shell32.get(), RegenerateUserEnvironment);
 
+        wchar_t* newEnvPtr = nullptr;
+        VERIFY_WIN32_BOOL_SUCCEEDED(func(&newEnvPtr, FALSE));
+
         // Use a WIL pointer to free it on scope exit.
-        wil::unique_environstrings_ptr newEnv;
-        VERIFY_WIN32_BOOL_SUCCEEDED(func((void**)&newEnv, FALSE));
+        const wil::unique_environstrings_ptr newEnv{ newEnvPtr };
 
         // Parse the string into our environment table.
         til::env expected{ newEnv.get() };
@@ -38,12 +40,15 @@ class EnvTests
         actual.regenerate();
 
         VERIFY_ARE_EQUAL(expected.as_map().size(), actual.as_map().size());
-        for (const auto& [expectedKey, expectedValue] : expected.as_map())
+
+        const auto expectedEnd = expected.as_map().end();
+        auto expectedIt = expected.as_map().begin();
+        auto actualIt = actual.as_map().begin();
+
+        for (; expectedIt != expectedEnd; ++expectedIt, ++actualIt)
         {
-            Log::Comment(String().Format(L"Environment Variable: '%s'", expectedKey.data()));
-            const auto actualValue = actual.as_map()[expectedKey];
-            VERIFY_IS_TRUE(actual.as_map().find(expectedKey) != actual.as_map().end());
-            VERIFY_ARE_EQUAL(expectedValue, actual.as_map()[expectedKey]);
+            VERIFY_ARE_EQUAL(expectedIt->first, actualIt->first);
+            VERIFY_ARE_EQUAL(expectedIt->second, actualIt->second);
         }
     }
 
@@ -54,16 +59,10 @@ class EnvTests
         environment.as_map().insert_or_assign(L"B", L"Banana");
         environment.as_map().insert_or_assign(L"C", L"Cassowary");
 
-        wchar_t expectedArray[] = L"A=Apple\0B=Banana\0C=Cassowary\0";
-
-        const std::wstring expected{ expectedArray, ARRAYSIZE(expectedArray) };
+        static constexpr wchar_t expectedArray[] = L"A=Apple\0B=Banana\0C=Cassowary\0";
+        static constexpr std::wstring_view expected{ expectedArray, std::size(expectedArray) };
         const auto& actual = environment.to_string();
-
-        VERIFY_ARE_EQUAL(expected.size(), actual.size());
-        for (size_t i = 0; i < expected.size(); ++i)
-        {
-            VERIFY_ARE_EQUAL(expected[i], actual[i]);
-        }
+        VERIFY_ARE_EQUAL(expected, actual);
     }
 
     TEST_METHOD(TestExpandEnvironmentStrings)


Test patch: