From e7db3f840b119e2fe08550d1e568b51dd0256eda Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Sat, 16 Jun 2018 01:05:59 +0530 Subject: [PATCH] Handle pipe name with whitespace properly --- .../Program.cs | 24 +++++++++++++-- .../ServerProtocol/ServerConnection.cs | 15 +++++++--- .../ServerProtocol/ServerLogger.cs | 2 +- .../BuildServerIntegrationTest.cs | 29 +++++++++++++++++++ .../BuildServerTestFixture.cs | 21 ++++---------- .../MSBuildIntegrationTestBase.cs | 2 +- 6 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.AspNetCore.Razor.Tools/Program.cs b/src/Microsoft.AspNetCore.Razor.Tools/Program.cs index 27807cd23e..8d9c5518c5 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/Program.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/Program.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO; using System.Threading; using Microsoft.CodeAnalysis; @@ -16,17 +17,34 @@ namespace Microsoft.AspNetCore.Razor.Tools var cancel = new CancellationTokenSource(); Console.CancelKeyPress += (sender, e) => { cancel.Cancel(); }; + var outputWriter = new StringWriter(); + var errorWriter = new StringWriter(); + // Prevent shadow copying. var loader = new DefaultExtensionAssemblyLoader(baseDirectory: null); - var checker = new DefaultExtensionDependencyChecker(loader, Console.Out, Console.Error); + var checker = new DefaultExtensionDependencyChecker(loader, outputWriter, errorWriter); var application = new Application( cancel.Token, loader, checker, - (path, properties) => MetadataReference.CreateFromFile(path, properties)); + (path, properties) => MetadataReference.CreateFromFile(path, properties), + outputWriter, + errorWriter); - return application.Execute(args); + var result = application.Execute(args); + + var output = outputWriter.ToString(); + var error = errorWriter.ToString(); + + outputWriter.Dispose(); + errorWriter.Dispose(); + + // This will no-op if server logging is not enabled. + ServerLogger.Log(output); + ServerLogger.Log(error); + + return result; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs index 2583d1e0dc..d9fb8d4b11 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs @@ -9,6 +9,7 @@ using System.Runtime.InteropServices; using System.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.CommandLineUtils; namespace Microsoft.AspNetCore.Razor.Tools { @@ -283,14 +284,20 @@ namespace Microsoft.AspNetCore.Razor.Tools // Internal for testing. internal static bool TryCreateServerCore(string clientDir, string pipeName, out int? processId, bool debug = false) { - string expectedPath; - string processArguments; processId = null; // The server should be in the same directory as the client var expectedCompilerPath = Path.Combine(clientDir, ServerName); - expectedPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") ?? "dotnet"; - processArguments = $@"""{expectedCompilerPath}"" {(debug ? "--debug" : "")} server -p {pipeName}"; + var expectedPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") ?? "dotnet"; + var argumentList = new string[] + { + expectedCompilerPath, + debug ? "--debug" : "", + "server", + "-p", + pipeName + }; + var processArguments = ArgumentEscaper.EscapeAndConcatenate(argumentList); if (!File.Exists(expectedCompilerPath)) { diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerLogger.cs b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerLogger.cs index be2e934160..2e38fc60c6 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerLogger.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerLogger.cs @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Razor.Tools // Otherwise, assume that the environment variable specifies the name of the log file. if (Directory.Exists(loggingFileName)) { - loggingFileName = Path.Combine(loggingFileName, $"server.{loggingFileName}.{GetCurrentProcessId()}.log"); + loggingFileName = Path.Combine(loggingFileName, $"razorserver.{GetCurrentProcessId()}.log"); } // Open allowing sharing. We allow multiple processes to log to the same file, so we use share mode to allow that. diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs index 60076a7a67..e570b9ee93 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs @@ -164,5 +164,34 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests Assert.Equal(0, exitCode); Assert.Contains("shut down completed", output.ToString()); } + + [Fact] + [InitializeTestProject("SimpleMvc")] + public async Task Build_WithWhiteSpaceInPipeName_BuildsSuccessfully() + { + // Start the server + var pipeName = "pipe with whitespace"; + var fixture = new BuildServerTestFixture(pipeName); + + try + { + // Run a build + var result = await DotnetMSBuild( + "Build", + "/p:_RazorForceBuildServer=true", + buildServerPipeName: pipeName); + + Assert.BuildPassed(result); + Assert.FileExists(result, OutputPath, "SimpleMvc.dll"); + Assert.FileExists(result, OutputPath, "SimpleMvc.pdb"); + Assert.FileExists(result, OutputPath, "SimpleMvc.Views.dll"); + Assert.FileExists(result, OutputPath, "SimpleMvc.Views.pdb"); + } + finally + { + // Shutdown the server + fixture.Dispose(); + } + } } } diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerTestFixture.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerTestFixture.cs index ea18e6cfa0..5c1a17e5c7 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerTestFixture.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerTestFixture.cs @@ -14,9 +14,13 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests { private static readonly TimeSpan _defaultShutdownTimeout = TimeSpan.FromSeconds(60); - public BuildServerTestFixture() + public BuildServerTestFixture() : this(Guid.NewGuid().ToString()) { - PipeName = Guid.NewGuid().ToString(); + } + + internal BuildServerTestFixture(string pipeName) + { + PipeName = pipeName; if (!ServerConnection.TryCreateServerCore(Environment.CurrentDirectory, PipeName, out var processId)) { @@ -54,18 +58,5 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests } } } - - private static string RecursiveFind(string path, string start) - { - var test = Path.Combine(start, path); - if (File.Exists(test)) - { - return start; - } - else - { - return RecursiveFind(path, new DirectoryInfo(start).Parent.FullName); - } - } } } diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs index c9765901cd..4cde21a898 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs @@ -71,7 +71,7 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests if (!suppressBuildServer) { - buildArgumentList.Add($"/p:_RazorBuildServerPipeName={buildServerPipeName ?? BuildServer.PipeName}"); + buildArgumentList.Add($@"/p:_RazorBuildServerPipeName=""{buildServerPipeName ?? BuildServer.PipeName}"""); // The build server will not be used in netcoreapp2.0 because PipeOptions.CurrentUserOnly is not available. // But we still want to make sure to run the tests on the server. So suppress that check.