mopidy / mopidy-soundcloud

Mopidy extension for playing music from SoundCloud
https://mopidy.com/ext/soundcloud/
MIT License
185 stars 59 forks source link

client crashes when artist's name contains unicode #103

Closed klntsky closed 4 years ago

klntsky commented 5 years ago

Steps to reproduce:

  1. follow this artist
  2. go to soundcloud/following

I've checked it with two different MPD clients, so I suppose it should be reported here.

klntsky commented 5 years ago

Looks like a parsing issue somewhere. Iris shows this instead of the artist's name:

this

klntsky commented 5 years ago

The problem is that safe_url(name) here returns empty string, because we're ignoring all non-ASCII.

Actually I don't see any reason to do so. I've patched the code like this:

 def new_folder(name, path):
     return models.Ref.directory(
         uri=generate_uri(path),
-        name=safe_url(name)
+        name=name
     )

and it works on my machine, and artist names are no more altered by quote_plus. From my point of view, things are oversanitized here.

As stated in the docs, mopidy.models.Ref is a

Model to represent URI references with a human friendly name and type attached.

"Human fiendly" most likely means that it may be any unicode string.

klntsky commented 5 years ago

Some more links to follow for those who try to resolve this issue:

One:

    def __init__(self, default=None):
        # TODO: normalize to unicode?
        # TODO: only allow unicode?
        # TODO: disallow empty strings?

Two:

 string_types = basestring # noqa

From these I conclude that passing artist names exactly as returned by soundcloud API is absolutely OK, and all possible issues are either on mopidy.models' side or on the side of a particular client.

jodal commented 4 years ago

I agree with the conclusion here. Fixed by #112.