From 2f46d83834002c05cda42e543f457c5130984228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= Date: Thu, 30 Aug 2018 14:01:44 +0200 Subject: [PATCH 1/3] subsonic: Catch ValueError when casting input parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed cast to int will raise ValueError, which is not currently caught by the error checking code, leading to a crash. Fix this so a proper error message can be returned. Also add test for getting artist with non-numeric ID. Signed-off-by: Toke Høiland-Jørgensen --- api/funkwhale_api/subsonic/views.py | 2 +- api/tests/subsonic/test_views.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index bb5f44166..de7284cd6 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -38,7 +38,7 @@ def find_object(queryset, model_field="pk", field="id", cast=int): ) try: value = cast(raw_value) - except (TypeError, ValidationError): + except (ValueError, TypeError, ValidationError): return response.Response( { "error": { diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index b7431efab..d9b50d3eb 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -102,6 +102,17 @@ def test_get_artist(f, db, logged_in_api_client, factories): assert response.data == expected +@pytest.mark.parametrize("f", ["xml", "json"]) +def test_get_invalid_artist(f, db, logged_in_api_client, factories): + url = reverse("api:subsonic-get-artist") + assert url.endswith("getArtist") is True + expected = {"error": {"code": 0, "message": 'For input string "asdf"'}} + response = logged_in_api_client.get(url, {"id": "asdf"}) + + assert response.status_code == 200 + assert response.data == expected + + @pytest.mark.parametrize("f", ["xml", "json"]) def test_get_artist_info2(f, db, logged_in_api_client, factories): url = reverse("api:subsonic-get-artist-info2") From 8193782f682a9f99fec634b6dc8603133cc00ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= Date: Thu, 30 Aug 2018 14:03:56 +0200 Subject: [PATCH 2/3] subsonic: Don't crash when serialising artist with no name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the name of an artist is not set, the serialiser will crash. Instead, just skip such an artist when serialising a list of artists. Also add test for serialising an artist with an empty name. Signed-off-by: Toke Høiland-Jørgensen --- api/funkwhale_api/subsonic/serializers.py | 3 ++- api/tests/subsonic/test_serializers.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/api/funkwhale_api/subsonic/serializers.py b/api/funkwhale_api/subsonic/serializers.py index fc21a99f2..a340a1aad 100644 --- a/api/funkwhale_api/subsonic/serializers.py +++ b/api/funkwhale_api/subsonic/serializers.py @@ -24,7 +24,8 @@ class GetArtistsSerializer(serializers.Serializer): first_letter_mapping = collections.defaultdict(list) for artist in values: - first_letter_mapping[artist["name"][0].upper()].append(artist) + if artist["name"]: + first_letter_mapping[artist["name"][0].upper()].append(artist) for letter, artists in sorted(first_letter_mapping.items()): letter_data = { diff --git a/api/tests/subsonic/test_serializers.py b/api/tests/subsonic/test_serializers.py index 6fdf02e2d..bd07008df 100644 --- a/api/tests/subsonic/test_serializers.py +++ b/api/tests/subsonic/test_serializers.py @@ -6,6 +6,7 @@ def test_get_artists_serializer(factories): artist1 = factories["music.Artist"](name="eliot") artist2 = factories["music.Artist"](name="Ellena") artist3 = factories["music.Artist"](name="Rilay") + artist4 = factories["music.Artist"](name="") # Shouldn't be serialised factories["music.Album"].create_batch(size=3, artist=artist1) factories["music.Album"].create_batch(size=2, artist=artist2) @@ -28,7 +29,7 @@ def test_get_artists_serializer(factories): } queryset = artist1.__class__.objects.filter( - pk__in=[artist1.pk, artist2.pk, artist3.pk] + pk__in=[artist1.pk, artist2.pk, artist3.pk, artist4.pk] ) assert serializers.GetArtistsSerializer(queryset).data == expected From 7d9220ed49550acbcca171d69eafa027dc4d19a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= Date: Thu, 30 Aug 2018 14:04:41 +0200 Subject: [PATCH 3/3] subsonic: Implement getSong API endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The getSong API endpoint is used by, e.g., the subsonic plugin to mopidy. Signed-off-by: Toke Høiland-Jørgensen --- api/funkwhale_api/subsonic/serializers.py | 8 ++++++++ api/funkwhale_api/subsonic/views.py | 9 +++++++++ api/tests/subsonic/test_views.py | 14 ++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/api/funkwhale_api/subsonic/serializers.py b/api/funkwhale_api/subsonic/serializers.py index a340a1aad..5308146e1 100644 --- a/api/funkwhale_api/subsonic/serializers.py +++ b/api/funkwhale_api/subsonic/serializers.py @@ -130,6 +130,14 @@ class GetAlbumSerializer(serializers.Serializer): return payload +class GetSongSerializer(serializers.Serializer): + def to_representation(self, track): + tf = track.files.all() + if not len(tf): + return {} + return get_track_data(track.album, track, tf[0]) + + def get_starred_tracks_data(favorites): by_track_id = {f.track_id: f for f in favorites} tracks = ( diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index de7284cd6..b2469a8ca 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -147,6 +147,15 @@ class SubsonicViewSet(viewsets.GenericViewSet): return response.Response(payload, status=200) + @list_route(methods=["get", "post"], url_name="get_song", url_path="getSong") + @find_object(music_models.Track.objects.all()) + def get_song(self, request, *args, **kwargs): + track = kwargs.pop("obj") + data = serializers.GetSongSerializer(track).data + payload = {"song": data} + + return response.Response(payload, status=200) + @list_route( methods=["get", "post"], url_name="get_artist_info2", url_path="getArtistInfo2" ) diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index d9b50d3eb..1c7c528cc 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -140,6 +140,20 @@ def test_get_album(f, db, logged_in_api_client, factories): assert response.data == expected +@pytest.mark.parametrize("f", ["xml", "json"]) +def test_get_song(f, db, logged_in_api_client, factories): + url = reverse("api:subsonic-get-song") + assert url.endswith("getSong") is True + artist = factories["music.Artist"]() + album = factories["music.Album"](artist=artist) + track = factories["music.Track"](album=album) + tf = factories["music.TrackFile"](track=track) + response = logged_in_api_client.get(url, {"f": f, "id": track.pk}) + + assert response.status_code == 200 + assert response.data == {"song": serializers.get_track_data(track.album, track, tf)} + + @pytest.mark.parametrize("f", ["xml", "json"]) def test_stream(f, db, logged_in_api_client, factories, mocker): url = reverse("api:subsonic-stream")