Fix various bugs for how retry and test host controllers start child processes by @Youssef1313 in #6462 (backport to rel/3.10) (#6463)

Co-authored-by: Youssef1313 <youssefvictor00@gmail.com>

Author: youssef-backport-bot

src/Platform/Microsoft.Testing.Extensions.Retry/RetryOrchestrator.cs

@@ -169,23 +169,23 @@ public async Task<int> OrchestrateTestHostExecutionAsync()
             finalArguments.Add($"--{RetryCommandLineOptionsProvider.RetryFailedTestsPipeNameOptionName}");
             finalArguments.Add(retryFailedTestsPipeServer.PipeName);
 
-            // Prepare the process start
-            ProcessStartInfo processStartInfo = new()
+#if NET8_0_OR_GREATER
+            List<string> arguments = finalArguments;
+#else
+            var builder = new StringBuilder();
+            foreach (string arg in finalArguments)
             {
-                FileName = executableInfo.FilePath,
-#if !NETCOREAPP
-                UseShellExecute = false,
+                PasteArguments.AppendArgument(builder, arg);
+            }
+
+            string arguments = builder.ToString();
 #endif
-            };
 
-            foreach (string argument in finalArguments)
+            // Prepare the process start
+            ProcessStartInfo processStartInfo = new(executableInfo.FilePath, arguments)
             {
-#if !NETCOREAPP
-                processStartInfo.Arguments += argument + " ";
-#else
-                processStartInfo.ArgumentList.Add(argument);
-#endif
-            }
+                UseShellExecute = false,
+            };
 
             await logger.LogDebugAsync($"Starting test host process, attempt {attemptCount}/{userMaxRetryCount}").ConfigureAwait(false);
             IProcess testHostProcess = _serviceProvider.GetProcessHandler().Start(processStartInfo)

src/Platform/Microsoft.Testing.Platform/Helpers/PasteArguments.cs

@@ -0,0 +1,98 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Microsoft.Testing.Platform.Helpers;
+
+// https://github.com/dotnet/runtime/blob/019c8d72effbdb4f69564695f1df7c4417ec060d/src/libraries/System.Private.CoreLib/src/System/PasteArguments.cs
+internal static partial class PasteArguments
+{
+    // WARNING: Don't change the signature of this internal method. It's exposed via IVT to other assemblies.
+    // Renaming it is a BREAKING CHANGE.
+    internal static void AppendArgument(StringBuilder stringBuilder, string argument)
+    {
+        if (stringBuilder.Length != 0)
+        {
+            stringBuilder.Append(' ');
+        }
+
+        // Parsing rules for non-argv[0] arguments:
+        //   - Backslash is a normal character except followed by a quote.
+        //   - 2N backslashes followed by a quote ==> N literal backslashes followed by unescaped quote
+        //   - 2N+1 backslashes followed by a quote ==> N literal backslashes followed by a literal quote
+        //   - Parsing stops at first whitespace outside of quoted region.
+        //   - (post 2008 rule): A closing quote followed by another quote ==> literal quote, and parsing remains in quoting mode.
+        if (argument.Length != 0 && ContainsNoWhitespaceOrQuotes(argument))
+        {
+            // Simple case - no quoting or changes needed.
+            stringBuilder.Append(argument);
+        }
+        else
+        {
+            stringBuilder.Append(Quote);
+            int idx = 0;
+            while (idx < argument.Length)
+            {
+                char c = argument[idx++];
+                if (c == Backslash)
+                {
+                    int numBackSlash = 1;
+                    while (idx < argument.Length && argument[idx] == Backslash)
+                    {
+                        idx++;
+                        numBackSlash++;
+                    }
+
+                    if (idx == argument.Length)
+                    {
+                        // We'll emit an end quote after this so must double the number of backslashes.
+                        stringBuilder.Append(Backslash, numBackSlash * 2);
+                    }
+                    else if (argument[idx] == Quote)
+                    {
+                        // Backslashes will be followed by a quote. Must double the number of backslashes.
+                        stringBuilder.Append(Backslash, (numBackSlash * 2) + 1);
+                        stringBuilder.Append(Quote);
+                        idx++;
+                    }
+                    else
+                    {
+                        // Backslash will not be followed by a quote, so emit as normal characters.
+                        stringBuilder.Append(Backslash, numBackSlash);
+                    }
+
+                    continue;
+                }
+
+                if (c == Quote)
+                {
+                    // Escape the quote so it appears as a literal. This also guarantees that we won't end up generating a closing quote followed
+                    // by another quote (which parses differently pre-2008 vs. post-2008.)
+                    stringBuilder.Append(Backslash);
+                    stringBuilder.Append(Quote);
+                    continue;
+                }
+
+                stringBuilder.Append(c);
+            }
+
+            stringBuilder.Append(Quote);
+        }
+    }
+
+    private static bool ContainsNoWhitespaceOrQuotes(string s)
+    {
+        for (int i = 0; i < s.Length; i++)
+        {
+            char c = s[i];
+            if (char.IsWhiteSpace(c) || c == Quote)
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    private const char Quote = '\"';
+    private const char Backslash = '\\';
+}

src/Platform/Microsoft.Testing.Platform/Hosts/TestHostControllersTestHost.cs

@@ -104,7 +104,13 @@ protected override async Task<int> InternalRunAsync()
 #if NET8_0_OR_GREATER
             IEnumerable<string> arguments = partialCommandLine;
 #else
-            string arguments = string.Join(' ', partialCommandLine);
+            var builder = new StringBuilder();
+            foreach (string arg in partialCommandLine)
+            {
+                PasteArguments.AppendArgument(builder, arg);
+            }
+
+            string arguments = builder.ToString();
 #endif
 
 #pragma warning disable CA1416 // Validate platform compatibility
@@ -119,9 +125,7 @@ protected override async Task<int> InternalRunAsync()
                     { $"{EnvironmentVariableConstants.TESTINGPLATFORM_TESTHOSTCONTROLLER_SKIPEXTENSION}_{currentPid}", "1" },
                     { $"{EnvironmentVariableConstants.TESTINGPLATFORM_TESTHOSTCONTROLLER_PIPENAME}_{currentPid}", testHostControllerIpc.PipeName.Name },
                 },
-#if !NETCOREAPP
                 UseShellExecute = false,
-#endif
             };
 #pragma warning restore CA1416