Launch Windows executables directly in stdio transport - #1839
Open
jozkee wants to merge 3 commits into
Open
Conversation
Bypass cmd.exe for rooted or existing .exe and .com commands so their arguments are passed without shell escaping, while preserving cmd.exe handling for shell-resolved commands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stage the complete test server payload so runtime-specific assets and .NET Framework binding redirects remain available when the executable is renamed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On Windows,
StdioClientTransportcurrently wraps every command other than an explicitcmd.exeinvocation with:cmd.exe /c <command> <arguments>The wrapper allows commands such as
npxto work becausecmd.exesearchesPATHandPATHEXTand resolvesnpxtonpx.cmd. However, it also unnecessarily routes executables throughcmd.exe.This wrapper introduces an additional command-line parsing layer with behavior that differs from normal Windows argv parsing.
ProcessStartInfo.ArgumentListpassescmd.exe /ca correctly quoted executable path and argument, but cmd's legacy quote handling strips the outer quotes, reparses the command line, and mistakes the path prefix ending at the first space—such asC:\Program—for the executable. Although the/sapproach explored in #1703 can avoid this, it requires constructing the entire command line manually and owning all cmd-specific quoting and escaping logic.Discarded alternative: Resolve the filename before starting the process
I implemented and iterated on this approach in https://github.com/jozkee/csharp-sdk/pull/4/changes#diff-3fc7a658a90921218be1ac3af08dcc12db8d1ba201b9cb9f186dfb0c6b7cab21.
The TypeScript SDK's stdio transport calls cross-spawn, which eventually calls the Node
whichimplementation; the Python SDK's Windows command helper callsshutil.which; and .NETProcess.Starton Unix performs its ownPATHresolution before callingexecve. All three therefore perform some form of filename resolution before executing the child. In contrast, .NET's Windows implementation performs no managed filename resolution whenUseShellExecuteis false: it builds a command line from the suppliedFileNameand arguments and passes it toCreateProcess, whose lookup can searchPATHbut does not expandPATHEXT.The reference implementations differ in their treatment of empty or quoted
PATHentries, current-directory participation, deduplication, defaultPATHEXTvalues, and commands that already have an extension. Those differences reinforced that reproducing a complete resolver in the SDK would add significant complexity and platform-specific behavior.https://github.com/npm/node-which/blob/v2.0.2/which.js
https://github.com/python/cpython/blob/v3.14.0/Lib/shutil.py
https://github.com/dotnet/runtime/blob/fea92eb70a2b115e3c8c73aba01eea615a82eee2/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs#L268-L330
This PR
This PR takes a deliberately surgical approach, suggested in #1703 (comment).
Explicit
.exeand.comcommands are launched directly when they are rooted or exist relative to the parent process's current directory, consistent with the relevant part of .NET's Unix resolution behavior.Directly launched executables receive their arguments through the normal argv path, without cmd-specific escaping.
Bare commands and script files, such as
.cmdand.batfiles, continue through the existingcmd.exe /cpath.Existing
PATH,PATHEXT, batch-file, andWorkingDirectorybehavior is therefore preserved for commands that require shell resolution.This fixes paths containing spaces without introducing a parallel command resolver or changing how common commands such as
npxare located.Fixes #1601
cc @yayayouyou