Sdk: ResolvePackageFileConflicts fails when paths have New Line characters

[natidea]

NewLine characters in HintPaths are causing an exception in ResolvePackageFileConflicts for projects that previously worked. E.g.

<Reference Include="System.Collections.Immutable, Version=1.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
      <HintPath>
      ..\..\packages\System.Collections.Immutable.1.3.1\lib\portable-net45+win8+wp8+wpa81\System.Collections.Immutable.dll
      </HintPath>
</Reference>

StackTrace:

The "ResolvePackageFileConflicts" task failed unexpectedly.
System.ArgumentException: Illegal characters in path.
   at System.IO.Path.CheckInvalidPathChars(String path, Boolean checkAdditional)
   at System.IO.Path.GetFileName(String path)
   at Microsoft.NET.Build.Tasks.ItemUtilities.GetTargetPath(ITaskItem item)
   at Microsoft.NET.Build.Tasks.ItemUtilities.GetReferenceTargetPath(ITaskItem item)
   at Microsoft.NET.Build.Tasks.ConflictResolution.ResolvePackageFileConflicts.<>c.<ExecuteCore>b__35_1(ConflictItem ci)
   at Microsoft.NET.Build.Tasks.ConflictResolution.ConflictResolver`1.ResolveConflicts(IEnumerable`1 conflictItems, Func`2 getItemKey, Action`1 foundConflict, Boolean commitWinner, Action`1 unresolvedConflict)
   at Microsoft.NET.Build.Tasks.ConflictResolution.ResolvePackageFileConflicts.ExecuteCore()
   at Microsoft.NET.Build.Tasks.TaskBase.Execute()
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() Services (Services\Services)            

Reports:

• VSFeedback | 484081
• StackOverflow: https://stackoverflow.com/questions/45796041/visual-studio-illegal-characters-in-path
• VSFeedback | 482572

/cc @dsplaisted

[wli3]

@rainersigwald i think MsBuild simply see it as there is a new line in the middle. Is that intended? I can see the benefit of not using 
 all the time

[ceddlyburge]

Projects like this used to work and now don't, so I think it will be causing people pain.

If it is desired behaviour, then maybe an improved error message would be a decent compromise?

This could detail the reference that has the problem, which would be a big help while fixing up.

[natidea]

@wli3 you could just add a Trim() to GetTargetPath

[livarcocc]

@wli3 this seems simple enough. I will mark it for 2.1.3xx, any chance you can get this in in between your other stuff?

[wli3]

I cannot repo it with 2.1.4xx branch master.

My test csproj is the following and I just run _dotnet build_ against it. @dsplaisted do you know anything else I need to repo this?

<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Library</OutputType>
    <TargetFramework>net462</TargetFramework>
  </PropertyGroup>
  <ItemGroup />
  <ItemGroup>
    <Reference Include="System.Collections.Immutable">
      <HintPath>
      ..\packages\System.Collections.Immutable.1.3.1\lib\portable-net45+win8+wp8+wpa81\System.Collections.Immutable.dll
      </HintPath>
    </Reference>
  </ItemGroup>
</Project>

tree /f

C:.
│   NETFrameworkLibrary.cs
│   NETFrameworkLibrary.csproj
│
├───bin
│   └───Debug
│       └───net462
│               NETFrameworkLibrary.dll
│               NETFrameworkLibrary.pdb
│               System.Collections.Immutable.dll
│               System.Collections.Immutable.xml
│
├───obj
│   │   NETFrameworkLibrary.csproj.nuget.cache
│   │   NETFrameworkLibrary.csproj.nuget.g.props
│   │   NETFrameworkLibrary.csproj.nuget.g.targets
│   │   project.assets.json
│   │
│   └───Debug
│       └───net462
│               NETFrameworkLibrary.AssemblyInfo.cs
│               NETFrameworkLibrary.AssemblyInfoInputs.cache
│               NETFrameworkLibrary.assets.cache
│               NETFrameworkLibrary.csproj.CopyComplete
│               NETFrameworkLibrary.csproj.CoreCompileInputs.cache
│               NETFrameworkLibrary.csproj.FileListAbsolute.txt
│               NETFrameworkLibrary.csprojAssemblyReference.cache
│               NETFrameworkLibrary.dll
│               NETFrameworkLibrary.pdb
│
└───packages
    └───system.collections.immutable.1.3.1
        │   dotnet_library_license.txt
        │   System.Collections.Immutable.nuspec
        │   ThirdPartyNotices.txt
        │   [Content_Types].xml
        │
        ├───lib
        │   ├───netstandard1.0
        │   │       System.Collections.Immutable.dll
        │   │       System.Collections.Immutable.xml
        │   │
        │   └───portable-net45%2Bwin8%2Bwp8%2Bwpa81
        │           System.Collections.Immutable.dll
        │           System.Collections.Immutable.xml
        │
        ├───package
        │   └───services
        │       └───metadata
        │           └───core-properties
        │                   29d09ebac0cf450d9158528bb737fd76.psmdcp
        │
        └───_rels
                .rels

[dsplaisted]

@wli3 I'm not sure, try changing the TargetFramework to netcoreapp2.0 and seeing what happens.

[wli3]

I can repo with msbuid /t:build

[wli3]

Further root cause

run the same code on fullframework and dotnet core
c# Path.GetFileName("\r\n net461\\lib\\System.Net.Http.dll\r\n");
fullframework throws but core is good

[wli3]

core impl is here https://github.com/dotnet/corefx/blob/master/src/Common/src/CoreLib/System/IO/Path.cs#L167

[wli3]

Core GetFileName don't check Path.GetInvalidPathChars()

[wli3]

https://github.com/dotnet/cli/issues/4523 similar to Nuget we should trim our own