Skip to content

[fix] Quote --diag path and harden AddDoubleQuote against embedded quotes/trailing backslashes #16456

Description

@github-actions

🤖 This is an automated fix generated by the Issue Triage workflow.

Fixes #16449

Root cause

  1. TestRunnerConnectionInfoExtensions.ToCommandLineOptions() embedded the user-supplied --diag log path directly into the shared options string without quoting it: $"{options} --diag {connectionInfo.LogFile} --tracelevel {connectionInfo.TraceLevel}". A diag path containing a space (e.g. a Windows working directory like C:\Users\Jane Doe\project) splits into multiple invalid arguments when the resulting string is used to launch the testhost/dotnet exec process.
  2. StringExtensions.AddDoubleQuote() was a naive "\"" + value + "\"" wrapper: it didn't escape embedded " characters, and it didn't defend against the Windows CommandLineToArgvW trailing-backslash-before-closing-quote ambiguity (a path ending in \ immediately followed by the closing " is parsed as an escaped quote, corrupting the argument boundary — e.g. a drive root like D:\).

Fix

  • TestRunnerConnectionInfoExtensions.ToCommandLineOptions() now calls AddDoubleQuote() on connectionInfo.LogFile before interpolating it into the --diag option.
  • StringExtensions.AddDoubleQuote() now:
    • Escapes embedded " characters as \".
    • Doubles any run of trailing backslashes immediately preceding the closing quote, so paths like D:\ round-trip correctly.

This directly addresses Task 1 (High priority) and Task 2 (Medium priority) from the linked quality-focus issue (#16449). Task 3 (migrating to ProcessStartInfo.ArgumentList) is a larger/architectural change better suited for separate follow-up work, and is not attempted here.

Tests

  • test/Microsoft.TestPlatform.ObjectModel.UnitTests/Hosting/TestRunnerConnectionInfoExtensionsTests.cs: updated the existing diag-option test to expect the quoted path, and added a new test asserting a LogFile path containing spaces is quoted correctly.
  • test/Microsoft.TestPlatform.CoreUtilities.UnitTests/Extensions/StringExtensionsTests.cs (new file): covers plain values, values with spaces, embedded double quotes, single/multiple trailing backslashes, and backslashes not at the end of the string.

All targeted tests pass:

Microsoft.TestPlatform.CoreUtilities.UnitTests (StringExtensionsTests): 6/6 passed
Microsoft.TestPlatform.ObjectModel.UnitTests (TestRunnerConnectionInfoExtensionsTests): 7/7 passed

No public API surface was added (AddDoubleQuote already existed), so PublicAPI.Unshipped.txt files are unchanged.

Warning

Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

  • github.com
  • southcentralus0.in.applicationinsights.azure.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"
    - "southcentralus0.in.applicationinsights.azure.com"

See Network Configuration for more information.

🔍 Triaged by Issue Repro Triage & Auto-Fix 🔍


Note

This was originally intended as a pull request, but GitHub Actions is not permitted to create or approve pull requests in this repository.
The changes have been pushed to branch fix/issue-16449-eb54d27cbbe12452.

Click here to create the pull request

To fix the permissions issue, go to SettingsActionsGeneral and enable Allow GitHub Actions to create and approve pull requests. See also: gh-aw FAQ

Show patch preview (162 of 162 lines)
From f819f40bf205096dca31835715834f0cea659181 Mon Sep 17 00:00:00 2001
X-GH-AW-Base-Commit: 1f0a9175d5dc8c95232e80ab538149c79f13597c
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Wed, 9 Sep 2026 01:13:42 +0000
Subject: [PATCH] Quote --diag path and harden AddDoubleQuote against embedded
 quotes/trailing backslashes

Fixes #16449

- TestRunnerConnectionInfoExtensions.ToCommandLineOptions now quotes the
  --diag log file path, so paths containing spaces don't split into
  multiple invalid testhost command-line arguments.
- StringExtensions.AddDoubleQuote now escapes embedded double quotes and
  doubles any run of trailing backslashes immediately before the closing
  quote, avoiding the classic CommandLineToArgvW trailing-backslash
  ambiguity for paths like drive roots (D:\).
- Added unit tests for both changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../Extensions/StringExtensions.cs            | 20 +++++++-
 .../TestRunnerConnectionInfoExtensions.cs     |  3 +-
 .../Extensions/StringExtensionsTests.cs       | 47 +++++++++++++++++++
 ...TestRunnerConnectionInfoExtensionsTests.cs | 12 ++++-
 4 files changed, 79 insertions(+), 3 deletions(-)
 create mode 100644 test/Microsoft.TestPlatform.CoreUtilities.UnitTests/Extensions/StringExtensionsTests.cs

diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Extensions/StringExtensions.cs b/src/Microsoft.TestPlatform.CoreUtilities/Extensions/StringExtensions.cs
index c47ab59..2f2d6a8 100644
--- a/src/Microsoft.TestPlatform.CoreUtilities/Extensions/StringExtensions.cs
+++ b/src/Microsoft.TestPlatform.CoreUtilities/Extensions/StringExtensions.cs
@@ -7,11 +7,29 @@ public static class StringExtensions
 {
     /// <summary>
     /// Add double quote around string. Useful in case of path which has white space in between.
+    /// Embedded double quotes are escaped and any run of trailing backslashes immediately
+    /// preceding the closing quote is doubled, so
... (truncated)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions