vmichalak / sonos-controller

Java API for controlling SONOS players
MIT License
43 stars 9 forks source link

Escape special XML characters #7

Closed mguntli closed 7 years ago

mguntli commented 7 years ago

While I was trying to start a playlist from Google Play Music, I noticed that it didn't work because the special characters were not escaped. <, >, &, " and ' all have special meanings in XML and have to be represented by entities.

Example from Google Play Music: sonosDevice.playUri("x-sonosapi-radio:vy_wPybY9vRCoaCMySC_D6Ol9WJ1aoH97kTmNIzBZ667Frppf2koUq7ZH7OJ9bXx41s_K0RZBSA?sid=151&flags=8300&sn=3", null);

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-2.3%) to 80.712% when pulling c42a8a7cbb92f8a4b7559f290c89e2406d64bc35 on mguntli:soap-escaping into cfda5a8f7c6e62f1c737a941547ea0a887d5f43f on vmichalak:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 82.965% when pulling 7061f1b4baace05aba78df110be0c337096ef8c4 on mguntli:soap-escaping into cfda5a8f7c6e62f1c737a941547ea0a887d5f43f on vmichalak:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 82.965% when pulling 7061f1b4baace05aba78df110be0c337096ef8c4 on mguntli:soap-escaping into cfda5a8f7c6e62f1c737a941547ea0a887d5f43f on vmichalak:master.

mguntli commented 7 years ago

Sorry was too early with pushing: UrlEncoder does the wrong escaping! --> maybe I should have written a UnitTest for it ;-)

vmichalak commented 7 years ago

Write a unit test that reproduces your problem please. It will be better for maintenance in future.

mguntli commented 7 years ago

How do you paste your URI? Because if they are already escaped, it will break the system as well.. I retrieve the playlists URI from my Sonos Favorites with https://github.com/jishi/Jishi.Intel.SonosUPnP

vmichalak commented 7 years ago

For the moment with playURI method i only use mp3 or radio feed, and it works well ;)

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-3.4%) to 79.532% when pulling f04a59a33d1ef82010059e73ce55b821bfb21f15 on mguntli:soap-escaping into cfda5a8f7c6e62f1c737a941547ea0a887d5f43f on vmichalak:master.

mguntli commented 7 years ago

Hi Valentin

Current approach: detect if the string is already escaped, if not escape the XML characters. https://commons.apache.org/proper/commons-lang/javadocs/api-2.6/org/apache/commons/lang/StringEscapeUtils.html#escapeXml(java.lang.String)

I will write a corresponding unit test for it with the following test cases:

Expected result: x-sonosapi-radio:vy_wPy0RZBSA?sid=151&amp;flags=8300&amp;sn=3

vmichalak commented 7 years ago

I think i gonna add org.apache.commons.commons-text dependency for this problem. It's more simple.

mguntli commented 7 years ago

I agree, my solution is kind of "self-made" since I didn't want to include any external dependency into your library. Please keep in mind that you don't double escape already escaped characters (e.g. "&" should not result in "&amp;"

vmichalak commented 7 years ago

It's done on master ! Thanks for your help 👍