Skip to content

Fix escaping for dotnet paths with spaces when spawning out-of-proc nodes#12644

Merged
rainersigwald merged 2 commits intovs18.0from
dev/ykovalova/fix_escaping
Oct 20, 2025
Merged

Fix escaping for dotnet paths with spaces when spawning out-of-proc nodes#12644
rainersigwald merged 2 commits intovs18.0from
dev/ykovalova/fix_escaping

Conversation

@YuliiaKovalova
Copy link
Member

@YuliiaKovalova YuliiaKovalova commented Oct 15, 2025

Context

When a path to dotnet.exe or MSBuild.dll contains spaces, it's incorrectly escaped and causes errors like this:
Could not execute because the specified command or file was not found.

  • You intended to execute a .NET program, but dotnet-C:\Program does not exist.
    The path gets truncated at the space character, causing the process launch to fail.

The important notice – it’s a critical scenario for the feature dogfooding , since dotnet.exe usually is placed in “Program Files” (path with spaces).
It wasn’t caught during the development because msbuild has a bootstrapped copy of the sdk and this path was ok ☹

Changes Made

Added proper escaping for the msbuild.dll path to ensure it works correctly even when the path contains spaces.

Regression

No, it’s a bug in the new feature

Risks

No risks, minimal well-boxed change

Testing

UT + manual

Notes

This logic is taken from #12630

Copilot AI review requested due to automatic review settings October 15, 2025 10:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes escaping for paths containing spaces when launching dotnet/MSBuild processes. The primary issue was that paths with spaces were not properly quoted, causing process launch failures where the path would be truncated at the first space character.

  • Adds proper quoting for MSBuild.dll path in command line arguments
  • Updates test infrastructure to support validation of path escaping
  • Adds comprehensive test to verify correct path handling with spaces

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs Added quotes around MSBuild.dll path in command line template
src/UnitTests.Shared/TestEnvironment.cs Refactored test environment setup and added method to retrieve environment variables
src/Build.UnitTests/NetTaskHost_E2E_Tests.cs Updated tests to use new test environment setup and added comprehensive path escaping validation
src/Build.UnitTests/TestAssets/ExampleNetTask/ExampleTask/ExampleTask.cs Added command line argument logging for test validation

@baronfel baronfel changed the title Fix escaping for dotnet paths with spaces Fix escaping for dotnet paths with spaces when spawning out-of-proc nodes Oct 15, 2025
@YuliiaKovalova YuliiaKovalova changed the base branch from main to vs18.0 October 15, 2025 13:55
@rainersigwald rainersigwald merged commit 6b217fc into vs18.0 Oct 20, 2025
10 checks passed
@rainersigwald rainersigwald deleted the dev/ykovalova/fix_escaping branch October 20, 2025 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants