MSBuild property escaping is invalid

[vbfox]

Description

Essentially the fix of #2112 has been a too broad I think. Properties passed to MSBuild command line need ; to be escaped as it's a list of properties separated by ; but after e7e9c10 it's also escaping other things like \\ but MSBuild doesn't support that at all ending up with an invalid command line when a property contains a path for example.

Repro steps + Expected & Actual behavior

Pass any path as a property to msbuild, for example to call Wix trying to generate /p:WixTargetsPath=c:\Code\packages\build\WiX\tools\wix.targets by you'll get /p:WixTargetsPath=c:%5CCode%5Cpackages%5Cbuild%5CWiX%5Ctools%5Cwix.targets

Known workarounds

Copy paste MSBuild.fs in your project and keep only ; being escaped in properties

Related information

• Version: 5.16.1
• Snark: MSBuild doing the logical thing here and unescaping all characters % escaped would be too logical it seem ☹

[matthid]

I don’t get it. As far as I can see we only escape the official msbuild special characters. And ‘\’ is not one of them?
We even added an integration test which contains the ‘\’ character.

• Where are we escaping ‘\’?
• Why is the integration test green?

[matthid]

Ah we escape ‘\’ in master and I think the reason is in fact the test.
Feel free to remove the escaping and see the test go red...

[vbfox]

Yeah I think i'll find a way to test every character somehow (at least ascii range) and see what sticks

[vbfox]

So after tests with something like that :

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" DefaultTargets="Build">
    <UsingTask TaskName="WixAssignCulture" AssemblyFile="$(WixTasksPath)" />
    <Target Name="Build">
        <Message Text="__BEFORE__$(Arg)__AFTER__" />
        <WixAssignCulture />
    </Target>
</Project>

My conclusion is that msbuild argument handling is Fucked Up Beyond All Repair.

Escaping arguments with % mostly works but some places (like AssemblyFile in my sample) don't work, they see the escape codes and don't interpret them.

After tests the minimum for the parameters to escape are ; and , (I love the fact that , isn't documented as needing escape but still needs it) so i'd suggest we limit the escape to theses 2. It mean than dll names with them for example can't be used but it's not that big of a loss.

Fun thing is that the /p:Foo="Biz, Baz and Boom" form works well even in the face of ; and , so we should be able to use it too if we wanted even if it's weird.
The thing I wonder about it is that it use/abuse the fact that quotes can start block everywhere in MSVCRT parsing rules and that can't possibly work the same on unix where it's the shell that does the argument splitting...

Anyway I'll PR limiting the list to only the 2 necessary ones it's good enough for now.

[matthid]

Yes I'm open for suggestions/prs maybe we need to open an issue in msbuild?

[vbfox]

Tried a few things but I don't see any way forward dotnet/msbuild#885 (comment) say this bug is by design and there is no way to avoid it hat is fully cross platform and future-proof.

2 solutions for others:

• $([MSBuild]::Unescape(...)) in the msbuild project
• Use forward slashes when possible