diff --git a/Tests/SonarScanner.MSBuild.Common.Test/ProcessRunnerTests.cs b/Tests/SonarScanner.MSBuild.Common.Test/ProcessRunnerTests.cs index 1d8528e4b..e6106fca4 100644 --- a/Tests/SonarScanner.MSBuild.Common.Test/ProcessRunnerTests.cs +++ b/Tests/SonarScanner.MSBuild.Common.Test/ProcessRunnerTests.cs @@ -23,6 +23,7 @@ using System.Diagnostics; using System.IO; using System.Linq; +using System.Reflection; using FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; @@ -342,11 +343,8 @@ REM The sonar-scanner.bat uses %* to pass the argument to javac.exe logger.InfoMessages.Should().BeEquivalentTo( @"""-Dsonar.scanAllFiles=true"" ""-Dproject.settings=D:\DevLibTest\ClassLibraryTest.sonarqube\out\sonar-project.properties"" ""--from=ScannerMSBuild/5.13.1"" ""--debug""", @"""-Dsonar.scanAllFiles=true""", - string.Empty, @"""-Dproject.settings=D:\DevLibTest\ClassLibraryTest.sonarqube\out\sonar-project.properties""", - string.Empty, @"""--from=ScannerMSBuild/5.13.1""", - string.Empty, @"""--debug"""); } @@ -398,6 +396,31 @@ public void ProcRunner_DoNotLogSensitiveData() AssertExpectedLogContents(testDir, allArgs); } + [TestMethod] + public void Test_ShellEscape_NoSpecialCharacters() + { + // Arrange + string input = "-Dsonar.scanAllFiles=true -Dproject.settings=D:\\DevLibTest\\ClassLibraryTest.sonarqube\\out\\sonar-project.properties --from=ScannerMSBuild/5.13.1"; + + // Act + string result = InvokeShellEscape(input); + + // Assert + Assert.AreEqual(input, result); + } + + [TestMethod] + public void Test_ShellEscape_WithSpecialCharacters() + { + // Arrange + string input = "-Dsonar.scanAllFiles=true| -Dproject.settings=D:\\DevLibTest\\ClassLibraryTest.sonarqube\\out\\sonar-project.properties^ --from=ScannerMSBuild/5.13.1"; + + // Act + string result = InvokeShellEscape(input); + + // Assert + Assert.AreEqual("-Dsonar.scanAllFiles=true^^^| -Dproject.settings=D:\\DevLibTest\\ClassLibraryTest.sonarqube\\out\\sonar-project.properties^^^^ --from=ScannerMSBuild/5.13.1", result); + } #endregion Tests #region Private methods @@ -441,6 +464,11 @@ private static void AssertTextDoesNotAppearInLog(string text, IList logE "Specified text should not appear anywhere in the log file: {0}", text); } + private string InvokeShellEscape(string input) + { + MethodInfo methodInfo = typeof(ProcessRunnerArguments).GetMethod("ShellEscape", BindingFlags.NonPublic | BindingFlags.Static); + return (string)methodInfo.Invoke(null, new object[] { input }); + } #endregion Private methods } } diff --git a/src/SonarScanner.MSBuild.Common/ProcessRunnerArguments.cs b/src/SonarScanner.MSBuild.Common/ProcessRunnerArguments.cs index 5d85eb852..65b3a2ba1 100644 --- a/src/SonarScanner.MSBuild.Common/ProcessRunnerArguments.cs +++ b/src/SonarScanner.MSBuild.Common/ProcessRunnerArguments.cs @@ -65,6 +65,8 @@ public ProcessRunnerArguments(string exeName, bool isBatchScript) /// public IDictionary EnvironmentVariables { get; set; } + private static readonly HashSet SpecialChars = new HashSet { '^', '>', '<', '|', '&', '"' }; + public string GetEscapedArguments() { if (CmdLineArgs == null) @@ -216,17 +218,16 @@ private static string ShellEscape(string argLine) var sb = new StringBuilder(); foreach (var c in argLine) { - // This escape is required after %* is expanded to prevent command injections - sb.Append('^'); - sb.Append('^'); - - // This escape is required only to pass the argument line to the batch script - sb.Append('^'); + if (SpecialChars.Contains(c)) + { + sb.Append('^'); + sb.Append('^'); + sb.Append('^'); + } sb.Append(c); } return sb.ToString(); } - #endregion Public properties } }