[Bug]: MSB4028 "XmlQuery" task's outputs could not be retrieved from the "Values" parameter. Specified cast is not valid.

[jehhynes]

Issue Description

After upgrading Visual Studio from 17.5.4 to 17.6.2, I am now encountering an error in our project's build. We are using MSBuildTasks and have the following target:

	<Target Name="SetConfig">
		<XmlQuery XmlFileName="$(ProjectDir)\test.xml" XPath="//add">
			<Output TaskParameter="Values" ItemName="ConfigSettings" />
		</XmlQuery>
	</Target>

When building/rebuilding the project, I get an error for this line stating the following:

MSB4028 The "XmlQuery" task's outputs could not be retrieved from the "Values" parameter. Specified cast is not valid.

I have included a zipped project with minimal code to reproduce the issue.
PLEASE NOTE: In the attached sample project, I was not always able to reproduce by using Build in Visual Studio, but using Rebuild did consistently generate this error.

XmlQueryError.zip

Steps to Reproduce

• Unzip XmlQueryError.zip (attached)
• Open the project in Visual Studio (I'm using version 17.6.2)
• Rebuild the project (Please Rebuild rather than Build)
• Note that the build fails.

Expected Behavior

An error should not occur.

Actual Behavior

The build fails. The Output window states: XmlQueryError\XmlQueryError.csproj(24,4): error MSB4028: The "XmlQuery" task's outputs could not be retrieved from the "Values" parameter. Specified cast is not valid.

Analysis

This same issue occurs on at least 3 separate solutions my company has.
I've reproduced it using the following versions of MSBuildTasks: 1.4.0.88, 1.5.0.235.
The error does not occur in VS 17.5.x but does occur in VS 17.6.x

See: https://github.com/loresoft/msbuildtasks/blob/master/Source/MSBuild.Community.Tasks/Xml/XmlQuery.cs#L70
The Values property is any array ITaskItem[].

Versions & Configurations

No response

[rainersigwald]

I repro; likely related to #8645. The task in question references MSBuild 4.0 instead of 3.5 but that's still pretty stale. Unfortunately since it's 4.0 that means you lose the override-it-to-run-out-of-proc workaround for 3.5 tasks.

[rainersigwald]

Depending on the details of your XmlQuery usage you may be able to work around this by switching to the built-in XmlPeek.

[rainersigwald]

Failing stack is

System.Core.dll!System.Linq.Enumerable.CastIterator<System.Collections.DictionaryEntry>(System.Collections.IEnumerable source) Line 2550
System.Core.dll!System.Linq.Enumerable.WhereSelectEnumerableIterator<System.Collections.DictionaryEntry, System.Collections.Generic.KeyValuePair<string, string>>.MoveNext() Line 392
System.Core.dll!System.Linq.Enumerable.WhereSelectEnumerableIterator<System.Collections.Generic.KeyValuePair<string, string>, Microsoft.Build.Execution.ProjectMetadataInstance>.MoveNext() Line 392
Microsoft.Build.dll!Microsoft.Build.Collections.CopyOnWritePropertyDictionary<Microsoft.Build.Execution.ProjectMetadataInstance>.ImportProperties.__Items|0() Line 294
System.Collections.Immutable.dll!System.Collections.Immutable.ImmutableDictionary<string, Microsoft.Build.Execution.ProjectMetadataInstance>.AddRange(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, Microsoft.Build.Execution.ProjectMetadataInstance>> items, System.Collections.Immutable.ImmutableDictionary<string, Microsoft.Build.Execution.ProjectMetadataInstance>.MutationInput origin, System.Collections.Immutable.ImmutableDictionary<string, Microsoft.Build.Execution.ProjectMetadataInstance>.KeyCollisionBehavior collisionBehavior)
System.Collections.Immutable.dll!System.Collections.Immutable.ImmutableDictionary<string, Microsoft.Build.Execution.ProjectMetadataInstance>.SetItems(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, Microsoft.Build.Execution.ProjectMetadataInstance>> items)
Microsoft.Build.dll!Microsoft.Build.Collections.CopyOnWritePropertyDictionary<Microsoft.Build.Execution.ProjectMetadataInstance>.ImportProperties(System.Collections.Generic.IEnumerable<Microsoft.Build.Execution.ProjectMetadataInstance> other) Line 290
Microsoft.Build.dll!Microsoft.Build.BackEnd.TaskExecutionHost.GatherTaskItemOutputs(bool outputTargetIsItem, string outputTargetName, Microsoft.Build.Framework.ITaskItem[] outputs, Microsoft.Build.Construction.ElementLocation parameterLocation, Microsoft.Build.Framework.TaskPropertyInfo parameter) Line 1461
Microsoft.Build.dll!Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.GatherTaskOutputs(string parameterName, Microsoft.Build.Construction.ElementLocation parameterLocation, bool outputTargetIsItem, string outputTargetName) Line 437
Microsoft.Build.dll!Microsoft.Build.BackEnd.TaskBuilder.GatherTaskOutputs(Microsoft.Build.BackEnd.ITaskExecutionHost taskExecutionHost, Microsoft.Build.BackEnd.TaskExecutionMode howToExecuteTask, Microsoft.Build.BackEnd.ItemBucket bucket) Line 1142
Microsoft.Build.dll!Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(Microsoft.Build.BackEnd.ITaskExecutionHost taskExecutionHost, Microsoft.Build.BackEnd.Logging.TaskLoggingContext taskLoggingContext, Microsoft.Build.BackEnd.TaskHost taskHost, Microsoft.Build.BackEnd.ItemBucket bucket, Microsoft.Build.BackEnd.TaskExecutionMode howToExecuteTask) Line 978

[jrdodds]

The MSBuildTasks NuGet package was last released in 2017. The project has been looking for a new maintainer since 2019.

The tasks are built to .Net Framework v4.0. It appears that a .Net Framework application configuration file is being read with XmlQuery. Your project is .Net 6.0.

Looking at the source for XmlQuery, it makes heavy use of adding metadata to the returned items.

XmlPeek doesn't add metadata. It doesn't try to mimic returning an XML node.

If you just need to cherry pick specific values, changing to XmlPeek won't be too difficult. You can, for example, write an XPath that will return the value of the value attribute for add elements with a key attribute with a value of key1 - e.g. /configuration/add[@key='key1']/@value.

Mapping the complete configuration file to an item collection might be implemented by using XmlPeek with an XPath that returns all the key values (/configuration/add/@key) and then batching to run XmlPeek to retrieve each value for each key.

[jehhynes]

@jrdodds Thanks for the workaround suggestions.

[jehhynes]

@rainersigwald Could the problem have been caused by this commit? 8ffc3fe

Perhaps this should fallback to the prior behavior. Something like this:

newItem.SetMetadataOnTaskOutput(output.CloneCustomMetadata().Cast<object>()
    .Select(x =>
        x is DictionaryEntry de ? new KeyValuePair<string, string>((string)de.Key, EscapingUtilities.Escape((string)de.Value)) :
        x is KeyValuePair<string, string> kvp ? new KeyValuePair<string, string>(kvp.Key, EscapingUtilities.Escape(kvp.Value)) :
        throw new Exception("Metadata item was neither DictionaryEntry nor KeyValuePair<string, string>")
    )
);

[jrdodds]

@jehhynes I don't think that code is being hit with XmlQuery because the XmlQuery task is returning TaskItem instances.

I think it is going down this path: https://github.com/dotnet/msbuild/blob/c6daff1259eb3c2a42d53f664f82c3c8a66c6166/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs#LL1374C1-L1381C30

[jehhynes]

It appears to me that the output is ITaskItem which may or may not be a TaskItem.

        [Output]
        public ITaskItem[] Values
        {
            get { return values.ToArray(); }
        }

In the Execute() method of XmlQuery (https://github.com/loresoft/msbuildtasks/blob/master/Source/MSBuild.Community.Tasks/Xml/XmlQuery.cs#L133), the case for XPathExpression.ReturnType of XPathResultType.NodeSet is adding values of type XmlNodeTaskItem which implements ITaskItem but does not inherit TaskItem (https://github.com/loresoft/msbuildtasks/blob/master/Source/MSBuild.Community.Tasks/Xml/XmlNodeTaskItem.cs#L14).

case XPathResultType.NodeSet:
    XPathNodeIterator nodes = navigator.Select(expression);
    while (nodes.MoveNext())
    {
        values.Add(new XmlNodeTaskItem(nodes.Current, reservedMetaDataPrefix));
    }

public class XmlNodeTaskItem : ITaskItem

Therefore I think it would actually be following this path: https://github.com/dotnet/msbuild/blob/c6daff1259eb3c2a42d53f664f82c3c8a66c6166/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs#LL1398C45-L1398C45

if (outputAsProjectItem != null) //This would be null since XmlNodeTaskItem does not inherit TaskItem
{
    ...
}
else //This would be entered
{
    if (output is ITaskItem2 outputAsITaskItem2) // This would not be entered since XmlNodeTaskItem does not inherit ITaskItem2
    {
        ...
    }
    else //This would be entered
    {
        // Not a ProjectItemInstance.TaskItem or even a ITaskItem2, so we have to fake it.
        // Setting an item spec expects the escaped value, as does setting metadata.
        newItem = new ProjectItemInstance(_projectInstance, outputTargetName, EscapingUtilities.Escape(output.ItemSpec), parameterLocationEscaped);

        newItem.SetMetadataOnTaskOutput(output.CloneCustomMetadata()
            .Cast<DictionaryEntry>()
            .Select(x => new KeyValuePair<string, string>((string)x.Key, EscapingUtilities.Escape((string)x.Value))));
    }
}

[jrdodds]

I had misread the class definition of XmlNodeTaskItem as inheriting from TaskItem.

[rainersigwald]

I was very confused as to what was going on here. I asked internally and @stephentoub helped me understand:

In the original code

msbuild/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs

Lines 1450 to 1453 in 698aac4

	foreach (DictionaryEntry entry in output.CloneCustomMetadata())
	{
	newItem.SetMetadataOnTaskOutput((string)entry.Key, EscapingUtilities.Escape((string)entry.Value));
	}

C# will, per its foreach rules, call GetEnumerator() on the return value of CloneCustomMetadata(). Because that's an IDictionary, it gets an implementation of IDictionary.GetEnumerator() which returns DictionaryEntry objects.

Inserting the LINQ changes things, because Cast<T> works on IEnumerable.GetEnumerator(), which returns KeyValuePair<TKey, TValue> when the concrete returned value is a Dictionary<TKey, TValue>.

To play around with this, check out this minimized SharpLab repro.

[AR-May]

Team triage: we would like to write a test catching this issue.