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, this will remove the possibility to have ';' in target name ](https://github.com/dotnet/msbuild/pull/9452)
rokonec marked this conversation as resolved.
Show resolved Hide resolved

### 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 UnquoteTragetSwitchParameters
f-alizada marked this conversation as resolved.
Show resolved Hide resolved
{
get
{
return ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10);
}
}

private bool? _isBinaryFormatterSerializationAllowed;
public bool IsBinaryFormatterSerializationAllowed
{
Expand Down
37 changes: 37 additions & 0 deletions src/MSBuild.UnitTests/CommandLineSwitches_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,43 @@ 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);
Assert.True(switches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Target));

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

/// <summary>
/// Verifies that the parsing behaviour of quoted target properties is not change when ChangeWave configured.
f-alizada marked this conversation as resolved.
Show resolved Hide resolved
/// </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);
Assert.True(switches.IsParameterizedSwitchSet(CommandLineSwitches.ParameterizedSwitch.Target));

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

[Fact]
public void AppendParameterizedSwitchesTests3()
{
Expand Down
25 changes: 25 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 (IsMultipleAllowedSwithParameterDueToUnquote(unquoteParameters, parameterizedSwitch))
JanKrivanek marked this conversation as resolved.
Show resolved Hide resolved
{
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,26 @@ internal string[][] GetFileLoggerParameters()
return groupedFileLoggerParameters;
}

/// <summary>
/// Checks if the provided 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 IsMultipleAllowedSwithParameterDueToUnquote(bool unquoteParameter, ParameterizedSwitch parameterizedSwitch)
JanKrivanek marked this conversation as resolved.
Show resolved Hide resolved
{
if (!unquoteParameter || !Traits.Instance.EscapeHatches.UnquoteTragetSwitchParameters)
{
return false;
}
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