StevenLooman / async_upnp_client

Async UPnP Client for Python
Other
46 stars 37 forks source link

Unescape of action responses can invalidate included XML data #50

Closed B5r1oJ0A9G closed 3 years ago

B5r1oJ0A9G commented 3 years ago

Hi there!

I intend to replace the blocking UPNP calls in teufel_raumfeld by async ones via async_upnp_client. Most of the methods and functions I could successfully adapt. However, I encounter problems with some of them.

It turns out that text values in XML data of UPNP responses got unescaped, what makes them impossible to be imported again via xmltodict - e.g. if there used to be an escaped ampersand included. https://github.com/StevenLooman/async_upnp_client/blob/a3580b97535af8c766721761e58418341ccfded7/async_upnp_client/client.py#L603

Is there a chance to add an option or property to disable the un-escaping of (action) responses?

StevenLooman commented 3 years ago

Hi @B5r1oJ0A9G! Should this be applied to every response, or just a single action?

I do think that this is not UPnP-specifition compliant. But we do have some exceptions already, i.e., https://github.com/StevenLooman/async_upnp_client/blob/a3580b97535af8c766721761e58418341ccfded7/async_upnp_client/client_factory.py#L41 and https://github.com/StevenLooman/async_upnp_client/blob/a3580b97535af8c766721761e58418341ccfded7/async_upnp_client/client_factory.py#L42.

Adding another one should not be a problem.

StevenLooman commented 3 years ago

Or what about storing the unescaped value or the ET element as well?

B5r1oJ0A9G commented 3 years ago

Thanks for the quick reply!

I guess the fields I am facing issues with should always remain escaped (URI and DIDL-Lite). I haven't studied the entire specs but at least here it is mentioned: ContentDirectory:1 Service Template Version 1.01 .

Both proposed approaches would work for me. Personally I'd opt for an optional setting to avoid additional payload that is not used. Whether the raw data is then optionally added or just kept as-is, would again work for me in both cases - where I'd also again opt for the latter for the same reasoning as before.

Edit: In my particular case I know where it is needed. Therefore it would be sufficient on action level - at least for the moment :)

StevenLooman commented 3 years ago

This library does some things with DIDL-Lite for DLNA/DMR devices: https://github.com/StevenLooman/async_upnp_client/blob/development/async_upnp_client/profiles/dlna.py#L780 and https://github.com/StevenLooman/async_upnp_client/blob/development/async_upnp_client/profiles/dlna.py#L799

This is used by the dlna_dmr component in Home Assistant.

As far as I can remember, no special escaping was needed. The spec you've linked mentions (page 14):

Note that since the DIDL-Lite format of Result is based on XML, it needs to be escaped (using the normal XML rules: [XML] Section 2.4 Character Data and Markup) before embedding in a SOAP response message.

Now I do know that there are numerous devices which do not fully adhere to the DLNA spec... but before we change something I want to be sure it is really needed.

B5r1oJ0A9G commented 3 years ago

Thanks for the tip with didl_lite! I'll look into it. I have no problem to replace xmltodict by another method of parsing the data, if it finally does the trick.

From a syntactical point of view though I think it is still not correct. However, I understand and agree to not change an existing implementation, if it finally works for all/most use cases so far and a change is not mandatory.

Here a simple example - just for the record:

Code to test with hassfeld ```python import hassfeld import xmltodict import asyncio def main(): raumfeld = hassfeld.RaumfeldHost("10.0.107.12") raumfeld.start_update_thread() zone = ['Werkraum'] position_info = raumfeld.get_position_info(zone) xml = position_info["TrackMetaData"] print(f"XML returned by upnpclient:\n\n{xml}") dict_ = xmltodict.parse(xml) print(f"\nXML from upnpclient succesfully converted into dict:\n\n{dict_}") position_info = asyncio.run(raumfeld.async_get_position_info(zone)) xml = position_info["TrackMetaData"] print(f"\n\n\nXML returned by async_upnp_client:\n\n{xml}") print("\nXML from async_upnp_client failed to convert:\n") dict_ = xmltodict.parse(xml) print(dict_) main() ```
Output of test script ``` XML returned by upnpclient: http://10.0.107.12:47366/raumfeldImage?albumId=bd9da4d2-cd16-4b5f-9e8d-2bbd0636c7b2&album=IIII&artist=Robin%20Schulz&service=TidalIIIIRobin SchulzRobin Schulztidal-track://169351013Dream (feat. colour your mind)13Trackobject.item.audioItem.musicTrackTidal86400 XML from upnpclient succesfully converted into dict: OrderedDict([('DIDL-Lite', OrderedDict([('@xmlns', 'urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/'), ('@xmlns:raumfeld', 'urn:schemas-raumfeld-com:meta-data/raumfeld'), ('@xmlns:upnp', 'urn:schemas-upnp-org:metadata-1-0/upnp/'), ('@xmlns:dlna', 'urn:schemas-dlna-org:metadata-1-0/'), ('@xmlns:dc', 'http://purl.org/dc/elements/1.1/'), ('@xmlns:pv', 'http://www.pv.com/pvns/'), ('@lang', 'en'), ('item', OrderedDict([('@parentID', '0/Tidal/New/NewAlbums/169351000'), ('@id', '0/Tidal/New/NewAlbums/169351000/169351013'), ('@restricted', '1'), ('upnp:albumArtURI', OrderedDict([('@dlna:profileID', 'JPEG_TN'), ('#text', 'http://10.0.107.12:47366/raumfeldImage?albumId=bd9da4d2-cd16-4b5f-9e8d-2bbd0636c7b2&album=IIII&artist=Robin%20Schulz&service=Tidal')])), ('upnp:album', 'IIII'), ('upnp:artist', 'Robin Schulz'), ('dc:creator', 'Robin Schulz'), ('res', OrderedDict([('@protocolInfo', 'tidal-track:*:audio/tidal-track:*'), ('@duration', '0:03:11.000'), ('#text', 'tidal-track://169351013')])), ('dc:title', 'Dream (feat. colour your mind)'), ('upnp:originalTrackNumber', '13'), ('raumfeld:name', 'Track'), ('upnp:class', 'object.item.audioItem.musicTrack'), ('raumfeld:section', 'Tidal'), ('raumfeld:durability', '86400')]))]))]) XML returned by async_upnp_client: http://10.0.107.12:47366/raumfeldImage?albumId=bd9da4d2-cd16-4b5f-9e8d-2bbd0636c7b2&album=IIII&artist=Robin%20Schulz&service=TidalIIIIRobin SchulzRobin Schulztidal-track://169351013Dream (feat. colour your mind)13Trackobject.item.audioItem.musicTrackTidal86400 XML from async_upnp_client failed to convert: Traceback (most recent call last): File "/var/home/nobody/hassfeld/test7.py", line 23, in main() File "/var/home/nobody/hassfeld/test7.py", line 20, in main dict_ = xmltodict.parse(xml) File "/var/home/nobody/hassfeld/venv/lib64/python3.9/site-packages/xmltodict.py", line 327, in parse parser.Parse(xml_input, True) xml.parsers.expat.ExpatError: not well-formed (invalid token): line 2, column 558 ```

Result of https://xmllint.com/: InvalidChar in line: 10 Char '&' is not expected.

<?xml version="1.0"?>
<DIDL-Lite
    xmlns="urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/"
    xmlns:raumfeld="urn:schemas-raumfeld-com:meta-data/raumfeld"
    xmlns:upnp="urn:schemas-upnp-org:metadata-1-0/upnp/"
    xmlns:dlna="urn:schemas-dlna-org:metadata-1-0/"
    xmlns:dc="http://purl.org/dc/elements/1.1/"
    xmlns:pv="http://www.pv.com/pvns/" lang="en">
    <item parentID="0/Tidal/New/NewAlbums/169351000" id="0/Tidal/New/NewAlbums/169351000/169351007" restricted="1">
        <upnp:albumArtURI dlna:profileID="JPEG_TN">http://10.0.107.12:47366/raumfeldImage?albumId=bd9da4d2-cd16-4b5f-9e8d-2bbd0636c7b2&album=IIII&artist=Robin%20Schulz&service=Tidal</upnp:albumArtURI>
        <upnp:album>IIII</upnp:album>
        <upnp:artist>Robin Schulz</upnp:artist>
        <dc:creator>Robin Schulz</dc:creator>
        <res protocolInfo="tidal-track:*:audio/tidal-track:*" duration="0:03:03.000">tidal-track://169351007</res>
        <dc:title>Better with You (feat. SVRCINA)</dc:title>
        <upnp:originalTrackNumber>7</upnp:originalTrackNumber>
        <raumfeld:name>Track</raumfeld:name>
        <upnp:class>object.item.audioItem.musicTrack</upnp:class>
        <raumfeld:section>Tidal</raumfeld:section>
        <raumfeld:durability>86400</raumfeld:durability>
    </item>
</DIDL-Lite>
StevenLooman commented 3 years ago

Agreed, the test script should be able to parse the XML, so lets investigate. It might take some time before I can look into it however.

From what I can see the python-didl-lite receives a XML-string from this library and parses that via ElementTree/defusedxml, no special handling there.

B5r1oJ0A9G commented 3 years ago

Just for your information, I just made a small test with python-didl-lite and observe the same issue:

Code to test with hassfeld and didl_lite ```python import hassfeld import asyncio from didl_lite import didl_lite def main(): raumfeld = hassfeld.RaumfeldHost("10.0.107.12") raumfeld.start_update_thread() zone = ['Werkraum'] print("With upnpclient:\n") position_info = raumfeld.get_position_info(zone) xml = position_info["TrackMetaData"] didl_item = didl_lite.from_xml_string(xml) item = didl_item.pop() print(item.album_art_uri) print("\n\nwith async_upnp_client:\n") position_info = asyncio.run(raumfeld.async_get_position_info(zone)) xml = position_info["TrackMetaData"] didl_item = didl_lite.from_xml_string(xml) item = didl_item.pop() print(item.album_art_uri) main() ```
Output of test script ``` With upnpclient: http://10.0.107.12:47366/raumfeldImage?albumId=bd9da4d2-cd16-4b5f-9e8d-2bbd0636c7b2&album=IIII&artist=Robin%20Schulz&service=Tidal with async_upnp_client: Traceback (most recent call last): File "/usr/lib64/python3.9/xml/etree/ElementTree.py", line 1720, in feed self.parser.Parse(data, False) xml.parsers.expat.ExpatError: not well-formed (invalid token): line 2, column 558 During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/var/home/nobody/hassfeld/test9.py", line 25, in main() File "/var/home/nobody/hassfeld/test9.py", line 21, in main didl_item = didl_lite.from_xml_string(xml) File "/var/home/nobody/hassfeld/venv/lib64/python3.9/site-packages/didl_lite/didl_lite.py", line 916, in from_xml_string xml_el = defusedxml.ElementTree.fromstring(xml_string) File "/var/home/nobody/hassfeld/venv/lib64/python3.9/site-packages/defusedxml/common.py", line 131, in fromstring parser.feed(text) File "/usr/lib64/python3.9/xml/etree/ElementTree.py", line 1722, in feed self._raiseerror(v) File "/usr/lib64/python3.9/xml/etree/ElementTree.py", line 1629, in _raiseerror raise err xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 2, column 558 ```
StevenLooman commented 3 years ago

Thank you for trying. I did not expect different results as from the XML you're posted it was already clear it was not well formed.

One last thing to try, you can use upnp-client to read/get data from the device. Can you try for example something like this:

$ upnp-client subscribe http://192.168.178.19:1400/xml/device_description.xml AVT
{"timestamp": 1614630975.688071, "service_id": "urn:upnp-org:serviceId:AVTransport", "service_type": "urn:schemas-upnp-org:service:AVTransport:1", "state_variables": {"LastChange": "<Event xmlns=\"urn:schemas-upnp-org:metadata-1-0/AVT/\" xmlns:r=\"urn:schemas-rinconnetworks-com:metadata-1-0/\"><InstanceID val=\"0\"><TransportState val=\"PLAYING\"/><CurrentPlayMode val=\"NORMAL\"/><CurrentCrossfadeMode val=\"0\"/><NumberOfTracks val=\"1\"/><CurrentTrack val=\"1\"/><CurrentSection val=\"0\"/><CurrentTrackURI val=\"x-rincon-mp3radio://http://stream.player.arrow.nl/arrow\"/><CurrentTrackDuration val=\"0:00:00\"/><CurrentTrackMetaData val=\"&lt;DIDL-Lite xmlns:dc=&quot;http://purl.org/dc/elements/1.1/&quot; xmlns:upnp=&quot;urn:schemas-upnp-org:metadata-1-0/upnp/&quot; xmlns:r=&quot;urn:schemas-rinconnetworks-com:metadata-1-0/&quot; xmlns=&quot;urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/&quot;&gt;&lt;item id=&quot;-1&quot; parentID=&quot;-1&quot; restricted=&quot;true&quot;&gt;&lt;res protocolInfo=&quot;sonos.com-http:*:*:*&quot;&gt;x-sonosapi-stream:s6702?sid=254&amp;amp;flags=8224&amp;amp;sn=0&lt;/res&gt;&lt;r:streamContent&gt;DUNCAN BROWNE - WILD PLACES&lt;/r:streamContent&gt;&lt;r:radioShowMd&gt;Rock of Ages,p422411&lt;/r:radioShowMd&gt;&lt;upnp:albumArtURI&gt;/getaa?s=1&amp;amp;u=x-sonosapi-stream%3as6702%3fsid%3d254%26flags%3d8224%26sn%3d0&lt;/upnp:albumArtURI&gt;&lt;dc:title&gt;x-sonosapi-stream:s6702?sid=254&amp;amp;flags=8224&amp;amp;sn=0&lt;/dc:title&gt;&lt;upnp:class&gt;object.item&lt;/upnp:class&gt;&lt;/item&gt;&lt;/DIDL-Lite&gt;\"/><r:NextTrackURI val=\"\"/><r:NextTrackMetaData val=\"\"/><r:EnqueuedTransportURI val=\"x-sonosapi-stream:s6702?sid=254&amp;flags=8224&amp;sn=0\"/><r:EnqueuedTransportURIMetaData val=\"&lt;DIDL-Lite xmlns:dc=&quot;http://purl.org/dc/elements/1.1/&quot; xmlns:upnp=&quot;urn:schemas-upnp-org:metadata-1-0/upnp/&quot; xmlns:r=&quot;urn:schemas-rinconnetworks-com:metadata-1-0/&quot; xmlns=&quot;urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/&quot;&gt;&lt;item id=&quot;-1&quot; parentID=&quot;-1&quot; restricted=&quot;true&quot;&gt;&lt;dc:title&gt;Arrow Classic Rock&lt;/dc:title&gt;&lt;upnp:class&gt;object.item.audioItem.audioBroadcast&lt;/upnp:class&gt;&lt;desc id=&quot;cdudn&quot; nameSpace=&quot;urn:schemas-rinconnetworks-com:metadata-1-0/&quot;&gt;SA_RINCON65031_&lt;/desc&gt;&lt;/item&gt;&lt;/DIDL-Lite&gt;\"/><PlaybackStorageMedium val=\"NETWORK\"/><AVTransportURI val=\"x-sonosapi-stream:s6702?sid=254&amp;flags=8224&amp;sn=0\"/><AVTransportURIMetaData val=\"&lt;DIDL-Lite xmlns:dc=&quot;http://purl.org/dc/elements/1.1/&quot; xmlns:upnp=&quot;urn:schemas-upnp-org:metadata-1-0/upnp/&quot; xmlns:r=&quot;urn:schemas-rinconnetworks-com:metadata-1-0/&quot; xmlns=&quot;urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/&quot;&gt;&lt;item id=&quot;-1&quot; parentID=&quot;-1&quot; restricted=&quot;true&quot;&gt;&lt;dc:title&gt;Arrow Classic Rock&lt;/dc:title&gt;&lt;upnp:class&gt;object.item.audioItem.audioBroadcast&lt;/upnp:class&gt;&lt;desc id=&quot;cdudn&quot; nameSpace=&quot;urn:schemas-rinconnetworks-com:metadata-1-0/&quot;&gt;SA_RINCON65031_&lt;/desc&gt;&lt;/item&gt;&lt;/DIDL-Lite&gt;\"/><NextAVTransportURI val=\"\"/><NextAVTransportURIMetaData val=\"\"/><CurrentTransportActions val=\"Set, Stop, Play\"/><r:CurrentValidPlayModes val=\"\"/><r:MuseSessions val=\"\"/><r:DirectControlClientID val=\"\"/><r:DirectControlIsSuspended val=\"0\"/><r:DirectControlAccountID val=\"\"/><TransportStatus val=\"OK\"/><r:SleepTimerGeneration val=\"0\"/><r:AlarmRunning val=\"0\"/><r:SnoozeRunning val=\"0\"/><r:RestartPending val=\"0\"/><TransportPlaySpeed val=\"NOT_IMPLEMENTED\"/><CurrentMediaDuration val=\"NOT_IMPLEMENTED\"/><RecordStorageMedium val=\"NOT_IMPLEMENTED\"/><PossiblePlaybackStorageMedia val=\"NONE, NETWORK\"/><PossibleRecordStorageMedia val=\"NOT_IMPLEMENTED\"/><RecordMediumWriteStatus val=\"NOT_IMPLEMENTED\"/><CurrentRecordQualityMode val=\"NOT_IMPLEMENTED\"/><PossibleRecordQualityModes val=\"NOT_IMPLEMENTED\"/></InstanceID></Event>"}}

The example above is from a Ikea Tradfri (Sonos) speaker playing a stream. upnp-client subscribes to the AVTransport service and listens for events. The xml from the example can be parsed without any problems by xmltodict.parse().

This is mostly to be sure that the problem lies within async_upnp_client and not with your device.

B5r1oJ0A9G commented 3 years ago

As far as I can tell, it works well.

Script:

import xmltodict

def main():
    upnp_client_output = {"timestamp": 1614634581.7702954, "service_id": "urn:upnp-org:serviceId:AVTransport", "service_type": "urn:schemas-upnp-org:service:AVTransport:1", "state_variables": {"CurrentTrackMetaData": "<?xml version=\"1.0\"?> <DIDL-Lite xmlns=\"urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/\" xmlns:raumfeld=\"urn:schemas-raumfeld-com:meta-data/raumfeld\" xmlns:upnp=\"urn:schemas-upnp-org:metadata-1-0/upnp/\" xmlns:dlna=\"urn:schemas-dlna-org:metadata-1-0/\" xmlns:dc=\"http://purl.org/dc/elements/1.1/\" xmlns:pv=\"http://www.pv.com/pvns/\" lang=\"en\"><item parentID=\"0/Tidal/Moods/party/5fe26a31-b468-4283-9682-7fef913ce578\" id=\"0/Tidal/Moods/party/5fe26a31-b468-4283-9682-7fef913ce578/151148050\" restricted=\"1\"><upnp:albumArtURI dlna:profileID=\"JPEG_TN\">http://10.0.107.12:47366/raumfeldImage?albumId=64782f38-86ab-4b56-abfa-434552c758b3&amp;album=More%20Life%20%28feat.%20Tinie%20Tempah%20%26%20L%20Devine%29&amp;artist=Torren%20Foot&amp;service=Tidal</upnp:albumArtURI><upnp:album>More Life (feat. Tinie Tempah &amp; L Devine)</upnp:album><upnp:artist>Torren Foot</upnp:artist><dc:creator>Torren Foot</dc:creator><res protocolInfo=\"tidal-track:*:audio/tidal-track:*\" duration=\"0:02:32.000\">tidal-track://151148050</res><dc:title>More Life (feat. Tinie Tempah &amp; L Devine)</dc:title><upnp:originalTrackNumber>1</upnp:originalTrackNumber><raumfeld:name>Track</raumfeld:name><upnp:class>object.item.audioItem.musicTrack</upnp:class><raumfeld:section>Tidal</raumfeld:section><raumfeld:durability>86400</raumfeld:durability></item></DIDL-Lite> ", "CurrentRecordQualityMode": "NOT_IMPLEMENTED", "AbsoluteTimePosition": "00:02:21", "SecondsUntilSleep": 0, "CurrentTrack": 3, "AVTransportURIMetaData": "<?xml version=\"1.0\"?> <DIDL-Lite xmlns=\"urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/\" xmlns:dc=\"http://purl.org/dc/elements/1.1/\" xmlns:dlna=\"urn:schemas-dlna-org:metadata-1-0/\" xmlns:upnp=\"urn:schemas-upnp-org:metadata-1-0/upnp/\" xmlns:raumfeld=\"urn:schemas-raumfeld-com:meta-data/raumfeld\" xmlns:pv=\"http://www.pv.com/pvns/\"><container parentID=\"0/Tidal/Moods/party\" restricted=\"1\" id=\"0/Tidal/Moods/party/5fe26a31-b468-4283-9682-7fef913ce578\"><upnp:class>object.container.playlistContainer</upnp:class><raumfeld:section>Tidal</raumfeld:section><dc:title>Electronic Party Mix</dc:title><upnp:albumArtURI>http://10.0.107.12:47366/raumfeldImage?playlistId=playlist.c5677293-b052-47c1-8a36-58adec86e5fe&amp;service=Tidal</upnp:albumArtURI><raumfeld:durability>86400</raumfeld:durability><raumfeld:name>Playlist</raumfeld:name><raumfeld:durability>86400</raumfeld:durability></container></DIDL-Lite> ", "PossiblePlaybackStorageMedia": "NETWORK", "TransportPlaySpeed": "1", "CurrentTrackDuration": "00:02:32", "PossibleRecordQualityModes": "NOT_IMPLEMENTED", "PossibleRecordStorageMedia": "NONE", "AVTransportURI": "dlna-playcontainer://uuid%3A26cf0c06-7bef-4f45-9d1e-6f5e87b37c3a?sid=urn%3Aupnp-org%3AserviceId%3AContentDirectory&cid=0%2FTidal%2FMoods%2Fparty%2F5fe26a31-b468-4283-9682-7fef913ce578&md=0", "RelativeTimePosition": "00:02:21", "RelativeCounterPosition": 1, "CurrentPlayMode": "NORMAL", "TransportState": "PLAYING", "AbsoluteCounterPosition": 1, "CurrentTransportActions": "Pause,Stop,Previous,Next,Seek,RepeatTrack,Repeat,Shuffle", "RoomStates": "uuid:664bfc30-623b-4621-9ffc-5b59700a4638=PLAYING", "NumberOfTracks": 132, "SleepTimerActive": False, "TransportStatus": "OK", "CurrentTrackURI": "https://ab-pr-fa.audio.tidal.com/ff10d1b1d586200a863d3958fd6d45e0_37.mp4"}}

    xml = upnp_client_output["state_variables"]["CurrentTrackMetaData"]
    dict_ = xmltodict.parse(xml)
    print(dict_["DIDL-Lite"]["item"]["upnp:albumArtURI"]["#text"])

main()

Output:

http://10.0.107.12:47366/raumfeldImage?albumId=64782f38-86ab-4b56-abfa-434552c758b3&album=More%20Life%20%28feat.%20Tinie%20Tempah%20%26%20L%20Devine%29&artist=Torren%20Foot&service=Tidal
StevenLooman commented 3 years ago

I am (still) a bit baffled. When using the dlna_dmr component in Home Assistant this library (and python-didl-lite) is used to communicate with the device. As far as I can tell the received data is not handled differently than other state variables, thus escaped. When I use the dlna_dmr component myself I see no errors and see the title of the currently playing track, thus the XML is parsed without problems.

Why then are you unable to parse it? Does your device not behave according to the UPnP spec?

Still, I'll try to add a work-around tonight, or this weekend otherwise.

StevenLooman commented 3 years ago

Did a test of my own:

#!/usr/bin/env python3

import asyncio
import xmltodict

from async_upnp_client import UpnpDevice
from async_upnp_client import UpnpFactory
from async_upnp_client.aiohttp import AiohttpRequester

async def main():
    requester = AiohttpRequester(4)
    factory = UpnpFactory(requester)
    device = await factory.async_create_device("http://192.168.178.19:1400/xml/device_description.xml")
    service = device.services["urn:schemas-upnp-org:service:AVTransport:1"]
    action = service.action("GetPositionInfo")
    result = await action.async_call(InstanceID=0)
    xml = result["TrackMetaData"]
    print(f"XML returned by async_upnp_client:\n\n{xml}")
    dict_ = xmltodict.parse(xml)
    print(f"\n\n\nDict: {dict_}")

asyncio.run(main())

Results:

$ ./test.py
XML returned by async_upnp_client:
<DIDL-Lite xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:upnp="urn:schemas-upnp-org:metadata-1-0/upnp/" xmlns:r="urn:schemas-rinconnetworks-com:metadata-1-0/" xmlns="urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/"><item id="-1" parentID="-1" restricted="true"><res protocolInfo="x-rincon-mp3radio:*:*:*">x-rincon-mp3radio://http://stream.player.arrow.nl/arrow</res><r:streamContent>ARROW - CLASSIC ROCK</r:streamContent><dc:title>arrow</dc:title><upnp:class>object.item</upnp:class></item></DIDL-Lite>

Dict:
OrderedDict([('DIDL-Lite', OrderedDict([('@xmlns:dc', 'http://purl.org/dc/elements/1.1/'), ('@xmlns:upnp', 'urn:schemas-upnp-org:metadata-1-0/upnp/'), ('@xmlns:r', 'urn:schemas-rinconnetworks-com:metadata-1-0/'), ('@xmlns', 'urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/'), ('item', OrderedDict([('@id', '-1'), ('@parentID', '-1'), ('@restricted', 'true'), ('res', OrderedDict([('@protocolInfo', 'x-rincon-mp3radio:*:*:*'), ('#text', 'x-rincon-mp3radio://http://stream.player.arrow.nl/arrow')])), ('r:streamContent', 'ARROW - CLASSIC ROCK'), ('dc:title', 'arrow'), ('upnp:class', 'object.item')]))]))])

I'm still convinced it's either the device itself which misbehaves, or something else is going on in teufel_raumfeld.

I've omitted a aiohttp-warning from the the run-result, not important:

/home/steven/src/python/async_upnp_client/.venv/lib/python3.9/site-packages/aiohttp-4.0.0a1-py3.9-linux-x86_64.egg/aiohttp/client.py:977: RuntimeWarning: coroutine 'noop' was never awaited
  self._resp.release()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
B5r1oJ0A9G commented 3 years ago

If I haven't overlooked something, then there is no "&" in your DIDL object that could be falsely unescaped and therefore lead to issues when interpreting as XML.

Running this code (example script adapted for GetMediaInfo action and to parse the DIDL-Lite XML object): ``` #!/usr/bin/env python import asyncio import logging import xmltodict from async_upnp_client import UpnpFactory from async_upnp_client.aiohttp import AiohttpRequester logging.basicConfig(level=logging.INFO) target = 'http://10.0.107.12:60810/a06bc8d9-d4e8-47a1-aa37-e1f251f13c2c.xml' async def main(): # create the factory requester = AiohttpRequester() factory = UpnpFactory(requester) # create a device device = await factory.async_create_device(target) print("Device: {}".format(device)) # get AVTransport-service service = device.service('urn:schemas-upnp-org:service:AVTransport:1') print("Service: {}".format(service)) # perform GetMediainfo action get_volume = service.action('GetMediaInfo') print("Action: {}".format(get_volume)) result = await get_volume.async_call(InstanceID=0, Channel='Master') print("Action result: {}".format(result)) # convert XML to Dict dict_ = xmltodict.parse(result["CurrentURIMetaData"]) print(dict_) loop = asyncio.get_event_loop() loop.run_until_complete(main()) ```
Fails because of having the escaped values returned from the device unescaped ("&"): ``` Device: Service: Action: Action result: {'NrTracks': 300, 'MediaDuration': 'NOT_IMPLEMENTED', 'CurrentURI': 'dlna-playcontainer://uuid%3A26cf0c06-7bef-4f45-9d1e-6f5e87b37c3a?sid=urn%3Aupnp-org%3AserviceId%3AContentDirectory&cid=0%2FTidal%2FDirectAccess%2FArtist%2F945%2FTopTracks&fid=0&fii=0', 'CurrentURIMetaData': '\nobject.container.person.musicArtistTidalhttp://10.0.107.12:47366/raumfeldImage?artistId=&artist=AC%2FDC&service=TidalAC/DC86400Artistdlna-playcontainer://uuid%3A26cf0c06-7bef-4f45-9d1e-6f5e87b37c3a?sid=urn%3Aupnp-org%3AserviceId%3AContentDirectory&cid=0%2FTidal%2FDirectAccess%2FArtist%2F945%2FTopTracks&fid=0&fii=086400\n', 'NextURI': 'NOT_IMPLEMENTED', 'NextURIMetaData': 'NOT_IMPLEMENTED', 'PlayMedium': 'NETWORK', 'RecordMedium': 'NOT_IMPLEMENTED', 'WriteStatus': 'NOT_IMPLEMENTED'} Traceback (most recent call last): File "/var/home/nobody/hassfeld/test10.py", line 41, in loop.run_until_complete(main()) File "/usr/lib64/python3.9/asyncio/base_events.py", line 642, in run_until_complete return future.result() File "/var/home/nobody/hassfeld/test10.py", line 37, in main dict_ = xmltodict.parse(result["CurrentURIMetaData"]) File "/var/home/nobodyhassfeld/venv/lib64/python3.9/site-packages/xmltodict.py", line 327, in parse parser.Parse(xml_input, True) xml.parsers.expat.ExpatError: not well-formed (invalid token): line 2, column 622 ```

After applying the following patch to async_upnp_client (to keept the values returned form the device escaped as they are and should be):

--- async_upnp_client/client.py.orig    2021-03-03 23:41:48.920006081 +0100
+++ async_upnp_client/client.py 2021-03-03 23:53:19.377029314 +0100
@@ -600,7 +600,8 @@
                                 (name, ET.tostring(xml, encoding='unicode')))

             try:
-                arg.upnp_value = unescape(arg_xml.text or '')
+                #arg.upnp_value = unescape(arg_xml.text or '')
+                arg.upnp_value = arg_xml.text or ''
             except AttributeError:
                 _LOGGER.debug("Error during unescape of: %s", arg_xml.text)
                 raise
It is working as expected (since "&" remains escaped as returned from the device): ``` Device: Service: Action: Action result: {'NrTracks': 300, 'MediaDuration': 'NOT_IMPLEMENTED', 'CurrentURI': 'dlna-playcontainer://uuid%3A26cf0c06-7bef-4f45-9d1e-6f5e87b37c3a?sid=urn%3Aupnp-org%3AserviceId%3AContentDirectory&cid=0%2FTidal%2FDirectAccess%2FArtist%2F945%2FTopTracks&fid=0&fii=0', 'CurrentURIMetaData': '\nobject.container.person.musicArtistTidalhttp://10.0.107.12:47366/raumfeldImage?artistId=&artist=AC%2FDC&service=TidalAC/DC86400Artistdlna-playcontainer://uuid%3A26cf0c06-7bef-4f45-9d1e-6f5e87b37c3a?sid=urn%3Aupnp-org%3AserviceId%3AContentDirectory&cid=0%2FTidal%2FDirectAccess%2FArtist%2F945%2FTopTracks&fid=0&fii=086400\n', 'NextURI': 'NOT_IMPLEMENTED', 'NextURIMetaData': 'NOT_IMPLEMENTED', 'PlayMedium': 'NETWORK', 'RecordMedium': 'NOT_IMPLEMENTED', 'WriteStatus': 'NOT_IMPLEMENTED'} OrderedDict([('DIDL-Lite', OrderedDict([('@xmlns', 'urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/'), ('@xmlns:dc', 'http://purl.org/dc/elements/1.1/'), ('@xmlns:dlna', 'urn:schemas-dlna-org:metadata-1-0/'), ('@xmlns:upnp', 'urn:schemas-upnp-org:metadata-1-0/upnp/'), ('@xmlns:raumfeld', 'urn:schemas-raumfeld-com:meta-data/raumfeld'), ('@xmlns:pv', 'http://www.pv.com/pvns/'), ('container', OrderedDict([('@parentID', '0/Favorites/RecentlyPlayed'), ('@restricted', '1'), ('@refID', '0/Tidal/DirectAccess/Artist/945'), ('@id', '0/Favorites/RecentlyPlayed/13474'), ('upnp:class', 'object.container.person.musicArtist'), ('raumfeld:section', 'Tidal'), ('upnp:albumArtURI', 'http://10.0.107.12:47366/raumfeldImage?artistId=&artist=AC%2FDC&service=Tidal'), ('dc:title', 'AC/DC'), ('raumfeld:durability', ['86400', '86400']), ('raumfeld:name', 'Artist'), ('res', OrderedDict([('@protocolInfo', 'dlna-playcontainer:*:application/xml:*'), ('#text', 'dlna-playcontainer://uuid%3A26cf0c06-7bef-4f45-9d1e-6f5e87b37c3a?sid=urn%3Aupnp-org%3AserviceId%3AContentDirectory&cid=0%2FTidal%2FDirectAccess%2FArtist%2F945%2FTopTracks&fid=0&fii=0')]))]))]))]) ```

If I don't misunderstand, then the unescape() will always lead to break XML objects, resp. make them unparsable, if the object returned from the device contains values expected to be escaped.

StevenLooman commented 3 years ago

Indeed, but still I'm think it is up to the device to properly handle this. I've tried several sources to have the &, but to no avail.

Nevertheless, I've added raw_upnp_value to Argument. Does this help?

B5r1oJ0A9G commented 3 years ago

I cannot tell yet as I haven't figured out how to make use of it. Can you please give me a hint, perhaps based on the recent code examle in https://github.com/StevenLooman/async_upnp_client/issues/50#issuecomment-790141649?

StevenLooman commented 3 years ago

The value is stored at the Argument for conversion of the received string to the Python type.

Now that I see this code, perhaps it should have been done in a different way, but this is the way it is now.

Example code:

#!/usr/bin/env python3

import asyncio

import xmltodict

from async_upnp_client import UpnpFactory
from async_upnp_client.aiohttp import AiohttpRequester

async def main():
    requester = AiohttpRequester(4)
    factory = UpnpFactory(requester)
    device = await factory.async_create_device("http://192.168.178.19:1400/xml/device_description.xml")
    service = device.services["urn:schemas-upnp-org:service:AVTransport:1"]
    action = service.action("GetMediaInfo")
    await action.async_call(InstanceID=0)
    xml = action.argument('CurrentURIMetaData').raw_upnp_value
    print(f"XML returned by async_upnp_client:\n{xml}")
    dict_ = xmltodict.parse(xml)
    print(f"\n\nDict:\n{dict_}")

asyncio.run(main())
B5r1oJ0A9G commented 3 years ago

Many thanks! I can confirm that this resolves this issue for me!

StevenLooman commented 3 years ago

Great! Do you want a release already, or can it wait? I plan to add some new functionality related to advertisements.

B5r1oJ0A9G commented 3 years ago

There is no urgency. The next release of hassfeld and teufel_raumfeld depend on it though. if it would not cause any significant inconvenience, it would be very welcomed - but it can also wait a bit :)

StevenLooman commented 3 years ago

Please let me know when you are ready to release. If by then I haven't added my new functionality, I'll do an extra release so you can keep going.

B5r1oJ0A9G commented 3 years ago

@StevenLooman,

I am ready to release. If your offer is still valid, I would like to accept it :)

StevenLooman commented 3 years ago

Is it okay to wait for #54? I also have some SSDP-related changes which I would like to add.

B5r1oJ0A9G commented 3 years ago

I had hoped to be able to continue working on my project already this weekend. However, I don't want to be pushy. In worst case I can also roll back the recent integration with async_upnp_client and continue without - to be re-implement again, when available.

StevenLooman commented 3 years ago

My change is done (#55).

@elupus, do you have an expectation when you can finish #54?

StevenLooman commented 3 years ago

I've added some changes to #54, resolving the review-comments.

Testing could/should be done some more, but I'll create a release in the mean time so you can continue (and indirectly also test.) Any new issues from testing will be fixed in a future version.

elupus commented 3 years ago

I'm good with it as now if you are.

Den lör 13 mars 2021 17:14Steven Looman @.***> skrev:

My change is done (#55 https://github.com/StevenLooman/async_upnp_client/pull/55).

@elupus https://github.com/elupus, do you have an expectation when you can finish #54 https://github.com/StevenLooman/async_upnp_client/pull/54 ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/StevenLooman/async_upnp_client/issues/50#issuecomment-798570349, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADV5ICHJA7KHIMNQAAMM4DTDOFKTANCNFSM4YKPKIVQ .

StevenLooman commented 3 years ago

I have released async_upnp_client==0.15.0

B5r1oJ0A9G commented 3 years ago

Awesome, thanks a lot!

Unfortunately I had to find out that there are version constraints for some packages in Home Assistent preventing the update of async_upnp_client. Need to see how to work-around this for now...

Anyhow, many thanks for your swift support and the quick release!

StevenLooman commented 3 years ago

Those will probably be from upnp and dlna_dmr, both are maintained by me. I think you can include this in your PR and tag me. Upgrading should not cause any problems for those components.

This does point to a future issue though. I think it is great that more components are using async_upnp_client, writing additional libraries providing the same functionality is wasteful. The problem might be that different components 'lock' to a specific version. Not sure Home Assistant would/should handle this. Feel free to address this.

chishm commented 3 years ago

Hi, Sorry to bump an old issue, but I think this is still actually a bug. The cause is that defusedxml is unescaping the response contents already, in the call xml = DET.fromstring(response_body) in UpnpAction.parse_response. The workaround works for me too, but it took me a while to track this issue down while trying to debug it.

Here's an example action call, with the raw request and response debug messages:

logger output ``` DEBUG:async_upnp_client.traffic.upnp:Sending request: POST http://--redacted-host--/ctl/ContentDir SOAPAction: "urn:schemas-upnp-org:service:ContentDirectory:1#Browse" Host: --redacted-host-- Content-Type: text/xml; charset="utf-8" Content-Length: 439 1$7$1AE BrowseMetadata * 0 0 DEBUG:async_upnp_client.traffic.upnp:Got response: 200 Content-Type: text/xml; charset="utf-8" Connection: close Content-Length: 1248 Server: Raspbian DLNADOC/1.50 UPnP/1.0 MiniDLNA/1.2.1 Date: Thu, 01 Apr 2021 11:34:07 GMT EXT: <DIDL-Lite xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:upnp="urn:schemas-upnp-org:metadata-1-0/upnp/" xmlns="urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/" xmlns:dlna="urn:schemas-dlna-org:metadata-1-0/"> <container id="1$7$1AE" parentID="1$7" restricted="1" searchable="1" childCount="20"><dc:title>Janet Jackson's Rhythm Nation 1814</dc:title><upnp:class>object.container.album.musicAlbum</upnp:class><upnp:storageUsed>-1</upnp:storageUsed><dc:creator>Janet Jackson</dc:creator><upnp:genre>R&amp;B</upnp:genre><upnp:artist>Janet Jackson</upnp:artist><upnp:albumArtURI dlna:profileID="JPEG_TN" xmlns:dlna="urn:schemas-dlna-org:metadata-1-0/">http://--redacted-host--/AlbumArt/1144-7943.jpg</upnp:albumArtURI></container></DIDL-Lite> 1 1 1994 ```

The important part is that the Envelope->BrowseResponse->Result is escaped didl-lite, and within that is:

&lt;upnp:genre&gt;R&amp;amp;B&lt;/upnp:genre&gt;

The DET.fromstring call will unescape this part once, giving the still valid XML:

<upnp:genre>R&amp;B</upnp:genre>

The call to arg.upnp_value = unescape(arg_xml.text or "") in _parse_response_args does another layer of unescaping that isn't necessary at this point, giving invalid XML:

<upnp:genre>R&B</upnp:genre>

This stops the XML from being reliably parsed later with didl_lite.didl_lite.from_xml_string.

So I think that DET.fromstring is correctly unescaping the response already, and the call to unescape is not needed (and potentially harmful).

StevenLooman commented 3 years ago

Hi @chishm, No problem to bump this, in fact I'd rather have this properly fixed. I did some testing myself with Kodi as the DMR and a IKEA Symfonisk speaker, and all seemed to work as expected.

Let me try again later today. Your clear analysis gives me something to compare against, so thank you for this.

StevenLooman commented 3 years ago

You are right, the XML shouldn't be escaped again. Thank you for pointing this out. I've fixed it in e76f7f3296c8351106625b6d013e0620fdf16f7e.

@B5r1oJ0A9G @chishm Do you want a release for this?

StevenLooman commented 3 years ago

Note that I've kept the raw_upnp_value property for backwards compatibility.

chishm commented 3 years ago

Thank you for re-examining this, @StevenLooman. I'm glad my bug hunting helped.

I've grabbed the latest development branch to test and can confirm the issue is fixed for me. I'm in no rush for a release, as what I'm working on is still a work in progress.

B5r1oJ0A9G commented 3 years ago

@StevenLooman, as far as I am concerned, there is no urgency neither. Once the new release is considered in HA too, I can revert the modifications made to use raw_upnp_value.

StevenLooman commented 3 years ago

Great. In case you change either of you change your mind, let me know.