Better windows argument generation with tests

src/Paket.Bootstrapper/WindowsProcessArguments.cs

@@ -5,53 +5,79 @@ namespace Paket.Bootstrapper
 {
     static class WindowsProcessArguments
     {
+        static void AddBackslashes(StringBuilder builder, int backslashes, bool beforeQuote)
+        {
+            if (backslashes == 0)
+            {
+                return;
+            }
+
+            // Always using 'backslashes * 2' would work it would just produce needless '\'
+            var count = beforeQuote || backslashes % 2 == 0
+                ? backslashes * 2
+                : (backslashes-1) * 2 + 1 ;
+
+            for(int i = 0; i < count; i++)
+            {
+                builder.Append('\\');
+            }
+        }
+
+        /* A good summary of the code used to process arguments in most windows programs can be found at :
+         * http://www.daviddeley.com/autohotkey/parameters/parameters.htm#WINARGV
+         */
         static void AppendEscaped(StringBuilder builder, string arg)
         {
-            var needQuote = arg.Contains(" ") || arg.Contains("\t");
+            builder.EnsureCapacity(builder.Length + arg.Length);
+
+            var needQuote = false;
+            var containsQuoteOrBackslash = false;
+            foreach(var c in arg)
+            {
+                needQuote |= (c == ' ');
+                needQuote |= (c == '\t');
+                containsQuoteOrBackslash |= (c == '"');
+                containsQuoteOrBackslash |= (c == '\\');
+            }
+
             if (needQuote)
             {
                 builder.Append('"');
             }
+            else if (!containsQuoteOrBackslash)
+            {
+                // No special characters are present, early exit
+                builder.Append(arg);
+                return;
+            }
 
             var index = 0;
+            var backslashes = 0;
             while (index < arg.Length)
             {
                 var c = arg[index];
-                if (c == '"')
+                if (c == '\\')
                 {
-                    builder.Append(@"\""");
-                    
-                } else if (c == '\\')
+                    backslashes++;
+                }
+                else if (c == '"')
                 {
-                    var isLast = index == arg.Length-1;
-                    if (isLast && needQuote)
-                    {
-                        builder.Append(@"\\");
-                    } else if (needQuote)
-                    {
-                        var next = arg[index+1];
-                        if (next == '"')
-                        {
-                            builder.Append(@"\\\""");
-                            index += 1;
-                        }
-                        else
-                        {
-                            builder.Append(c);
-                        }
-
-                    } else
-                    {
-                        builder.Append(c);
-                    }
+                    AddBackslashes(builder, backslashes, true);
+                    backslashes = 0;
+                    builder.Append('\\');
+                    builder.Append(c);
                 }
                 else
                 {
+                    AddBackslashes(builder, backslashes, false);
+                    backslashes = 0;
                     builder.Append(c);
                 }
                 index += 1;
             }
 
+            AddBackslashes(builder, backslashes, needQuote);
+
             if (needQuote)
             {
                 builder.Append('"');
@@ -60,7 +86,7 @@ static void AppendEscaped(StringBuilder builder, string arg)
 
         public static string ToString(IEnumerable<string> args)
         {
-            var builder = new StringBuilder();
+            var builder = new StringBuilder(255);
             foreach (var arg in args)
             {
                 if (builder.Length != 0)

tests/Paket.Bootstrapper.Tests/Paket.Bootstrapper.Tests.csproj

@@ -48,6 +48,7 @@
     <Compile Include="SemVerTests.cs" />
     <Compile Include="StartPaketBootstrappingTests.cs" />
     <Compile Include="DownloadStrategies\TemporarilyIgnoreUpdatesDownloadStrategyTest.cs" />
+    <Compile Include="WindowsProcessArguments.cs" />
   </ItemGroup>
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
   <Import Project="$(SolutionDir)\.paket\paket.targets" />

tests/Paket.Bootstrapper.Tests/WindowsProcessArguments.cs

@@ -0,0 +1,91 @@
+using NUnit.Framework;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Paket.Bootstrapper.Tests
+{
+    [TestFixture]
+    public class WindowsProcessArgumentsTests
+    {
+        private void Verify(string expected, params string[] argv)
+        {
+            var result = WindowsProcessArguments.ToString(argv);
+            Assert.That(result, Is.EqualTo(expected));
+        }
+
+        [Test]
+        public void Multiple_parameters_are_separated_by_spaces()
+        {
+            Verify("Hello World", "Hello", "World");
+        }
+
+        [Test]
+        public void No_quotes_are_added_when_not_needed()
+        {
+            Verify("Hello_World", "Hello_World");
+        }
+
+        [Test]
+        public void Quotes_are_added_when_arg_contains_space()
+        {
+            Verify(@"""Hello World""", "Hello World");
+        }
+
+        [Test]
+        public void Quote_is_escaped_inside()
+        {
+            Verify(@"Hello\""World", @"Hello""World");
+        }
+
+        [Test]
+        public void Quote_is_escaped_at_start()
+        {
+            Verify(@"\""HelloWorld", @"""HelloWorld");
+        }
+
+        [Test]
+        public void Quote_is_escaped_at_end()
+        {
+            Verify(@"HelloWorld\""", @"HelloWorld""");
+        }
+
+        [Test]
+        public void Backslash_alone_not_escaped()
+        {
+            Verify(@"Hello\World", @"Hello\World");
+        }
+
+        [Test]
+        public void Backslash_escaped_if_at_end_and_need_quote()
+        {
+            Verify(@"""Hello World\\""", @"Hello World\");
+        }
+
+        [Test]
+        public void Backslash_not_escaped_if_at_end_and_no_need_to_need_quote()
+        {
+            Verify(@"Hello_World\", @"Hello_World\");
+        }
+
+        [Test]
+        public void Backslash_before_quote_escaped()
+        {
+            Verify(@"Hello\\\""World", @"Hello\""World");
+        }
+
+        [Test]
+        public void Odd_backslash_escaped()
+        {
+            Verify(@"""a\\\\b c"" d e", @"a\\b c", "d", "e");
+        }
+
+        [Test]
+        public void Even_backslash_escaped()
+        {
+            Verify(@"""a\\\\\b c"" d e", @"a\\\b c", "d", "e");
+        }
+    }
+}