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

XmlPeek not working correctly for element nodes #1537

Closed
peter-perot opened this issue Mar 20, 2017 · 4 comments
Closed

XmlPeek not working correctly for element nodes #1537

peter-perot opened this issue Mar 20, 2017 · 4 comments
Labels
Milestone

Comments

@peter-perot
Copy link

peter-perot commented Mar 20, 2017

What You Are Seeing?

I have the following XML document:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
    <WebPublishMethod>FileSystem</WebPublishMethod>
    <LastUsedBuildConfiguration>DeploymentTemplate</LastUsedBuildConfiguration>
    <LastUsedPlatform>Any CPU</LastUsedPlatform>
    <SiteUrlToLaunchAfterPublish />
    <LaunchSiteAfterPublish>True</LaunchSiteAfterPublish>
    <ExcludeApp_Data>False</ExcludeApp_Data>
    <publishUrl>C:\Deployment\DeploymentTemplate\WebApi</publishUrl>
    <DeleteExistingFiles>False</DeleteExistingFiles>
  </PropertyGroup>
</Project>

The following code does not yield the publishUrl element (instead null ist returned and a warning is logged):

var xmlPeekSettings = new XmlPeekSettings
{
  Namespaces = new Dictionary<string, string>{
    { "msbuild", "http://schemas.microsoft.com/developer/msbuild/2003" }
  }
};

var xmlElementContents = XmlPeek(pubXmlFile,
  "/msbuild:Project/msbuild:PropertyGroup/msbuild:publishUrl", xmlPeekSettings);

Running the same code using the standard .NET XPathDocument works okay.

What is Expected?

XmlPeek should return the contents of the publishUrl element, i.e. C:\Deployment\DeploymentTemplate\WebApi.

When using XPathDocument everything works okay:

using (var stream = new System.IO.FileStream(pubXmlFile.ToString(), FileMode.Open))
{
  var xPathDocument = new System.Xml.XPath.XPathDocument(stream);
  var navigator = xPathDocument.CreateNavigator();

  System.Xml.XmlNamespaceManager ns = null;

  if (navigator.NameTable != null)
  {
    ns = new System.Xml.XmlNamespaceManager(navigator.NameTable);
    ns.AddNamespace("msbuild", "http://schemas.microsoft.com/developer/msbuild/2003");
  }

  var node = navigator.SelectSingleNode(
    "/msbuild:Project/msbuild:PropertyGroup/msbuild:publishUrl", ns);
}

Here node contains the selected node.

What version of Cake are you using?

Version 0.17.0

Are you running on a 32 or 64 bit system?

64bit

What environment are you running on? Windows? Linux? Mac?

Windows

How Did You Get This To Happen? (Steps to Reproduce)

See code above.

@peter-perot
Copy link
Author

peter-perot commented Mar 21, 2017

[Proposal for a Bugfix]

After some investigation I found the bug. Have a look at line

var node = document.SelectSingleNode(xpath, namespaceManager);
:

When calling method SelectSingleNode(...) on an instance of XmlDocument, an XmlNode is returned. Then in turn when reading the Value property of the XmlNode, null is returned for nodes of type XmlNodeType.Element (so say the docs from Microsoft).

This does not happen if you are using an XPathNavigator - yes, this is really weird, but it works.

Here the corrected code:

/// <summary>
/// Gets the value of a target node.
/// </summary>
/// <returns>The value found at the given XPath query (or the first, if multiple eligible nodes are found).</returns>
/// <param name="source">The xml to peek into.</param>
/// <param name="xpath">The xpath of the single node to get.</param>
/// <param name="settings">Additional settings to tweak Xml Peek behavior.</param>
private static string XmlPeek(XmlReader source, string xpath, XmlPeekSettings settings)
{
    if (source == null)
    {
        throw new ArgumentNullException(nameof(source));
    }

    if (string.IsNullOrWhiteSpace(xpath))
    {
        throw new ArgumentNullException(nameof(xpath));
    }

    if (settings == null)
    {
        throw new ArgumentNullException(nameof(settings));
    }

    var document = new XmlDocument();
    document.PreserveWhitespace = settings.PreserveWhitespace;
    document.Load(source);

    var navigator = document.CreateNavigator();
    XmlNamespaceManager namespaceManager = null;

    if (navigator.NameTable != null)
    {
        namespaceManager = new XmlNamespaceManager(navigator.NameTable);
        foreach (var xmlNamespace in settings.Namespaces)
        {
            namespaceManager.AddNamespace(xmlNamespace.Key /* Prefix */, xmlNamespace.Value /* URI */);
        }
    }

    var node = navigator.SelectSingleNode(xpath, namespaceManager);
    return node?.Value;
}

Hint:

Have a look at the XML doc comments in this file - some are misleading. For the method above I already corrected the comments.

@peter-perot peter-perot changed the title XmlPeek not working correctly with namespaces XmlPeek not working correctly for element nodes Mar 21, 2017
@alvaromongon
Copy link

I have just updated to Cake version 0.21.1 and it is still happening.

Any estimation about when this will be fixed?

Thanks!

@peter-perot
Copy link
Author

peter-perot commented Jul 26, 2017

Currently I don't have time to drop a pull request, but the code I posted above fixes the issue. Maybe some of the Cake guys can take the code snippet and fix that bug. ;-)

kcamp added a commit to kcamp/cake that referenced this issue Jul 26, 2017
… handle retrieval of element values; add supporting tests
kcamp added a commit to kcamp/cake that referenced this issue Jul 26, 2017
… handle retrieval of element values; add supporting tests
kcamp added a commit to kcamp/cake that referenced this issue Jul 27, 2017
… handle retrieval of element values; add supporting tests
kcamp added a commit to kcamp/cake that referenced this issue Jul 31, 2017
… handle retrieval of element values; add supporting tests
kcamp added a commit to kcamp/cake that referenced this issue Aug 2, 2017
… handle retrieval of element values; add supporting tests
@devlead devlead added this to the v0.22.0 milestone Aug 3, 2017
@devlead devlead added the Bug label Aug 3, 2017
@devlead
Copy link
Member

devlead commented Aug 3, 2017

fixed by #1701

@devlead devlead closed this as completed Aug 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants