Merge pull request #221 from netspiri/microsoft/#3

Avoid Quoting Quoted Paths

BoostTestAdapter/Utility/CommandLine.cs

@@ -5,6 +5,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 
 namespace BoostTestAdapter.Utility
@@ -44,13 +45,25 @@ public static IEnumerable<string> Split(this string str, Func<char, bool> contro
         /// <returns>'input' with the beginning and end 'quote' removed</returns>
         public static string TrimMatchingQuotes(this string input, char quote)
         {
-            if ((input.Length >= 2) &&
-                (input[0] == quote) && (input[input.Length - 1] == quote))
+            if (IsQuoted(input, quote))
+            {
                 return input.Substring(1, input.Length - 2);
+            }
 
             return input;
         }
 
+        /// <summary>
+        /// States if the provided string is quoted using the provided quotation marks
+        /// </summary>
+        /// <param name="input">The input string to test</param>
+        /// <param name="mark">The quotation mark to test for</param>
+        /// <returns>true if the input string is within quotation marks; false otherwise</returns>
+        internal static bool IsQuoted(string input, char mark = '"')
+        {
+            return (input.Length >= 2) && (input[0] == mark) && (input[input.Length - 1] == mark);
+        }
+
         /// <summary>
         /// Safely splits a command line string into its argument components.
         /// </summary>
@@ -64,14 +77,20 @@ public static IEnumerable<string> SplitCommandLine(string commandLine)
 
             return commandLine.Split(c =>
             {
-                if (c == '\\' && !isEscaping) { isEscaping = true; return false; }
+                if ((c == '\\') && (!isEscaping))
+                {
+                    isEscaping = true;
+                    return false;
+                }
 
-                if (c == '\"' && !isEscaping)
+                if ((c == '\"') && (!isEscaping))
+                {
                     inQuotes = !inQuotes;
+                }
 
                 isEscaping = false;
 
-                return !inQuotes && Char.IsWhiteSpace(c)/*c == ' '*/;
+                return !inQuotes && char.IsWhiteSpace(c);
             })
                 .Select(arg => arg.Trim().TrimMatchingQuotes('\"').Replace("\\\"", "\""))
                 .Where(arg => !string.IsNullOrEmpty(arg));
@@ -108,26 +127,17 @@ public CommandLine(string fileName, string args)
         /// <param name="fileName">The file component of the command line</param>
         /// <param name="arguments">The arguments of the command line</param>
         /// <remarks>Handles quoting in case of spaces</remarks>
-        public CommandLine(string fileName, IEnumerable<string> arguments)
+        public CommandLine(string fileName, IEnumerable<string> arguments) :
+            this(fileName, JoinArguments(arguments))
         {
-            FileName = fileName;
-            Arguments = "";
-
-            if (arguments == null)
-                return;
-
-            foreach(string arg in arguments)
-            {
-                Arguments += (arg.Contains(' ') ? "\"" + arg + "\"" : arg) + " ";
-            }
         }
 
         public string FileName { get; set; }
         public string Arguments { get; set; }
 
         public override string ToString()
         {
-            return FileName + ' ' + Arguments;
+            return NormalizePath(FileName) + ' ' + Arguments;
         }
 
         /// <summary>
@@ -137,13 +147,55 @@ public override string ToString()
         /// <returns>The command line instance parsed from cmdLine</returns>
         public static CommandLine FromString(string cmdLine)
         {
-            cmdLine = (cmdLine == null) ? string.Empty : cmdLine;
+            cmdLine = cmdLine ?? string.Empty;
 
             var splitCommandLine = CommandLineArgExtensions.SplitCommandLine(cmdLine);
             return new CommandLine(
                 splitCommandLine.FirstOrDefault(),
                 splitCommandLine.Skip(1)
             );
         }
+
+        /// <summary>
+        /// Concatenates a collection of strings as a valid command-line argument set
+        /// </summary>
+        /// <param name="arguments">The arguments to serialize</param>
+        /// <returns>string representing the concatenation set of all arguments</returns>
+        private static string JoinArguments(IEnumerable<string> arguments)
+        {
+            if (arguments == null)
+            {
+                return string.Empty;
+            }
+
+            var quotedArgs = arguments.Select(arg => arg.Contains(' ') ? string.Format(CultureInfo.InvariantCulture, "\"{0}\"", arg) : arg);
+            return string.Join(" ", quotedArgs);
+        }
+
+        /// <summary>
+        /// Normalizes the file path and adds quotes should the path not be quoted already
+        /// </summary>
+        /// <param name="filePath">The path to notmalized</param>
+        /// <returns>A normalized file path</returns>
+        private static string NormalizePath(string filePath)
+        {
+            var path = filePath?.Trim();
+
+            // Invalid input, return immediately
+            if (string.IsNullOrEmpty(path))
+            {
+                return path;
+            }
+
+            const char mark = '"';
+
+            // If the path is already quoted, leave as is
+            if (CommandLineArgExtensions.IsQuoted(path, mark))
+            {
+                return path;
+            }
+
+            return path.Contains(' ') ? (mark + path + mark) : path;
+        }
     }
 }

BoostTestAdapter/Utility/ExecutionContext/DefaultProcessExecutionContext.cs

@@ -5,14 +5,15 @@
 
 using System;
 using System.Diagnostics;
+using System.Globalization;
 
 using JobManagement;
 
 namespace BoostTestAdapter.Utility.ExecutionContext
 {
     /// <summary>
     /// An IProcessExecutionContext which produces regular sub-processes.
-    /// Guarantees that sub-processes are destoryed when this processes is killed.
+    /// Guarantees that sub-processes are destroyed when this processes is killed.
     /// </summary>
     public class DefaultProcessExecutionContext : IProcessExecutionContext
     {
@@ -101,10 +102,10 @@ private static ProcessStartInfo CreateStartInfo(ProcessExecutionContextArgs args
                 WindowStyle = ProcessWindowStyle.Hidden,
 
                 WorkingDirectory = args.WorkingDirectory,
-                
+
                 // Start the process through cmd.exe to allow redirection operators to work as expected
                 FileName = "cmd.exe",
-                Arguments = "/S /C \"\"" + args.FilePath + "\" " + args.Arguments + '"',
+                Arguments = string.Format(CultureInfo.InvariantCulture, "/S /C \"{0}\"", new CommandLine(args.FilePath, args.Arguments).ToString()),
 
                 // Redirection should be specified as part of 'args.Arguments' and sent to a file
                 RedirectStandardError = false,

BoostTestAdapterNunit/BoostTestSettingsTest.cs

@@ -106,7 +106,7 @@ public void ParseSampleSettings()
 
             Assert.That(settings.ExternalTestRunner, Is.Not.Null);
             Assert.That(settings.ExternalTestRunner.ExtensionType.ToString(), Is.EqualTo(".dll"));
-            Assert.That(settings.ExternalTestRunner.ExecutionCommandLine.ToString(), Is.EqualTo("C:\\ExternalTestRunner.exe --test {source} "));
+            Assert.That(settings.ExternalTestRunner.ExecutionCommandLine.ToString(), Is.EqualTo("C:\\ExternalTestRunner.exe --test {source}"));
         }
 
         /// <summary>