Skip to content
This repository has been archived by the owner on Dec 19, 2018. It is now read-only.

Handle pipe names with whitespace properly #2409

Merged
merged 1 commit into from
Jun 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/Microsoft.AspNetCore.Razor.Tools/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -16,17 +17,34 @@ public static int Main(string[] args)
var cancel = new CancellationTokenSource();
Console.CancelKeyPress += (sender, e) => { cancel.Cancel(); };

var outputWriter = new StringWriter();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the StringWriter is disposable, so should be wrapped with a using statement. However, this is in the main method, which means when it's unused, the process will exit and all the resources will be cleaned up anyway.

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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.CommandLineUtils;

namespace Microsoft.AspNetCore.Razor.Tools
{
Expand Down Expand Up @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change lets me specify absolute path for directories here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

}

// Open allowing sharing. We allow multiple processes to log to the same file, so we use share mode to allow that.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down Expand Up @@ -54,18 +58,5 @@ public void Dispose()
}
}
}

private static string RecursiveFind(string path, string start)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused

{
var test = Path.Combine(start, path);
if (File.Exists(test))
{
return start;
}
else
{
return RecursiveFind(path, new DirectoryInfo(start).Parent.FullName);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ internal Task<MSBuildResult> DotnetMSBuild(

if (!suppressBuildServer)
{
buildArgumentList.Add($"/p:_RazorBuildServerPipeName={buildServerPipeName ?? BuildServer.PipeName}");
buildArgumentList.Add($@"/p:_RazorBuildServerPipeName=""{buildServerPipeName ?? BuildServer.PipeName}""");
}

if (!string.IsNullOrEmpty(target))
Expand Down