diff --git a/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs b/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs index 876ae0c08e0ad2453a717e61e078cdb81e1023c3..45659e9427acbb7034b96a0340f2b137bf0e70a5 100644 --- a/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs +++ b/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs @@ -542,5 +542,39 @@ public async Task IncorrectServerHashReturnsIncorrectHashResponse() Assert.Equal(BuildResponse.ResponseType.IncorrectHash, buildResponse.Type); } } + + [ConditionalFact(typeof(WindowsDesktopOnly))] + [WorkItem(33452, "https://github.com/dotnet/roslyn/issues/33452")] + public void QuotePipeName_Desktop() + { + var serverInfo = BuildServerConnection.GetServerProcessInfo(@"q:\tools", "name with space"); + Assert.Equal(@"q:\tools\VBCSCompiler.exe", serverInfo.processFilePath); + Assert.Equal(@"q:\tools\VBCSCompiler.exe", serverInfo.toolFilePath); + Assert.Equal(@"""-pipename:name with space""", serverInfo.commandLineArguments); + } + + [ConditionalFact(typeof(CoreClrOnly))] + [WorkItem(33452, "https://github.com/dotnet/roslyn/issues/33452")] + public void QuotePipeName_CoreClr() + { + var toolDir = ExecutionConditionUtil.IsWindows + ? @"q:\tools" + : "/tools"; + var serverInfo = BuildServerConnection.GetServerProcessInfo(toolDir, "name with space"); + var vbcsFilePath = Path.Combine(toolDir, "VBCSCompiler.dll"); + Assert.Equal(vbcsFilePath, serverInfo.toolFilePath); + Assert.Equal($@"exec ""{vbcsFilePath}"" ""-pipename:name with space""", serverInfo.commandLineArguments); + } + + [Theory] + [InlineData(@"name with space.T.basename", "name with space", true, "basename")] + [InlineData(@"ha_ha.T.basename", @"ha""ha", true, "basename")] + [InlineData(@"jared.T.ha_ha", @"jared", true, @"ha""ha")] + [InlineData(@"jared.F.ha_ha", @"jared", false, @"ha""ha")] + [InlineData(@"jared.F.ha_ha", @"jared", false, @"ha\ha")] + public void GetPipeNameCore(string expectedName, string userName, bool isAdmin, string basePipeName) + { + Assert.Equal(expectedName, BuildServerConnection.GetPipeNameCore(userName, isAdmin, basePipeName)); + } } } diff --git a/src/Compilers/Shared/BuildServerConnection.cs b/src/Compilers/Shared/BuildServerConnection.cs index 67efa1d6de0c70a5046e6e68263aa28811cbfc5a..496bb756f6d05db975ba0f33809eecb4631af83a 100644 --- a/src/Compilers/Shared/BuildServerConnection.cs +++ b/src/Compilers/Shared/BuildServerConnection.cs @@ -378,10 +378,16 @@ internal static bool IsCompilerServerSupported(string tempPath) } } - internal static bool TryCreateServerCore(string clientDir, string pipeName) + internal static (string processFilePath, string commandLineArguments, string toolFilePath) GetServerProcessInfo(string clientDir, string pipeName) { var serverPathWithoutExtension = Path.Combine(clientDir, "VBCSCompiler"); - var serverInfo = RuntimeHostInfo.GetProcessInfo(serverPathWithoutExtension, $"-pipename:{pipeName}"); + var commandLineArgs = $@"""-pipename:{pipeName}"""; + return RuntimeHostInfo.GetProcessInfo(serverPathWithoutExtension, commandLineArgs); + } + + internal static bool TryCreateServerCore(string clientDir, string pipeName) + { + var serverInfo = GetServerProcessInfo(clientDir, pipeName); if (!File.Exists(serverInfo.toolFilePath)) { @@ -480,7 +486,24 @@ internal static string GetPipeNameForPathOpt(string compilerExeDirectory) return null; } - return $"{userName}.{(isAdmin ? 'T' : 'F')}.{basePipeName}"; + return GetPipeNameCore(userName, isAdmin, basePipeName); + } + + internal static string GetPipeNameCore(string userName, bool isAdmin, string basePipeName) + { + var pipeName = $"{userName}.{(isAdmin ? 'T' : 'F')}.{basePipeName}"; + + // The pipe name is passed between processes as a command line argument as a + // quoted value. Unfortunately we can't use ProcessStartInfo.ArgumentList as + // we still target net472 (API only available on CoreClr + netstandard). To + // make the problem approachable we remove the troublesome characters. + // + // This does mean if two users on the same machine are building simultaneously + // and the user names differ only be a " or / and a _ then there will be a + // conflict. That seems rather obscure though. + return pipeName + .Replace('"', '_') + .Replace('\\', '_'); } /// diff --git a/src/Test/Utilities/Portable/Assert/ConditionalFactAttribute.cs b/src/Test/Utilities/Portable/Assert/ConditionalFactAttribute.cs index 5c7cfb72b0c8f1f02e782a73484f4ebe12dedfef..640694999fa6598f76c426d6c91a44d54fe294fd 100644 --- a/src/Test/Utilities/Portable/Assert/ConditionalFactAttribute.cs +++ b/src/Test/Utilities/Portable/Assert/ConditionalFactAttribute.cs @@ -225,6 +225,12 @@ public class ClrOnly : ExecutionCondition public override string SkipReason => "Test not supported on Mono"; } + public class CoreClrOnly : ExecutionCondition + { + public override bool ShouldSkip => !ExecutionConditionUtil.IsCoreClr; + public override string SkipReason => "Test only supported on CoreClr"; + } + public class DesktopOnly : ExecutionCondition { public override bool ShouldSkip => !ExecutionConditionUtil.IsDesktop;