Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always unquote target parameters #9452

Merged
1 change: 1 addition & 0 deletions documentation/wiki/ChangeWaves.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
- [AppDomain configuration is serialized without using BinFmt](https://github.com/dotnet/msbuild/pull/9320) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json`
- [Warning on serialization custom events by default in .NET framework](https://github.com/dotnet/msbuild/pull/9318)
- [Cache SDK resolver data process-wide](https://github.com/dotnet/msbuild/pull/9335)
- [Target parameters will be unquoted](https://github.com/dotnet/msbuild/pull/9452), meaning the ';' symbol in the parameter target name will always be treated as separator

### 17.8
- [[RAR] Don't do I/O on SDK-provided references](https://github.com/dotnet/msbuild/pull/8688)
Expand Down
8 changes: 8 additions & 0 deletions src/Framework/Traits.cs
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,14 @@ public bool EnableWarningOnCustomBuildEvent
}
}

public bool UnquoteTargetSwitchParameters
{
get
{
return ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10);
}
}

private bool? _isBinaryFormatterSerializationAllowed;
public bool IsBinaryFormatterSerializationAllowed
{
Expand Down
38 changes: 38 additions & 0 deletions src/MSBuild.UnitTests/CommandLineSwitches_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,44 @@ public void AppendParameterizedSwitchesTests2()
Assert.Equal("build", parameters[2]);
}

/// <summary>
/// Verifies that the Target property is unquoted and parsed properly.
/// This will remove the possibility to have the ';' in the target name.
/// </summary>
[Theory]
[InlineData("/t:Clean;Build", "\"Clean;Build\"")]
rainersigwald marked this conversation as resolved.
Show resolved Hide resolved
[InlineData("/t:Clean;Build", "Clean;Build")]
public void ParameterizedSwitchTargetQuotedTest(string commandLineArg, string switchParameters)
{
CommandLineSwitches switches = new CommandLineSwitches();
switches.SetParameterizedSwitch(CommandLineSwitches.ParameterizedSwitch.Target, commandLineArg, switchParameters, true, true, false);
switches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Target).ShouldBeTrue();

switches[CommandLineSwitches.ParameterizedSwitch.Target].Length.ShouldBe(2);
switches[CommandLineSwitches.ParameterizedSwitch.Target][0].ShouldBe("Clean");
switches[CommandLineSwitches.ParameterizedSwitch.Target][1].ShouldBe("Build");
switches.GetParameterizedSwitchCommandLineArg(CommandLineSwitches.ParameterizedSwitch.Target).ShouldBe(commandLineArg);
}

/// <summary>
/// Verifies that the parsing behavior of quoted target properties is not changed when ChangeWave configured.
/// </summary>
[Fact]
public void ParameterizedSwitchTargetQuotedChangeWaveTest()
{
using (TestEnvironment env = TestEnvironment.Create())
{
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", "17.10");

CommandLineSwitches switches = new CommandLineSwitches();
switches.SetParameterizedSwitch(CommandLineSwitches.ParameterizedSwitch.Target, "/t:Clean;Build", "\"Clean;Build\"", true, true, false);
switches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Target).ShouldBeTrue();

switches[CommandLineSwitches.ParameterizedSwitch.Target].Length.ShouldBe(1);
switches[CommandLineSwitches.ParameterizedSwitch.Target][0].ShouldBe("Clean;Build");
}
}

[Fact]
public void AppendParameterizedSwitchesTests3()
{
Expand Down
29 changes: 29 additions & 0 deletions src/MSBuild/CommandLineSwitches.cs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,11 @@ internal bool SetParameterizedSwitch(
}
else
{
if (IsMultipleAllowedSwitchParameterDueToUnquote(unquoteParameters, parameterizedSwitch))
{
switchParameters = QuotingUtilities.Unquote(switchParameters);
}

// store all the switch parameters
int emptyParameters;
_parameterizedSwitches[(int)parameterizedSwitch].parameters.AddRange(QuotingUtilities.SplitUnquoted(switchParameters, int.MaxValue, false /* discard empty parameters */, unquoteParameters, out emptyParameters, s_parameterSeparators));
Expand Down Expand Up @@ -651,6 +656,30 @@ internal string[][] GetFileLoggerParameters()
return groupedFileLoggerParameters;
}

/// <summary>
/// Checks if the provided multiple valued parametrized switch needs to be unquoted.
/// The method will return 'true' in case:
/// The changewave 17.10 is not set and
/// The parametrized switch is 'Target'
/// </summary>
private bool IsMultipleAllowedSwitchParameterDueToUnquote(bool unquoteParameter, ParameterizedSwitch parameterizedSwitch)
{
if (!unquoteParameter || !Traits.Instance.EscapeHatches.UnquoteTargetSwitchParameters)
{
return false;
}

// issue: https://github.com/dotnet/msbuild/issues/9442
// In order to align the parsing behaviour of Target property when MSBuild invoked from PowerShell or CMD,
// the target property value will be unquoted before processing further
if (parameterizedSwitch == ParameterizedSwitch.Target)
{
return true;
rokonec marked this conversation as resolved.
Show resolved Hide resolved
}

return false;
}

/// <summary>
/// If the specified parameterized switch is set, returns the array of parameters.
/// Otherwise, if the specified parameterless switch is set, returns an empty array.
Expand Down
Loading