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

XmlReader ReadElementContentAsString Hangs if End of Document Tag Reached in Stream #1029

Open
Tracked by #64602
iUnknwn opened this issue Dec 18, 2019 · 5 comments
Open
Tracked by #64602

Comments

@iUnknwn
Copy link

iUnknwn commented Dec 18, 2019

Given the following XML sent over a network stream:
<?xml version="1.0" encoding="utf-16"?><DeviceCommand>PlayTone</DeviceCommand>

Which is generated by the XmlWriter command:

var writer = XmlWriter.Create(namedPipeClientStream)
writer.WriteElementString("DeviceCommand", "PlayTone")
writer.Flush()

This reader command will cause an XML reader on the server to hang until the network stream is closed:

var reader = XmlReader.Create(namedPipeServerStream)
reader.MoveToContent()
var txt = reader.ReadElementContentAsString()

The command fails even though the entire element was sent. This alternative command works as expected:

reader.MoveToContent()
reader.Read()
var txt = reader.Value;

However, if the stream is closed, then the XmlReader is able to read the node without issue and the call to ReadElementContentAsString works as expected.

It looks like XmlReader is waiting until it gets the node after the EndElement before it returns the string, but it doesn't properly recognize the end of the root node. This appears to be undocumented behavior.

This was tested on .NET Core 3.0 on Windows.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Dec 18, 2019
@buyaa-n buyaa-n removed the untriaged New issue has not been triaged by the area owner label Jun 29, 2020
@buyaa-n buyaa-n added this to the 6.0.0 milestone Jun 29, 2020
@buyaa-n
Copy link
Contributor

buyaa-n commented Jun 29, 2020

@iUnknwn thank you for reporting the issue, this issue needs some detailed investigations which cannot be done within .Net 5.
It would be great if you would be interested in doing the investigation/fixes.

@iUnknwn
Copy link
Author

iUnknwn commented Jun 30, 2020

@buyaa-n I'd be potentially interested, what are you asking for?

I could probably write up an MSUnit/xUnit test to test for this behavior, if you're looking for something basic to demo the problem.

If you're looking for a patch/PR to fix the issue, that might be a bit trickier.

@buyaa-n
Copy link
Contributor

buyaa-n commented Jun 30, 2020

I could probably write up an MSUnit/xUnit test to test for this behavior, if you're looking for something basic to demo the problem.

That would be very helpful for whoever would investigate it

If you're looking for a patch/PR to fix the issue, that might be a bit trickier.

Sure it is not easy, and it is optional/voluntary. If you are willing to contribute for test repro, finding the root cause, propose a fix or raise a PR etc., whichever level it was that would be very helpful. Thanks again!

@iUnknwn
Copy link
Author

iUnknwn commented Aug 6, 2020

@buyaa-n Sorry this took so long, but I have a repo setup with test cases for this issue. It shows the issue only happens on open streams, and it specifically occurs on single-element html items.

https://github.com/iUnknwn/XmlReaderBugDemo

Please let me know if you need more info.

@krwq
Copy link
Member

krwq commented Jul 15, 2021

For now I believe you should be able to use ReadContentAsString which ReadElementContentAsString calls internally except ReadElementContentAsString also tries to move to the next element.

I think the fix for this should be somewhere here

private void FinishReadElementContentAsXxx()

but this will likely require more investigation as there are more than one XmlReader implementation in the framework.

I'm moved this to 7.0 for now since we do not plan to do this investigation in 6.0

@ghost ghost added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jul 15, 2021
@krwq krwq removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jul 15, 2021
@jeffhandley jeffhandley modified the milestones: 7.0.0, Future Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

6 participants