The validation of shell paths does not work properly for empty strings

[roottool]

Environment

• VSCode Version: 1.34.0
• OS Version: Windows_NT x64 10.0.17763

Description

I found this issue when I was trying to fix #72425.
_validateShellPaths function in src\vs\workbench\contrib\terminal\common\terminalService.ts does not work properly when an array include an empty string at the beginning is sent to _validateShellPaths function.

Steps to Reproduce

1. Add the below to expectedLocations in src\vs\workbench\contrib\terminal\electron-browser\terminalService.ts.

test: ['', `${system32Path}\\cmd.exe`],

2. Run npm(or yarn) run watch
3. Choose the Launch VS Code launch configuration from the launch dropdown in the Debug viewlet and press F5.
4. Press Ctrl+Shift+P.
5. Select Terminal: Select Default Shell.

Expect behavior

Choose ${system32Path}\\cmd.exe

Actual behavior

Choose an empty string

Screenshot

Note

Does this issue occur when all extensions are disabled?: Yes

[Tyriar]

I guess we can just special case '' and not do the file check to catch accidental errors?

Code pointer: https://github.com/Microsoft/vscode/blob/3a12b7ac2efd2f1f01a646ef5272313ad50bf618/src/vs/workbench/contrib/terminal/common/terminalService.ts#L429

[roottool]

I have an idea.
It is to add this code between the below into _validateShellPaths().

if (current! === '') {
    return this._validateShellPaths(label, potentialPaths);
}

https://github.com/Microsoft/vscode/blob/3a12b7ac2efd2f1f01a646ef5272313ad50bf618/src/vs/workbench/contrib/terminal/common/terminalService.ts#L428-L429

[skprabhanjan]

@roottool , you want to send a PR for this or can I handle this ?

[roottool]

@skprabhanjan, PR for this issue is created by @NizamLZ.
Sorry, I think that I entrust him.

Edited

I do not know how to best deal with this situation.

[skprabhanjan]

@roottool , @NizamLZ sent the PR after the comment and while I was waiting for your confirmation and maybe @Tyriar to suggest the best approach.