Merge pull request #9153 from peterhuene/quote-msbuild-properties

Fix MSBuild invocation to quote property option values.
This commit is contained in:
Peter Huene 2018-04-27 16:52:02 -07:00 committed by GitHub
commit eaf5e95996
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 128 additions and 99 deletions

View file

@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>library</OutputType>
<TargetFramework>netcoreapp2.0</TargetFramework>
</PropertyGroup>
<Target Name="PrintWarning" BeforeTargets="CoreBuild">
<Warning Text="NoWarn => $(NoWarn)"/>
</Target>
</Project>

View file

@ -59,14 +59,10 @@ namespace Microsoft.DotNet.Cli.Utils
/// <returns></returns>
private static IEnumerable<string> EscapeArgArray(IEnumerable<string> args)
{
var escapedArgs = new List<string>();
foreach (var arg in args)
{
escapedArgs.Add(EscapeSingleArg(arg));
yield return EscapeSingleArg(arg);
}
return escapedArgs;
}
/// <summary>
@ -79,23 +75,19 @@ namespace Microsoft.DotNet.Cli.Utils
/// </summary>
/// <param name="args"></param>
/// <returns></returns>
private static IEnumerable<string> EscapeArgArrayForCmd(IEnumerable<string> arguments)
private static IEnumerable<string> EscapeArgArrayForCmd(IEnumerable<string> args)
{
var escapedArgs = new List<string>();
foreach (var arg in arguments)
foreach (var arg in args)
{
escapedArgs.Add(EscapeArgForCmd(arg));
yield return EscapeArgForCmd(arg);
}
}
return escapedArgs;
}
public static string EscapeSingleArg(string arg)
public static string EscapeSingleArg(string arg, bool forceQuotes = false)
{
var sb = new StringBuilder();
var needsQuotes = ShouldSurroundWithQuotes(arg);
var needsQuotes = forceQuotes || ShouldSurroundWithQuotes(arg);
var isQuoted = needsQuotes || IsSurroundedWithQuotes(arg);
if (needsQuotes) sb.Append("\"");

View file

@ -34,7 +34,7 @@ namespace Microsoft.DotNet.Cli.Utils
{
_forwardingApp = new ForwardingAppImplementation(
msbuildPath ?? GetMSBuildExePath(),
_msbuildRequiredParameters.Concat(argsToForward.Select(Escape)),
_msbuildRequiredParameters.Concat(argsToForward.Select(QuotePropertyValue)),
environmentVariables: _msbuildRequiredEnvironmentVariables);
}
@ -49,13 +49,6 @@ namespace Microsoft.DotNet.Cli.Utils
return GetProcessStartInfo().Execute();
}
private static string Escape(string arg) =>
// this is a workaround for https://github.com/Microsoft/msbuild/issues/1622
IsRestoreSources(arg) ?
arg.Replace(";", "%3B")
.Replace("://", ":%2F%2F") :
arg;
private static string GetMSBuildExePath()
{
return Path.Combine(
@ -82,12 +75,37 @@ namespace Microsoft.DotNet.Cli.Utils
return new Muxer().MuxerPath;
}
private static bool IsRestoreSources(string arg)
private static bool IsPropertyArgument(string arg)
{
return arg.StartsWith("/p:RestoreSources=", StringComparison.OrdinalIgnoreCase) ||
arg.StartsWith("/property:RestoreSources=", StringComparison.OrdinalIgnoreCase) ||
arg.StartsWith("-p:RestoreSources=", StringComparison.OrdinalIgnoreCase) ||
arg.StartsWith("-property:RestoreSources=", StringComparison.OrdinalIgnoreCase);
return
arg.StartsWith("/p:", StringComparison.OrdinalIgnoreCase) ||
arg.StartsWith("/property:", StringComparison.OrdinalIgnoreCase) ||
arg.StartsWith("-p:", StringComparison.OrdinalIgnoreCase) ||
arg.StartsWith("-property:", StringComparison.OrdinalIgnoreCase);
}
private static string QuotePropertyValue(string arg)
{
if (!IsPropertyArgument(arg))
{
return arg;
}
var parts = arg.Split(new[] { '=' }, 2);
if (parts.Length != 2)
{
return arg;
}
// Escaping `://` is a workaround for https://github.com/Microsoft/msbuild/issues/1622
// The issue is that MSBuild is collapsing multiple slashes to a single slash due to a bad regex.
var value = parts[1].Replace("://", ":%2f%2f");
if (ArgumentEscaper.IsSurroundedWithQuotes(value))
{
return $"{parts[0]}={value}";
}
return $"{parts[0]}={ArgumentEscaper.EscapeSingleArg(value, forceQuotes: true)}";
}
}
}

View file

@ -125,19 +125,6 @@ namespace Microsoft.DotNet.Tools.Test
return arg;
}
private static string[] GetSemiColonEscapedArgs(List<string> args)
{
int counter = 0;
string[] array = new string[args.Count];
foreach (string arg in args)
{
array[counter++] = GetSemiColonEscapedString(arg);
}
return array;
}
private static void UpdateRunSettingsArgumentsText()
{
DefaultHelpViewText.Synopsis.AdditionalArguments = " [[--] <RunSettings arguments>...]]";

View file

@ -13,21 +13,21 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
[Theory]
[InlineData(new string[] { }, "-target:Build")]
[InlineData(new string[] { "-o", "foo" }, "-target:Build -property:OutputPath=foo")]
[InlineData(new string[] { "-property:Verbosity=diag" }, "-target:Build -property:Verbosity=diag")]
[InlineData(new string[] { "--output", "foo" }, "-target:Build -property:OutputPath=foo")]
[InlineData(new string[] { "-o", "foo1 foo2" }, "-target:Build \"-property:OutputPath=foo1 foo2\"")]
[InlineData(new string[] { "-o", "foo" }, @"-target:Build -property:OutputPath=\""foo\""")]
[InlineData(new string[] { "-property:Verbosity=diag" }, @"-target:Build -property:Verbosity=\""diag\""")]
[InlineData(new string[] { "--output", "foo" }, @"-target:Build -property:OutputPath=\""foo\""")]
[InlineData(new string[] { "-o", "foo1 foo2" }, @"-target:Build ""-property:OutputPath=\""foo1 foo2\""""")]
[InlineData(new string[] { "--no-incremental" }, "-target:Rebuild")]
[InlineData(new string[] { "-r", "rid" }, "-target:Build -property:RuntimeIdentifier=rid")]
[InlineData(new string[] { "--runtime", "rid" }, "-target:Build -property:RuntimeIdentifier=rid")]
[InlineData(new string[] { "-c", "config" }, "-target:Build -property:Configuration=config")]
[InlineData(new string[] { "--configuration", "config" }, "-target:Build -property:Configuration=config")]
[InlineData(new string[] { "--version-suffix", "mysuffix" }, "-target:Build -property:VersionSuffix=mysuffix")]
[InlineData(new string[] { "--no-dependencies" }, "-target:Build -property:BuildProjectReferences=false")]
[InlineData(new string[] { "-r", "rid" }, @"-target:Build -property:RuntimeIdentifier=\""rid\""")]
[InlineData(new string[] { "--runtime", "rid" }, @"-target:Build -property:RuntimeIdentifier=\""rid\""")]
[InlineData(new string[] { "-c", "config" }, @"-target:Build -property:Configuration=\""config\""")]
[InlineData(new string[] { "--configuration", "config" }, @"-target:Build -property:Configuration=\""config\""")]
[InlineData(new string[] { "--version-suffix", "mysuffix" }, @"-target:Build -property:VersionSuffix=\""mysuffix\""")]
[InlineData(new string[] { "--no-dependencies" }, @"-target:Build -property:BuildProjectReferences=\""false\""")]
[InlineData(new string[] { "-v", "diag" }, "-target:Build -verbosity:diag")]
[InlineData(new string[] { "--verbosity", "diag" }, "-target:Build -verbosity:diag")]
[InlineData(new string[] { "--no-incremental", "-o", "myoutput", "-r", "myruntime", "-v", "diag", "/ArbitrarySwitchForMSBuild" },
"-target:Rebuild -property:OutputPath=myoutput -property:RuntimeIdentifier=myruntime -verbosity:diag /ArbitrarySwitchForMSBuild")]
@"-target:Rebuild -property:OutputPath=\""myoutput\"" -property:RuntimeIdentifier=\""myruntime\"" -verbosity:diag /ArbitrarySwitchForMSBuild")]
public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalArgs)
{
expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}");
@ -43,10 +43,10 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
}
[Theory]
[InlineData(new string[] { "-f", "tfm" }, "-target:Restore", "-target:Build -property:TargetFramework=tfm")]
[InlineData(new string[] { "-f", "tfm" }, "-target:Restore", @"-target:Build -property:TargetFramework=\""tfm\""")]
[InlineData(new string[] { "-o", "myoutput", "-f", "tfm", "-v", "diag", "/ArbitrarySwitchForMSBuild" },
"-target:Restore -property:OutputPath=myoutput -verbosity:diag /ArbitrarySwitchForMSBuild",
"-target:Build -property:OutputPath=myoutput -property:TargetFramework=tfm -verbosity:diag /ArbitrarySwitchForMSBuild")]
@"-target:Restore -property:OutputPath=\""myoutput\"" -verbosity:diag /ArbitrarySwitchForMSBuild",
@"-target:Build -property:OutputPath=\""myoutput\"" -property:TargetFramework=\""tfm\"" -verbosity:diag /ArbitrarySwitchForMSBuild")]
public void MsbuildInvocationIsCorrectForSeparateRestore(
string[] args,
string expectedAdditionalArgsForRestore,

View file

@ -22,12 +22,12 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
[Theory]
[InlineData(new string[] { }, "")]
[InlineData(new string[] { "-o", "<output>" }, "-property:OutputPath=<output>")]
[InlineData(new string[] { "--output", "<output>" }, "-property:OutputPath=<output>")]
[InlineData(new string[] { "-f", "<framework>" }, "-property:TargetFramework=<framework>")]
[InlineData(new string[] { "--framework", "<framework>" }, "-property:TargetFramework=<framework>")]
[InlineData(new string[] { "-c", "<configuration>" }, "-property:Configuration=<configuration>")]
[InlineData(new string[] { "--configuration", "<configuration>" }, "-property:Configuration=<configuration>")]
[InlineData(new string[] { "-o", "<output>" }, @"-property:OutputPath=\""<output>\""")]
[InlineData(new string[] { "--output", "<output>" }, @"-property:OutputPath=\""<output>\""")]
[InlineData(new string[] { "-f", "<framework>" }, @"-property:TargetFramework=\""<framework>\""")]
[InlineData(new string[] { "--framework", "<framework>" }, @"-property:TargetFramework=\""<framework>\""")]
[InlineData(new string[] { "-c", "<configuration>" }, @"-property:Configuration=\""<configuration>\""")]
[InlineData(new string[] { "--configuration", "<configuration>" }, @"-property:Configuration=\""<configuration>\""")]
[InlineData(new string[] { "-v", "diag" }, "-verbosity:diag")]
[InlineData(new string[] { "--verbosity", "diag" }, "-verbosity:diag")]
public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalArgs)

View file

@ -159,6 +159,26 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
$"The MSBuild logger argument should not be specified when telemetry is disabled.");
}
[Theory]
[InlineData("/p")]
[InlineData("/property")]
[InlineData("-p")]
[InlineData("-property")]
public void GivenAPropertyValueWithASemicolonItIsQuotedToMSBuild(string propertyOption)
{
var testInstance = TestAssets.Get("ProjectPrintsNoWarn")
.CreateInstance()
.WithSourceFiles();
new MSBuildCommand()
.WithWorkingDirectory(testInstance.Root)
.ExecuteWithCapturedOutput($@"/restore {propertyOption}:NoWarn=1234;5678;90\")
.Should()
.Pass()
.And
.HaveStdOutContaining(@"NoWarn => 1234;5678;90\");
}
private string[] GetArgsForMSBuild(Func<bool> sentinelExists)
{
Telemetry.Telemetry telemetry;

View file

@ -16,16 +16,16 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
[Theory]
[InlineData(new string[] { }, "")]
[InlineData(new string[] { "-o", "<packageoutputpath>" }, "-property:PackageOutputPath=<packageoutputpath>")]
[InlineData(new string[] { "--output", "<packageoutputpath>" }, "-property:PackageOutputPath=<packageoutputpath>")]
[InlineData(new string[] { "--no-build" }, "-property:NoBuild=true")]
[InlineData(new string[] { "--include-symbols" }, "-property:IncludeSymbols=true")]
[InlineData(new string[] { "--include-source" }, "-property:IncludeSource=true")]
[InlineData(new string[] { "-c", "<config>" }, "-property:Configuration=<config>")]
[InlineData(new string[] { "--configuration", "<config>" }, "-property:Configuration=<config>")]
[InlineData(new string[] { "--version-suffix", "<versionsuffix>" }, "-property:VersionSuffix=<versionsuffix>")]
[InlineData(new string[] { "-s" }, "-property:Serviceable=true")]
[InlineData(new string[] { "--serviceable" }, "-property:Serviceable=true")]
[InlineData(new string[] { "-o", "<packageoutputpath>" }, @"-property:PackageOutputPath=\""<packageoutputpath>\""")]
[InlineData(new string[] { "--output", "<packageoutputpath>" }, @"-property:PackageOutputPath=\""<packageoutputpath>\""")]
[InlineData(new string[] { "--no-build" }, @"-property:NoBuild=\""true\""")]
[InlineData(new string[] { "--include-symbols" }, @"-property:IncludeSymbols=\""true\""")]
[InlineData(new string[] { "--include-source" }, @"-property:IncludeSource=\""true\""")]
[InlineData(new string[] { "-c", "<config>" }, @"-property:Configuration=\""<config>\""")]
[InlineData(new string[] { "--configuration", "<config>" }, @"-property:Configuration=\""<config>\""")]
[InlineData(new string[] { "--version-suffix", "<versionsuffix>" }, @"-property:VersionSuffix=\""<versionsuffix>\""")]
[InlineData(new string[] { "-s" }, @"-property:Serviceable=\""true\""")]
[InlineData(new string[] { "--serviceable" }, @"-property:Serviceable=\""true\""")]
[InlineData(new string[] { "-v", "diag" }, "-verbosity:diag")]
[InlineData(new string[] { "--verbosity", "diag" }, "-verbosity:diag")]
[InlineData(new string[] { "<project>" }, "<project>")]

View file

@ -24,14 +24,14 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
[Theory]
[InlineData(new string[] { }, "")]
[InlineData(new string[] { "-r", "<rid>" }, "-property:RuntimeIdentifier=<rid>")]
[InlineData(new string[] { "--runtime", "<rid>" }, "-property:RuntimeIdentifier=<rid>")]
[InlineData(new string[] { "-o", "<publishdir>" }, "-property:PublishDir=<publishdir>")]
[InlineData(new string[] { "--output", "<publishdir>" }, "-property:PublishDir=<publishdir>")]
[InlineData(new string[] { "-c", "<config>" }, "-property:Configuration=<config>")]
[InlineData(new string[] { "--configuration", "<config>" }, "-property:Configuration=<config>")]
[InlineData(new string[] { "--version-suffix", "<versionsuffix>" }, "-property:VersionSuffix=<versionsuffix>")]
[InlineData(new string[] { "--manifest", "<manifestfiles>" }, "-property:TargetManifestFiles=<manifestfiles>")]
[InlineData(new string[] { "-r", "<rid>" }, @"-property:RuntimeIdentifier=\""<rid>\""")]
[InlineData(new string[] { "--runtime", "<rid>" }, @"-property:RuntimeIdentifier=\""<rid>\""")]
[InlineData(new string[] { "-o", "<publishdir>" }, @"-property:PublishDir=\""<publishdir>\""")]
[InlineData(new string[] { "--output", "<publishdir>" }, @"-property:PublishDir=\""<publishdir>\""")]
[InlineData(new string[] { "-c", "<config>" }, @"-property:Configuration=\""<config>\""")]
[InlineData(new string[] { "--configuration", "<config>" }, @"-property:Configuration=\""<config>\""")]
[InlineData(new string[] { "--version-suffix", "<versionsuffix>" }, @"-property:VersionSuffix=\""<versionsuffix>\""")]
[InlineData(new string[] { "--manifest", "<manifestfiles>" }, @"-property:TargetManifestFiles=\""<manifestfiles>\""")]
[InlineData(new string[] { "-v", "minimal" }, "-verbosity:minimal")]
[InlineData(new string[] { "--verbosity", "minimal" }, "-verbosity:minimal")]
[InlineData(new string[] { "<project>" }, "<project>")]
@ -53,8 +53,8 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
}
[Theory]
[InlineData(new string[] { "-f", "<tfm>" }, "-property:TargetFramework=<tfm>")]
[InlineData(new string[] { "--framework", "<tfm>" }, "-property:TargetFramework=<tfm>")]
[InlineData(new string[] { "-f", "<tfm>" }, @"-property:TargetFramework=\""<tfm>\""")]
[InlineData(new string[] { "--framework", "<tfm>" }, @"-property:TargetFramework=\""<tfm>\""")]
public void MsbuildInvocationIsCorrectForSeparateRestore(string[] args, string expectedAdditionalArgs)
{
expectedAdditionalArgs = (string.IsNullOrEmpty(expectedAdditionalArgs) ? "" : $" {expectedAdditionalArgs}");
@ -86,7 +86,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
command.GetProcessStartInfo()
.Arguments
.Should()
.Be($"{ExpectedPrefix} -target:Publish -property:NoBuild=true");
.Be($"{ExpectedPrefix} -target:Publish -property:NoBuild=\\\"true\\\"");
}
[Theory]

View file

@ -15,18 +15,18 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
[Theory]
[InlineData(new string[] { }, "")]
[InlineData(new string[] { "-s", "<source>" }, "-property:RestoreSources=<source>")]
[InlineData(new string[] { "--source", "<source>" }, "-property:RestoreSources=<source>")]
[InlineData(new string[] { "-s", "<source0>", "-s", "<source1>" }, "-property:RestoreSources=<source0>%3B<source1>")]
[InlineData(new string[] { "-r", "<runtime>" }, "-property:RuntimeIdentifiers=<runtime>")]
[InlineData(new string[] { "--runtime", "<runtime>" }, "-property:RuntimeIdentifiers=<runtime>")]
[InlineData(new string[] { "-r", "<runtime0>", "-r", "<runtime1>" }, "-property:RuntimeIdentifiers=<runtime0>%3B<runtime1>")]
[InlineData(new string[] { "--packages", "<packages>" }, "-property:RestorePackagesPath=<packages>")]
[InlineData(new string[] { "--disable-parallel" }, "-property:RestoreDisableParallel=true")]
[InlineData(new string[] { "--configfile", "<config>" }, "-property:RestoreConfigFile=<config>")]
[InlineData(new string[] { "--no-cache" }, "-property:RestoreNoCache=true")]
[InlineData(new string[] { "--ignore-failed-sources" }, "-property:RestoreIgnoreFailedSources=true")]
[InlineData(new string[] { "--no-dependencies" }, "-property:RestoreRecursive=false")]
[InlineData(new string[] { "-s", "<source>" }, @"-property:RestoreSources=\""<source>\""")]
[InlineData(new string[] { "--source", "<source>" }, @"-property:RestoreSources=\""<source>\""")]
[InlineData(new string[] { "-s", "<source0>", "-s", "<source1>" }, @"-property:RestoreSources=\""<source0>%3B<source1>\""")]
[InlineData(new string[] { "-r", "<runtime>" }, @"-property:RuntimeIdentifiers=\""<runtime>\""")]
[InlineData(new string[] { "--runtime", "<runtime>" }, @"-property:RuntimeIdentifiers=\""<runtime>\""")]
[InlineData(new string[] { "-r", "<runtime0>", "-r", "<runtime1>" }, @"-property:RuntimeIdentifiers=\""<runtime0>%3B<runtime1>\""")]
[InlineData(new string[] { "--packages", "<packages>" }, @"-property:RestorePackagesPath=\""<packages>\""")]
[InlineData(new string[] { "--disable-parallel" }, @"-property:RestoreDisableParallel=\""true\""")]
[InlineData(new string[] { "--configfile", "<config>" }, @"-property:RestoreConfigFile=\""<config>\""")]
[InlineData(new string[] { "--no-cache" }, @"-property:RestoreNoCache=\""true\""")]
[InlineData(new string[] { "--ignore-failed-sources" }, @"-property:RestoreIgnoreFailedSources=\""true\""")]
[InlineData(new string[] { "--no-dependencies" }, @"-property:RestoreRecursive=\""false\""")]
[InlineData(new string[] { "-v", "minimal" }, @"-verbosity:minimal")]
[InlineData(new string[] { "--verbosity", "minimal" }, @"-verbosity:minimal")]
public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalArgs)

View file

@ -26,11 +26,11 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
}
[Theory]
[InlineData(new string[] { "-f", "<tfm>" }, @"-property:TargetFramework=<tfm>")]
[InlineData(new string[] { "--framework", "<tfm>" }, @"-property:TargetFramework=<tfm>")]
[InlineData(new string[] { "-r", "<rid>" }, @"-property:RuntimeIdentifier=<rid>")]
[InlineData(new string[] { "--runtime", "<rid>" }, @"-property:RuntimeIdentifier=<rid>")]
[InlineData(new string[] { "--manifest", "one.xml", "--manifest", "two.xml", "--manifest", "three.xml" }, @"-property:AdditionalProjects=one.xml%3Btwo.xml%3Bthree.xml")]
[InlineData(new string[] { "-f", "<tfm>" }, @"-property:TargetFramework=\""<tfm>\""")]
[InlineData(new string[] { "--framework", "<tfm>" }, @"-property:TargetFramework=\""<tfm>\""")]
[InlineData(new string[] { "-r", "<rid>" }, @"-property:RuntimeIdentifier=\""<rid>\""")]
[InlineData(new string[] { "--runtime", "<rid>" }, @"-property:RuntimeIdentifier=\""<rid>\""")]
[InlineData(new string[] { "--manifest", "one.xml", "--manifest", "two.xml", "--manifest", "three.xml" }, @"-property:AdditionalProjects=\""one.xml%3Btwo.xml%3Bthree.xml\""")]
public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalArgs)
{
args = ArgsPrefix.Concat(args).ToArray();
@ -51,7 +51,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
var msbuildPath = "<msbuildpath>";
StoreCommand.FromArgs(args, msbuildPath)
.GetProcessStartInfo().Arguments.Should().Be($"{ExpectedPrefix} -property:ComposeDir={Path.GetFullPath(path)}");
.GetProcessStartInfo().Arguments.Should().Be($"{ExpectedPrefix} -property:ComposeDir=\\\"{Path.GetFullPath(path)}\\\"");
}
}
}