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

XNodeReader.GetAttribute(int) should throw on invalid indices #44287

Closed
krwq opened this issue Nov 5, 2020 · 2 comments · Fixed by #44589
Closed

XNodeReader.GetAttribute(int) should throw on invalid indices #44287

krwq opened this issue Nov 5, 2020 · 2 comments · Fixed by #44589
Labels
area-System.Xml good first issue Issue should be easy to implement, good for first-time contributors
Milestone

Comments

@krwq
Copy link
Member

krwq commented Nov 5, 2020

Note: this class is internal but this method is accessible through XmlReader's public surface which can be created with i.e. XNode.CreateReader.

All XmlReader implementations except XNodeReader throw ArgumentOutOfRangeException on invalid indices. This specific class returns null though. Documentation of XmlReader says that ArgumentOutOfRange is expected behavior.

Also same method returns null when reader state is not in interactive mode - likely this should be replaced with InvalidOperationException.

Found with nullable annotations.

@krwq krwq added the good first issue Issue should be easy to implement, good for first-time contributors label Nov 5, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Xml untriaged New issue has not been triaged by the area owner labels Nov 5, 2020
@ghost
Copy link

ghost commented Nov 5, 2020

Tagging subscribers to this area: @buyaa-n, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.

@krwq
Copy link
Member Author

krwq commented Nov 5, 2020

I think we should fix this for consistency but there is no urgency here considering this issue has been there for years and no one has complained.

@krwq krwq added this to the Future milestone Nov 5, 2020
@krwq krwq removed the untriaged New issue has not been triaged by the area owner label Nov 5, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Xml good first issue Issue should be easy to implement, good for first-time contributors
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants