Skip to content

Log and restore XmlNodeType in XmlNodeReader#124559

Open
lilinus wants to merge 3 commits intodotnet:mainfrom
lilinus:fix-xml-node-reader
Open

Log and restore XmlNodeType in XmlNodeReader#124559
lilinus wants to merge 3 commits intodotnet:mainfrom
lilinus:fix-xml-node-reader

Conversation

@lilinus
Copy link
Contributor

@lilinus lilinus commented Feb 18, 2026

Fixes #34443

Copilot AI review requested due to automatic review settings February 18, 2026 15:29
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 18, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lilinus lilinus marked this pull request as ready for review February 19, 2026 08:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 3, 2026 13:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines +129 to +134
XmlNodeReader nodeReader = NodeReaderTestHelper.CreateNodeReader(xml);
Assert.True(nodeReader.ReadToDescendant("child"));
Assert.True(nodeReader.MoveToAttribute("attr1"));
Assert.False(nodeReader.MoveToAttribute("attr2"));
nodeReader.ReadStartElement("child");
}
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new regression test verifies success by calling ReadStartElement, but it doesn’t directly assert the reader state after MoveToAttribute("attr2") returns false. To better lock in the intended fix (reader position must be unchanged), add assertions that NodeType/Name (and/or Depth) are still those of the existing attribute (e.g., still on attr1) before calling ReadStartElement.

Copilot generated this review using guidance from repository custom instructions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Xml community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XmlNodeReader MoveToAttribute bug, leaves XmlReader in inconsistent state

2 participants