GNOME Bugzilla – Bug 793955
Missing quote in action response XML
Last modified: 2018-03-17 16:30:27 UTC
The GUPnPServiceProxy creates invalid XML in the action response: <?xml version="1.0"?><s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/" s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"><s:Body><u:GetMuteResponse xmlns:u=urn:upnp-org:serviceId:RenderingControl"><CurrentMute>0</CurrentMute></u:GetMuteResponse></s:Body></s:Envelope> There is an opening quote missing in xmlns:u=urn:upnp-org:serviceId:RenderingControl"
Created attachment 369146 [details] [review] Proposed fix The attached patch should fix the problem. The code will still create questionable XML if the service_type isn't set, but that's a different issue.
Interesting, it seems that I had fixed this years ago already, but for some undocumented reason Zeeshan reverted it on the next day: commit 481f399f80a6eaa5c30a77e2a5539e5f967c84df Author: Zeeshan Ali (Khattak) <zeeshanak@gnome.org> Date: Fri Feb 18 11:45:57 2011 +0200 Revert "Add missing quote in action response" This reverts commit 1ff7bd81f152b2579744fe2de46fb709cf51f279. commit 1ff7bd81f152b2579744fe2de46fb709cf51f279 Author: Sven Neumann <s.neumann@raumfeld.com> Date: Thu Feb 17 17:57:57 2011 +0100 Add missing quote in action response The action response document was missing an opening quote sign in the namespace declaration of the <actionResponse> element. Fix it by adding the missing quote and also tweak the code a bit to output well-formed XML in case that the service-type is not set.
Found the reason for the revert in the mailing-list archives: https://mail.gnome.org/archives/gupnp-list/2011-February/msg00011.html OK, so the root of problem was actually on the client. But maybe the code should still be changed because (a) It created invalid XML in response to an invalid request. Maybe it should rather respond with an error so that people fix their clients, or alternatively deal gracefully with the client error and produce a valid response. (b) The looks obviously wrong and there is no comment explaining why there's an unbalanced quote added.
Review of attachment 369146 [details] [review]: This patch is wrong.
I vaguely remember this discussion as well
So this code assumes well-quoted SOAP-Action header? That's probably not that wise.
Created attachment 369156 [details] [review] Patch suggestion
Review of attachment 369156 [details] [review]: Looks good to me. I've run our automated tests against a system that has this patch applied and did not run into any problems.