From 17e3aa8ee40ea357eeda81b2c51b23c178a33bdd 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 27807cd23..8d9c5518c 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 @@ public static int Main(string[] args) 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); + + 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 application.Execute(args); + 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 2583d1e0d..d9fb8d4b1 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.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.CommandLineUtils; namespace Microsoft.AspNetCore.Razor.Tools { @@ -283,14 +284,20 @@ private static async Task TryProcessRequest( // 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 be2e93416..2e38fc60c 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerLogger.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerLogger.cs @@ -46,7 +46,7 @@ static ServerLogger() // 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 60076a7a6..e570b9ee9 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 @@ public async Task ManualServerShutdown_NoPipeName_ShutsDownServer() 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 ea18e6cfa..5c1a17e5c 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 @@ public class BuildServerTestFixture : IDisposable { 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 @@ public void Dispose() } } } - - 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 9b225130e..1d44f27d4 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs @@ -82,7 +82,7 @@ internal Task DotnetMSBuild( if (!suppressBuildServer) { - buildArgumentList.Add($"/p:_RazorBuildServerPipeName={buildServerPipeName ?? BuildServer.PipeName}"); + buildArgumentList.Add($@"/p:_RazorBuildServerPipeName=""{buildServerPipeName ?? BuildServer.PipeName}"""); } if (!string.IsNullOrEmpty(target))