diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index dc6273622ff..1e25829f60b 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/documentation/wiki/ChangeWaves.md @@ -26,6 +26,7 @@ The opt-out comes in the form of setting the environment variable `MSBuildDisabl ### 16.10 - [Error when a property expansion in a condition has whitespace](https://github.com/dotnet/msbuild/pull/5672) - [Allow Custom CopyToOutputDirectory Location With TargetPath](https://github.com/dotnet/msbuild/pull/6237) +- [Allow users that have certain special characters in their username to build successfully when using exec](https://github.com/dotnet/msbuild/pull/6223) ### 17.0 ## Change Waves No Longer In Rotation diff --git a/src/Shared/TempFileUtilities.cs b/src/Shared/TempFileUtilities.cs index ae76d151a37..6f560189522 100644 --- a/src/Shared/TempFileUtilities.cs +++ b/src/Shared/TempFileUtilities.cs @@ -19,9 +19,10 @@ internal static partial class FileUtilities /// Caller must delete when finished. /// /// - internal static string GetTemporaryDirectory(bool createDirectory = true) + /// + internal static string GetTemporaryDirectory(bool createDirectory = true, string subfolder = null) { - string temporaryDirectory = Path.Combine(Path.GetTempPath(), "Temporary" + Guid.NewGuid().ToString("N")); + string temporaryDirectory = Path.Combine(Path.GetTempPath(), "Temporary" + Guid.NewGuid().ToString("N"), subfolder ?? string.Empty); if (createDirectory) { diff --git a/src/Shared/UnitTests/TestEnvironment.cs b/src/Shared/UnitTests/TestEnvironment.cs index 569bd92a66e..9922585a325 100644 --- a/src/Shared/UnitTests/TestEnvironment.cs +++ b/src/Shared/UnitTests/TestEnvironment.cs @@ -172,6 +172,15 @@ public TransientTempPath CreateNewTempPath() return SetTempPath(folder.Path, true); } + /// + /// Creates a new temp path with a custom subfolder + /// + public TransientTempPath CreateNewTempPathWithSubfolder(string subfolder) + { + var folder = CreateFolder(null, true, subfolder); + return SetTempPath(folder.Path, true); + } + /// /// Creates a new temp path /// Sets all OS temp environment variables to the new path @@ -266,9 +275,9 @@ public TransientTestFile ExpectFile(TransientTestFolder transientTestFolder, str /// Creates a test variant used to add a unique temporary folder during a test. Will be deleted when the test /// completes. /// - public TransientTestFolder CreateFolder(string folderPath = null, bool createFolder = true) + public TransientTestFolder CreateFolder(string folderPath = null, bool createFolder = true, string subfolder = null) { - var folder = WithTransientTestState(new TransientTestFolder(folderPath, createFolder)); + var folder = WithTransientTestState(new TransientTestFolder(folderPath, createFolder, subfolder)); Assert.True(!(createFolder ^ FileSystems.Default.DirectoryExists(folder.Path))); @@ -605,9 +614,9 @@ public void Delete() public class TransientTestFolder : TransientTestState { - public TransientTestFolder(string folderPath = null, bool createFolder = true) + public TransientTestFolder(string folderPath = null, bool createFolder = true, string subfolder = null) { - Path = folderPath ?? FileUtilities.GetTemporaryDirectory(createFolder); + Path = folderPath ?? FileUtilities.GetTemporaryDirectory(createFolder, subfolder); if (createFolder) { diff --git a/src/Tasks.UnitTests/Exec_Tests.cs b/src/Tasks.UnitTests/Exec_Tests.cs index 3bc0657cab2..67fc7772e5b 100644 --- a/src/Tasks.UnitTests/Exec_Tests.cs +++ b/src/Tasks.UnitTests/Exec_Tests.cs @@ -48,6 +48,50 @@ private ExecWrapper PrepareExecWrapper(string command) return exec; } + [Fact] + [Trait("Category", "mono-osx-failing")] + [Trait("Category", "netcore-osx-failing")] + [Trait("Category", "netcore-linux-failing")] + public void EscapeSpecifiedCharactersInPathToGeneratedBatchFile() + { + using (var testEnvironment = TestEnvironment.Create()) + { + var newTempPath = testEnvironment.CreateNewTempPathWithSubfolder("hello()w]o(rld)").TempPath; + + string tempPath = Path.GetTempPath(); + Assert.StartsWith(newTempPath, tempPath); + + // Now run the Exec task on a simple command. + Exec exec = PrepareExec("echo Hello World!"); + exec.Execute().ShouldBeTrue(); + } + } + + [Fact] + [Trait("Category", "mono-osx-failing")] + [Trait("Category", "netcore-osx-failing")] + [Trait("Category", "netcore-linux-failing")] + public void EscapeSpecifiedCharactersInPathToGeneratedBatchFile_DisabledUnderChangeWave16_10() + { + using (var testEnvironment = TestEnvironment.Create()) + { + ChangeWaves.ResetStateForTests(); + testEnvironment.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave16_10.ToString()); + BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly(); + + var newTempPath = testEnvironment.CreateNewTempPathWithSubfolder("hello()w]o(rld)").TempPath; + + string tempPath = Path.GetTempPath(); + Assert.StartsWith(newTempPath, tempPath); + + // Now run the Exec task on a simple command. + Exec exec = PrepareExec("echo Hello World!"); + exec.Execute().ShouldBeFalse(); + + ChangeWaves.ResetStateForTests(); + } + } + /// /// Ensures that calling the Exec task does not leave any extra TEMP files /// lying around. @@ -918,6 +962,117 @@ public void EndToEndMultilineExec() } } } + + [Fact] + [Trait("Category", "mono-osx-failing")] + [Trait("Category", "netcore-osx-failing")] + [Trait("Category", "netcore-linux-failing")] + public void EndToEndMultilineExec_EscapeSpecialCharacters() + { + using (var env = TestEnvironment.Create(_output)) + { + var testProject = env.CreateTestProjectWithFiles(@" + + + +"); + + // Ensure path has subfolders + var newTempPath = env.CreateNewTempPathWithSubfolder("hello()wo(rld)").TempPath; + string tempPath = Path.GetTempPath(); + Assert.StartsWith(newTempPath, tempPath); + + using (var buildManager = new BuildManager()) + { + MockLogger logger = new MockLogger(_output, profileEvaluation: false, printEventsToStdout: false); + + var parameters = new BuildParameters() + { + Loggers = new[] { logger }, + }; + + var collection = new ProjectCollection( + new Dictionary(), + new[] { logger }, + remoteLoggers: null, + ToolsetDefinitionLocations.Default, + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: true); + + var project = collection.LoadProject(testProject.ProjectFile).CreateProjectInstance(); + + var request = new BuildRequestData( + project, + targetsToBuild: new[] { "ExecCommand" }, + hostServices: null); + + var result = buildManager.Build(parameters, request); + + logger.AssertLogContains("Hello, World!"); + + result.OverallResult.ShouldBe(BuildResultCode.Success); + } + } + } + + [Fact] + [Trait("Category", "mono-osx-failing")] + [Trait("Category", "netcore-osx-failing")] + [Trait("Category", "netcore-linux-failing")] + public void EndToEndMultilineExec_EscapeSpecialCharacters_DisabledUnderChangeWave16_10() + { + using (var env = TestEnvironment.Create(_output)) + { + ChangeWaves.ResetStateForTests(); + env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave16_10.ToString()); + BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly(); + + var testProject = env.CreateTestProjectWithFiles(@" + + + +"); + + // Ensure path has subfolders + var newTempPath = env.CreateNewTempPathWithSubfolder("hello()wo(rld)").TempPath; + string tempPath = Path.GetTempPath(); + Assert.StartsWith(newTempPath, tempPath); + + using (var buildManager = new BuildManager()) + { + MockLogger logger = new MockLogger(_output, profileEvaluation: false, printEventsToStdout: false); + + var parameters = new BuildParameters() + { + Loggers = new[] { logger }, + }; + + var collection = new ProjectCollection( + new Dictionary(), + new[] { logger }, + remoteLoggers: null, + ToolsetDefinitionLocations.Default, + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: true); + + var project = collection.LoadProject(testProject.ProjectFile).CreateProjectInstance(); + + var request = new BuildRequestData( + project, + targetsToBuild: new[] { "ExecCommand" }, + hostServices: null); + + var result = buildManager.Build(parameters, request); + + logger.AssertLogContains("Hello, World!"); + + result.OverallResult.ShouldBe(BuildResultCode.Failure); + } + ChangeWaves.ResetStateForTests(); + } + } } internal class ExecWrapper : Exec diff --git a/src/Tasks/Exec.cs b/src/Tasks/Exec.cs index 2132b8266db..77fa495a484 100644 --- a/src/Tasks/Exec.cs +++ b/src/Tasks/Exec.cs @@ -54,6 +54,9 @@ public Exec() private Encoding _standardOutputEncoding; private string _command; + // '^' before _any_ character escapes that character, don't escape it. + private static readonly char[] _charactersToEscape = { '(', ')', '=', ';', '!', ',', '&', ' '}; + #endregion #region Properties @@ -601,13 +604,45 @@ protected internal override void AddCommandLineCommands(CommandLineBuilderExtens } commandLine.AppendSwitch("/C"); // run then terminate - // If for some crazy reason the path has a & character and a space in it - // then get the short path of the temp path, which should not have spaces in it - // and then escape the & - if (batchFileForCommandLine.Contains("&") && !batchFileForCommandLine.Contains("^&")) + if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_10)) { - batchFileForCommandLine = NativeMethodsShared.GetShortFilePath(batchFileForCommandLine); - batchFileForCommandLine = batchFileForCommandLine.Replace("&", "^&"); + StringBuilder fileName = null; + + // Escape special characters that need to be escaped. + for (int i = 0; i < batchFileForCommandLine.Length; i++) + { + char c = batchFileForCommandLine[i]; + + if (ShouldEscapeCharacter(c) && (i == 0 || batchFileForCommandLine[i - 1] != '^')) + { + // Avoid allocating a new string until we know we have something to escape. + if (fileName == null) + { + fileName = StringBuilderCache.Acquire(batchFileForCommandLine.Length); + fileName.Append(batchFileForCommandLine, 0, i); + } + + fileName.Append('^'); + } + + fileName?.Append(c); + } + + if (fileName != null) + { + batchFileForCommandLine = StringBuilderCache.GetStringAndRelease(fileName); + } + } + else + { + // If for some crazy reason the path has a & character and a space in it + // then get the short path of the temp path, which should not have spaces in it + // and then escape the & + if (batchFileForCommandLine.Contains("&") && !batchFileForCommandLine.Contains("^&")) + { + batchFileForCommandLine = NativeMethodsShared.GetShortFilePath(batchFileForCommandLine); + batchFileForCommandLine = batchFileForCommandLine.Replace("&", "^&"); + } } } @@ -615,6 +650,19 @@ protected internal override void AddCommandLineCommands(CommandLineBuilderExtens } } + private bool ShouldEscapeCharacter(char c) + { + for (int i = 0; i < _charactersToEscape.Length; i++) + { + if (c == _charactersToEscape[i]) + { + return true; + } + } + + return false; + } + #endregion #region Overridden properties