feat: escape msbuild argument when contains special chars

Author: filzrev

src/BenchmarkDotNet/Jobs/Argument.cs

@@ -47,6 +47,71 @@ public MonoArgument(string value) : base(value)
     [PublicAPI]
     public class MsBuildArgument : Argument
     {
+        // Characters that need to be escaped.
+        // 1. Space
+        // 2. Comma (Special char that is used for separater for value of `-property:{name}={value}` and `-restoreProperty:{name}={value}`)
+        // 3. Other MSBuild special chars (https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-special-characters?view=vs-2022)
+        private static readonly char[] MSBuildCharsToEscape = [' ', ',', '%', '$', '@', '\'', '(', ')', ';', '?', '*'];
+
         public MsBuildArgument(string value) : base(value) { }
+
+        /// <summary>
+        /// Gets the MSBuild argument that is used for build script.
+        /// </summary>
+        internal string GetEscapedTextRepresentation()
+        {
+            var originalArgument = TextRepresentation;
+
+            // If entire argument surrounded with double quote, returns original argument.
+            // In this case. MSBuild special chars must be escaped by user. https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-special-characters
+            if (originalArgument.StartsWith("\""))
+                return originalArgument;
+
+            // Process MSBuildArgument that contains '=' char. (e.g. `--property:{key}={value}` and `-restoreProperty:{key}={value}`)
+            // See: https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-command-line-reference?view=vs-2022
+            var values = originalArgument.Split(['='], 2);
+            if (values.Length != 2)
+                return originalArgument;
+
+            var key = values[0];
+            var value = values[1];
+
+            // If value starts with `\` char. It is expected that the escaped value is specified by the user.
+            if (value.StartsWith("\\"))
+                return originalArgument;
+
+            // If value don't contains special chars. return original value.
+            if (value.IndexOfAny(MSBuildCharsToEscape) < 0)
+                return originalArgument;
+
+            return $"{key}={GetEscapedValue(value)}";
+        }
+
+        private static string GetEscapedValue(string value)
+        {
+            // If value starts with double quote. Trim leading/trailing double quote
+            if (value.StartsWith("\""))
+                value = value.Trim(['"']);
+
+            bool isWindows = true;
+#if NET
+            isWindows = OperatingSystem.IsWindows();
+#endif
+            if (isWindows)
+            {
+                // On Windows environment.
+                // Returns double-quoted value. (Command line execution and `.bat` file requires escape double quote with `\`)
+                return $"""
+                        \"{value}\"
+                        """;
+            }
+
+            // On non-Windows environment.
+            // Returns value that surround with `'"` and `"'`. See: https://github.com/dotnet/sdk/issues/8792#issuecomment-393756980
+            // It requires escape with `\` when running command with `.sh` file. )
+            return $"""
+                    \'\"{value}\"\'
+                    """;
+        }
     }
-}
+}

src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs

@@ -201,7 +201,7 @@ private static string GetCustomMsBuildArguments(BenchmarkCase benchmarkCase, IRe
 
             var msBuildArguments = benchmarkCase.Job.ResolveValue(InfrastructureMode.ArgumentsCharacteristic, resolver).OfType<MsBuildArgument>();
 
-            return string.Join(" ", msBuildArguments.Select(arg => arg.TextRepresentation));
+            return string.Join(" ", msBuildArguments.Select(arg => arg.GetEscapedTextRepresentation()));
         }
 
         private static IEnumerable<string> GetNuGetAddPackageCommands(BenchmarkCase benchmarkCase, IResolver resolver)

tests/BenchmarkDotNet.Tests/Jobs/MsBuildArgumentTests.cs

@@ -0,0 +1,55 @@
+using BenchmarkDotNet.Jobs;
+using System;
+using Xunit;
+
+namespace BenchmarkDotNet.Tests
+{
+    public class MsBuildArgumentTests
+    {
+        [Theory]
+        [InlineData("/p:CustomProperty=100%", "100%")]               // Contains percentage
+        [InlineData("/p:CustomProperty=AAA;BBB", "AAA;BBB")]         // Contains semicolon
+        [InlineData("/p:CustomProperty=AAA,BBB", "AAA,BBB")]         // Contains comma
+        [InlineData("/p:CustomProperty=AAA BBB", "AAA BBB")]         // Contains space
+        [InlineData("/p:CustomProperty=AAA%3BBBB", "AAA%3BBBB")]     // Contains escaped semicolon
+        [InlineData("/p:CustomProperty=\"AAA%3BBBB\"", "AAA%3BBBB")] // Contains escaped semicolon. and surrounded with non-escaped double quote
+        [InlineData("/p:NoWarn=1591;1573;3001", "1591;1573;3001")]   // NoWarn property that contains semicolon
+        public void MSBuildArgument_ContainsSpecialChars(string argument, string expected)
+        {
+            var result = new MsBuildArgument(argument).GetEscapedTextRepresentation();
+            AssertEscapedValue(result, expected);
+        }
+
+        [Theory]
+        [InlineData("/p:CustomProperty=AAA_BBB")]                   // Argument that don't contains special chars
+        [InlineData("\"/p:CustomProperty=AAA%3BBBB\"")]             // Argument is surrounded with double quote and value is escaped
+        [InlineData("/p:CustomProperty=\\\"100%3B200\\\"")]         // Value is surrounded with double quote and value is escaped (For Windows environment)
+        [InlineData("/p:CustomProperty=\\\'\\\"100%3B200\\\"\\\'")] // Value is surrounded with double quote and value is escaped (For non-Windows environment)
+        [InlineData("/noWarn:1591;1573;3001")]                      // Other argument should not be escaped
+        public void MSBuildArgument_ShouldNotBeEscaped(string originalArg)
+        {
+            var result = new MsBuildArgument(originalArg).GetEscapedTextRepresentation();
+            Assert.Equal(expected: originalArg, result);
+        }
+
+        private static void AssertEscapedValue(string escapedArgument, string expectedValue)
+        {
+            var values = escapedArgument.Split(['='], 2);
+            Assert.Equal(2, values.Length);
+
+            var key = values[0];
+            var value = values[1];
+
+#if NET
+            // On non-Windows environment value should be surrounded with escaped `'"` and `"'`
+            if (!OperatingSystem.IsWindows())
+            {
+                Assert.Equal("\\\'\\\"" + expectedValue + "\\\"\\\'", value);
+                return;
+            }
+#endif
+            // On Windows environment. Value should be quote with escaped `\"`
+            Assert.Equal("\\\"" + expectedValue + "\\\"", value);
+        }
+    }
+}