Handle pipe name with whitespace properly

Author: ajaybhargavb

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;
         }
     }
 }

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<ServerResponse> 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))
             {

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.

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();
+            }
+        }
     }
 }

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);
-            }
-        }
     }
 }

test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs

@@ -71,7 +71,7 @@ internal Task<MSBuildResult> DotnetMSBuild(
 
             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.