Static Graph RestoreEx: NuGet.Build.Tasks.Console.exe crash with wildcards and UNC paths

[KirillOsenkov]

I have a situation where normal restore works fine, but switching to static graph restore crashes MSBuild with:

The UNC path should be of the form \\server\share.

C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error : The UNC path should be of the form \\server\share. [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at System.IO.LongPathHelper.Normalize(String path, UInt32 maxPathLength, Boolean checkInvalidCharacters, Boolean expandShortPaths) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at System.IO.Path.NormalizePath(String path, Boolean fullCheck, Int32 maxPathLength, Boolean expandShortPaths) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at System.IO.Path.GetFullPathInternal(String path) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at Microsoft.Build.Shared.FileUtilities.GetFullPath(String path) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at Microsoft.Build.Shared.FileUtilities.NormalizePath(String path) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at Microsoft.Build.Shared.FileUtilities.GetFullPath(String fileSpec, String currentDirectory) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at Microsoft.Build.Evaluation.LazyItemEvaluator`4.LazyItemList.ProcessNonWildCardItemUpdates(Dictionary`2 itemsWithNoWildcards, Builder items) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at Microsoft.Build.Evaluation.LazyItemEvaluator`4.LazyItemList.ComputeItems(LazyItemList lazyItemList, ImmutableHashSet`1 globsToIgnore) [C:\Ide\Ide.sln]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.RestoreEx.targets(19,5): error :    at Microsoft.Build.Evaluation.LazyItemEvaluator`4.LazyItemList.GetItemData(ImmutableHashSet`1 globsToIgnore) [C:\Ide\Ide.sln]

The specific process that crashes is NuGet.Build.Tasks.Console.exe with the following stack:

    mscorlib    LongPathHelper.Normalize Line 167
    mscorlib    Path.NewNormalizePath Line 503
    mscorlib    Path.NormalizePath Line 427
    mscorlib    Path.NormalizePath Line 391
    mscorlib    Path.NormalizePath Line 376
    mscorlib    Path.GetFullPathInternal Line 366
    mscorlib    Path.GetFullPath Line 330
    Microsoft.Build FileUtilities.GetFullPath Line 500
    Microsoft.Build FileUtilities.NormalizePath Line 465
    Microsoft.Build FileUtilities.GetFullPath Line 762
    Microsoft.Build LazyItemEvaluator`4.LazyItemList.ProcessNonWildCardItemUpdates Line 447
    Microsoft.Build LazyItemEvaluator`4.LazyItemList.ComputeItems Line 418
    Microsoft.Build LazyItemEvaluator`4.LazyItemList.GetItemData Line 306
    Microsoft.Build LazyItemEvaluator`4.GetAllItemsDeferred Line 506
    System.Core Enumerable.SelectManyIterator
    System.Core Buffer`1..ctor
    System.Core OrderedEnumerable`1.GetEnumerator
    Microsoft.Build Evaluator`4.Evaluate Line 686
    Microsoft.Build Evaluator`4.Evaluate Line 341
    Microsoft.Build ProjectInstance.Initialize Line 2804
    Microsoft.Build ProjectInstance..ctor Line 285
    Microsoft.Build ProjectInstance.FromFile Line 757
    NuGet.Build.Tasks.Console   MSBuildStaticGraphRestore.LoadProjects
    Microsoft.Build GraphBuilder.ParseProject Line 536
    Microsoft.Build GraphBuilder.SubmitProjectForParsing Line 576
    mscorlib    Lazy`1.CreateValue Line 437
    mscorlib    Lazy`1.LazyInitValue Line 388
    mscorlib    Lazy`1.get_Value Line 339
    Microsoft.Build ParallelWorkSet`2.ExecuteWorkItem Line 194
    Microsoft.Build ParallelWorkSet`2.<CreateProcessorItemTask>b__17_0 Line 169
    mscorlib    AsyncMethodBuilderCore.MoveNextRunner.InvokeMoveNext Line 1090
    mscorlib    ExecutionContext.RunInternal Line 981
    mscorlib    ExecutionContext.Run Line 928
    mscorlib    AsyncMethodBuilderCore.MoveNextRunner.Run Line 1071
    mscorlib    AwaitTaskContinuation.RunOrScheduleAction Line 811
    mscorlib    Task.FinishContinuations Line 3617
    mscorlib    Task.FinishStageThree Line 2363
    mscorlib    Task`1.TrySetResult Line 490
    mscorlib    SemaphoreSlim.TaskNode.ExecuteWorkItem Line 95
    mscorlib    ThreadPoolWorkQueue.Dispatch Line 820
    mscorlib    _ThreadPoolWaitCallback.PerformWaitCallback Line 1161

The actual problem is this call to Path.GetFullPath: https://github.com/dotnet/msbuild/blob/c36a54ed3308d1516ffe1a86b9086c42e4ca996f/src/Build/Evaluation/LazyItemEvaluator.cs#L447

It is being passed the value of \\csc.*, which results in GetFullPath throwing an ArgumentException: The UNC path should be of the form \\server\share.

Unfortunately I've failed to isolate a standalone repro for this exact scenario. I do have an internal repo at commit bd36c7032f0950eeca634d7aa34fcdf441e89ed9 that's easy to reproduce this on. Just contact me on Teams and we can take a look together.

However I did build a repro that hits a very similar case, with slightly different behavior. It no longer crashes, but fails the build with this error:

"C:\temp\staticGraphFail\staticGraphFail.sln" (Restore target) (1) ->
(Restore target) ->
  C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.DefaultItems.targets(101,13): error MSB4248: Cannot expand metadata in expression "$([MSBuild]::
ValueOrDefault('%(FullPath)', '').StartsWith($([MSBuild]::EnsureTrailingSlash($(MSBuildProjectDirectory)))))". The item metadata "%(FullPath)" cannot be applied to the path "\\csc.*". The UNC path
 should be of the form \\server\share.  C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.DefaultItems.targets [C:\temp\staticGraphFail\staticGraphF
ail.sln]

To repro, create this fail.csproj file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <RestoreUseStaticGraphEvaluation>true</RestoreUseStaticGraphEvaluation>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="13.0.3" ExcludeAssets="all" GeneratePathProperty="true"/>
  </ItemGroup>

  <PropertyGroup>
    <A>$(PkgNewtonsoft_Json)\</A>
  </PropertyGroup>

  <ItemGroup>
    <None Include="$(A)\csc.*" />
  </ItemGroup>

</Project>

and build with msbuild /t:Restore fail.csproj.

In this repro, the crash happens here: https://github.com/dotnet/msbuild/blob/c36a54ed3308d1516ffe1a86b9086c42e4ca996f/src/Build/Evaluation/LazyItemEvaluator.cs#L429

There's a good try/catch around this callstack, so it doesn't bring down the process, because this catch block catches it: https://github.com/dotnet/msbuild/blob/c36a54ed3308d1516ffe1a86b9086c42e4ca996f/src/Shared/Modifiers.cs#L602

There's another catch block here: https://github.com/dotnet/msbuild/blob/c36a54ed3308d1516ffe1a86b9086c42e4ca996f/src/Build/Evaluation/Expander.cs#L982

However in my actual case, there isn't such a good catch block around this codepath: https://github.com/dotnet/msbuild/blob/c36a54ed3308d1516ffe1a86b9086c42e4ca996f/src/Build/Evaluation/LazyItemEvaluator.cs#L418

I wasn't able to recreate a standalone repro which takes that path (line 418) instead of the regular path (line 429). Ping me on Teams for a real-life repro of the actual issue.

I suspect the problem is that wildcards are not expected to be in the item spec \\csc.* but they are, and the item spec starts with \\ because the $(Pkg*) property evaluates to an empty string at the time of restore eval, and then Path.GetFullPath() crashes on it, but there's no good catch block to catch it around line 418, so it is more fatal.

[KirillOsenkov]

Rainer thinks this is an MSBuild, rather than NuGet, bug.

It's happening in static graph restore because that sets MSBuild config to disable glob expansion, and thus passes the * into the normalize method. That should have the same behavior as elsewhere.

[KirillOsenkov]

I GOT THE MINIMAL REPRO!!!!

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <RestoreUseStaticGraphEvaluation>true</RestoreUseStaticGraphEvaluation>
  </PropertyGroup>

  <PropertyGroup>
    <A>$(B)\</A>
  </PropertyGroup>

  <ItemGroup>
    <None Include="$(A)\csc.*" />
    <None Update="2.txt">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </None>
  </ItemGroup>

</Project>

Just build this bad boy with msbuild /t:Restore fail.csproj

[KirillOsenkov]

Since the actual exception happens in NuGet.Build.Tasks.Console.exe, I found it's easier to debug this by starting the .exe directly:

C:\msbuild\artifacts\bin\bootstrap\net472\Common7\IDE\CommonExtensions\Microsoft\NuGet\NuGet.Build.Tasks.Console.exe

and passing it these args:

"Recursive=True;CleanupAssetsForUnsupportedProjects=True;DisableParallel=False;Force=False;ForceEvaluate=False;HideWarningsAndErrors=False;IgnoreFailedSources=False;Interactive=False;NoCache=False;NoHttpCache=False;RestorePackagesConfig=False" "C:\msbuild\artifacts\bin\bootstrap\net472\MSBuild\Current\Bin\amd64\MSBuild.exe" "C:\temp\fail\fail.csproj" "MSBuildRestoreSessionId=2b93b8ed-8a76-41be-bb2f-a7c3e3c92e01;ExcludeRestorePackageImports=True;OriginalMSBuildStartupDirectory=C:\msbuild;SolutionPath=C:\temp\fail\fail.csproj"

Have to start it under debugger, of course, for example by opening the .exe as a project in Visual Studio, editing the project properties to pass the argument string, and then pressing F5. Remember to disable Just My Code and enable all first-chance exceptions.

[ladipro]

Just to be sure - the expected behavior is for the attempted glob expansion to silently fail and \\csc.* be added to None verbatim, is that correct?

[rainersigwald]

I found it's easier to debug this by starting the .exe directly

Because @jeffkl has been around the block a time or two (complimentary) you can also set an environment variable DEBUG_RESTORE_TASK=true around the normal repro.

Just to be sure - the expected behavior is for the attempted glob expansion to silently fail and \\csc.* be added to None verbatim, is that correct?

Sort of; in this case glob expansion is disabled via MSBuildSkipEagerWildCardEvaluationRegexes, so an item with an "escaped" wildcard is passed into the Update machinery.

I think I see the fix.

[rainersigwald]

Hm. Well. Almost!

diff --git a/src/Build/Evaluation/LazyItemEvaluator.cs b/src/Build/Evaluation/LazyItemEvaluator.cs
index bd34997b83..4fa4f02291 100644
--- a/src/Build/Evaluation/LazyItemEvaluator.cs
+++ b/src/Build/Evaluation/LazyItemEvaluator.cs
@@ -444,7 +444,7 @@ namespace Microsoft.Build.Evaluation
                 {
                     for (int i = 0; i < items.Count; i++)
                     {
-                        string fullPath = FileUtilities.GetFullPath(items[i].Item.EvaluatedIncludeEscaped, items[i].Item.ProjectDirectory);
+                        string fullPath = FileUtilities.NormalizePathForComparisonNoThrow(items[i].Item.EvaluatedIncludeEscaped, items[i].Item.ProjectDirectory);
                         if (itemsWithNoWildcards.TryGetValue(fullPath, out UpdateOperation op))
                         {
                             items[i] = op.UpdateItem(items[i]);

Gets past the crash but still fails with

C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.DefaultItems.targets(101,13): error MSB4248: Cannot expand metadata in expression "$([MSBuild]::ValueOrDefault('%(FullPath)', '').StartsWith($([MSBuild]::EnsureTrailingSlash($(MSBuildProjectDirectory)))))". The item metadata "%(FullPath)" cannot be applied to the path "\\csc.*". The UNC path should be of the form \\server\share.  C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.DefaultItems.targets

[rainersigwald]

@KirillOsenkov to work around in your original scenario can you condition the globbed item on $(ExcludeRestorePackageImports)?

[rainersigwald]

Gets past the crash but still fails with

C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.DefaultItems.targets(101,13): error MSB4248: Cannot expand metadata in expression "$([MSBuild]::ValueOrDefault('%(FullPath)', '').StartsWith($([MSBuild]::EnsureTrailingSlash($(MSBuildProjectDirectory)))))". The item metadata "%(FullPath)" cannot be applied to the path "\\csc.*". The UNC path should be of the form \\server\share.  C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.DefaultItems.targets

This can be worked around with SetLinkMetadataAutomatically=false

❯ artifacts\bin\bootstrap\net472\MSBuild\Current\Bin\amd64\MSBuild.exe S:\repro\dotnet\msbuild\issues\9405\fail.csproj -t:restore -p:SetLinkMetadataAutomatically=false
Restore complete (1.6s)

Build succeeded in 2.1s

[baronfel]

Do we have a documented way for someone to generate that Link metadata on-demand if they need it? One thing I see with a lot of our automatic behavior toggles is that what users have to do to recreate that automatic behavior is not always documented or clear.

[rainersigwald]

"Bake it into the project like VS used to" . . . but yeah I don't think there are docs.

[KirillOsenkov]

Thanks Rainer! NormalizePathForComparisonNoThrow does look like a good fix! It brings the behavior inline with the simpler repro that I pasted at the top: it doesn't crash and gives a helpful error message that helps me understand what's going on and fix it.

SetLinkMetadataAutomatically=false is the cherry on top that lets me fix that separately as well.

I'd say let's just ship that diff you've pasted above and I'll work around the rest of it in my solution.

Food for thought is that this is another cursed case of $(NonExistingProperty)\... which evaluates to the root of the drive if the property is not set. Should we think about emitting a warning when a property of that shape evaluates to empty, resulting in a leading directory separator? I've seen this wreak havoc time and time again, with accidental drive enumeration and other unintended consequences.

[rainersigwald]

Should we think about emitting a warning when a property of that shape evaluates to empty, resulting in a leading directory separator?

Yes! https://github.com/dotnet/msbuild/pull/7029 added a message about this, that can be elevated to a break via MSBUILDFAILONDRIVEENUMERATINGWILDCARD. And when we have analyzers that's a pattern we should flag immediately.

[rainersigwald]

I'd say let's just ship that diff you've pasted above and I'll work around the rest of it in my solution.

works for me! #9409