GNOME Bugzilla – Bug 679204
Fix GUPnPXMLDoc error reporting
Last modified: 2012-08-19 14:13:15 UTC
Currently GUPnPXMLDoc uses xmlRecoverFile which always returns a non-null xmlDocPtr so can't detect invalid XML files. The second patch disables libxml2's console messages unless GUPNP_DEBUG is set.
Created attachment 217743 [details] [review] Don't use xmlRecoverFile, use xmlReadFile xmlRecoverFile never errors out (returning a non-null file). So invalid XML files can't be detected.
Created attachment 217744 [details] [review] Only show libxml2 console messages in debug mode
Review of attachment 217743 [details] [review]: I thought that was the point of starting to use xmlRecoverFile: To be very lenient with broken XML descriptions out there.
Created attachment 217745 [details] [review] Only show libxml2 console messages in debug mode
(In reply to comment #3) > Review of attachment 217743 [details] [review]: > > I thought that was the point of starting to use xmlRecoverFile: To be very > lenient with broken XML descriptions out there. But _new_from_path is used for local description files for services. You should be able to control those. The remote files are still read with xmlRecoverMemory.
The current behaviour causes Rygel to crash on invalid files in ~/.config/Rygel as we can't detect errors there and just carry on, sometimes crashing nicely.
Review of attachment 217745 [details] [review]: Ah ok, just make sure that that really is the case. :)
Review of attachment 217745 [details] [review]: Sorry commented on the wrong one but this looks good too. :)
Review of attachment 217743 [details] [review]: Please read the comment I put mistakenly on the other patch.
I don't understand that comment. what is always the case?
(In reply to comment #10) > I don't understand that comment. what is always the case? I didn't say 'always' but 'really'. :) I meant that make sure we are not using this code-path for device proxy (i-e for XML docs from other devices).
Ah, ok. The only place libgupnp uses _new_from_path is in gupnp-root-device.c. If I read the code correctly, that should be only for local devices. The ControlPoint uses xmlRecoverMemory and does not explicitly create a root device.
I'm kind-of tempted to replace those with xmlReadMemory with XML_PARSE_RECOVER set (which is hopefully equivalent to xmlRecoverMemory) to also turn on the warning/error messages on in debug only - thoughts?
(In reply to comment #13) > I'm kind-of tempted to replace those with xmlReadMemory with XML_PARSE_RECOVER > set (which is hopefully equivalent to xmlRecoverMemory) to also turn on the > warning/error messages on in debug only - thoughts? Yeah, sounds right!
Attachment 217743 [details] pushed as 4a0ec89 - Don't use xmlRecoverFile, use xmlReadFile Attachment 217745 [details] pushed as 7010c35 - Only show libxml2 console messages in debug mode