GeezerVison

please add some validation for this new behavior

James Truher (@JamesWTruher) - the bug was caused by unexpected return data from this API call in ModuleIntrinsics.cs

string myDocumentsPath = Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments);

To add tests for this in the build we'd either have to have the ability to run under an account where it returns "" or provide some way to simulate the API returning the empty string (e.g. through a layer of indirection controlled by an environment variable). Is this something you'd like to see, or if you have an alternative approach please let me know.

The easiest way I've found to test the impact of this change manually is to use psexec from SysInternals to launch a System account (elevated) command prompt (which returns "" for the API call above). See issue #19875 comments for some screenshots from that testing.

psexec -i -s cmd.exe

Then in the system account elevated command prompt start the pwsh executable and observe:

• pwsh-7.4-preview 4 crashes with null pointer exception when starting
• with this PR it starts successfully

I have also tested it manually in the scenario where a domain joined PC with network located MyDocuments is disconnected from the domain network and confirmed it no longer crashes in that scenario either.

I'm not familiar enough with the build environment to know how such a test could be orchestrated. The only alternative would be to use some interfaces and create some C# unit tests but I haven't seen any unit test projects in the source base so don't know if this would be an acceptable approach here.

stevenebutler Easiest way is to use a test hook. See

stevenebutler Easiest way is to use a test hook. See

PowerShell/src/System.Management.Automation/engine/Utils.cs

Line 1532 in 5595072

public static class InternalTestHooks

PowerShell/test/powershell/Host/HostUtilities.Tests.ps1

Line 74 in 5595072

[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('NoPromptForPassword', $true)

The issue is the test hook would have to be configured before powershell starts up to trigger the bug because this crash happens long before you get an interactive shell or can run a script. It happens while PowerShell itself is initializing.

Maybe we can start up PowerShell then tweak the test hook and call into the method that crashes in the module intrinsics code and possibly trigger the same exception afterPowerShell has started. I'll look into that possibility.

The issue is the test hook would have to be configured before powershell starts up to trigger the bug because this crash happens long before you get an interactive shell or can run a script. It happens while PowerShell itself is initializing.

We could use specific/hidden env variable like __SuppressAnsiEscapeSequences or __PWSH_LOGIN_CHECKED for turning on the test hook.

This PR has 30 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Label      : Extra Small
Size       : +21 -9
Percentile : 12%

Total files changed: 3

Change summary by file extension:
.cs : +12 -9
.ps1 : +9 -0

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

It's not necessary to use env vars as the code that was failing can be called directly after startup with the test hook set to fake an empty MyDocuments location.

I've added this test, which failed without the patch.

      try {
            [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('SetMyDocumentsSpecialFolderToBlank', $true)
            [System.Management.Automation.ModuleIntrinsics]::GetModulePath($null, $null, $null)
        }
        finally {
            [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('SetMyDocumentsSpecialFolderToBlank', $false)
        }

James Truher (@JamesWTruher) Can you please take another look and update your review? Thanks!