From b21debf6eec60f146c9f6f6c7378a1be5d813528 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 5 Dec 2015 13:51:36 +0100 Subject: [PATCH] mpd: Sanity check stored playlist names --- mopidy/mpd/exceptions.py | 9 ++++++ mopidy/mpd/protocol/stored_playlists.py | 11 +++++++ tests/mpd/protocol/test_stored_playlists.py | 34 +++++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/mopidy/mpd/exceptions.py b/mopidy/mpd/exceptions.py index b64a6cf0..65771f2a 100644 --- a/mopidy/mpd/exceptions.py +++ b/mopidy/mpd/exceptions.py @@ -84,6 +84,15 @@ class MpdSystemError(MpdAckError): error_code = MpdAckError.ACK_ERROR_SYSTEM +class MpdInvalidPlaylistName(MpdAckError): + error_code = MpdAckError.ACK_ERROR_ARG + + def __init__(self, *args, **kwargs): + super(MpdInvalidPlaylistName, self).__init__(*args, **kwargs) + self.message = ('playlist name is invalid: playlist names may not ' + 'contain slashes, newlines or carriage returns') + + class MpdNotImplemented(MpdAckError): error_code = 0 diff --git a/mopidy/mpd/protocol/stored_playlists.py b/mopidy/mpd/protocol/stored_playlists.py index 1a24608c..fd395696 100644 --- a/mopidy/mpd/protocol/stored_playlists.py +++ b/mopidy/mpd/protocol/stored_playlists.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, division, unicode_literals import datetime import logging +import re import warnings from mopidy.compat import urllib @@ -10,6 +11,11 @@ from mopidy.mpd import exceptions, protocol, translator logger = logging.getLogger(__name__) +def _check_playlist_name(name): + if re.search('[/\n\r]', name): + raise exceptions.MpdInvalidPlaylistName() + + @protocol.commands.add('listplaylist') def listplaylist(context, name): """ @@ -149,6 +155,7 @@ def playlistadd(context, name, track_uri): ``NAME.m3u`` will be created if it does not exist. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) old_playlist = uri is not None and context.core.playlists.lookup(uri).get() if not old_playlist: @@ -219,6 +226,7 @@ def playlistclear(context, name): The playlist will be created if it does not exist. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) playlist = uri is not None and context.core.playlists.lookup(uri).get() if not playlist: @@ -240,6 +248,7 @@ def playlistdelete(context, name, songpos): Deletes ``SONGPOS`` from the playlist ``NAME.m3u``. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) playlist = uri is not None and context.core.playlists.lookup(uri).get() if not playlist: @@ -327,6 +336,7 @@ def rm(context, name): Removes the playlist ``NAME.m3u`` from the playlist directory. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) if not uri: raise exceptions.MpdNoExistError('No such playlist') @@ -343,6 +353,7 @@ def save(context, name): Saves the current playlist to ``NAME.m3u`` in the playlist directory. """ + _check_playlist_name(name) tracks = context.core.tracklist.get_tracks().get() uri = context.lookup_playlist_uri_from_name(name) playlist = uri is not None and context.core.playlists.lookup(uri).get() diff --git a/tests/mpd/protocol/test_stored_playlists.py b/tests/mpd/protocol/test_stored_playlists.py index 36635505..e33b1bc2 100644 --- a/tests/mpd/protocol/test_stored_playlists.py +++ b/tests/mpd/protocol/test_stored_playlists.py @@ -232,6 +232,10 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertEqual(0, len(self.core.tracklist.tracks.get())) self.assertEqualResponse('ACK [50@0] {load} No such playlist') + # No invalid name check for load. + self.send_request('load "unknown/playlist"') + self.assertEqualResponse('ACK [50@0] {load} No such playlist') + def test_playlistadd(self): tracks = [ Track(uri='dummy:a'), @@ -259,6 +263,12 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + def test_playlistadd_invalid_name_acks(self): + self.send_request('playlistadd "foo/bar" "dummy:a"') + self.assertInResponse('ACK [2@0] {playlistadd} playlist name is ' + 'invalid: playlist names may not contain ' + 'slashes, newlines or carriage returns') + def test_playlistclear(self): self.backend.playlists.set_dummy_playlists([ Playlist( @@ -276,6 +286,12 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + def test_playlistclear_invalid_name_acks(self): + self.send_request('playlistclear "foo/bar"') + self.assertInResponse('ACK [2@0] {playlistclear} playlist name is ' + 'invalid: playlist names may not contain ' + 'slashes, newlines or carriage returns') + def test_playlistdelete(self): tracks = [ Track(uri='dummy:a'), @@ -292,6 +308,12 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertEqual( 2, len(self.backend.playlists.get_items('dummy:a1').get())) + def test_playlistdelete_invalid_name_acks(self): + self.send_request('playlistdelete "foo/bar" "0"') + self.assertInResponse('ACK [2@0] {playlistdelete} playlist name is ' + 'invalid: playlist names may not contain ' + 'slashes, newlines or carriage returns') + def test_playlistmove(self): tracks = [ Track(uri='dummy:a'), @@ -334,8 +356,20 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.send_request('rm "name"') self.assertInResponse('ACK [50@0] {rm} No such playlist') + def test_rm_invalid_name_acks(self): + self.send_request('rm "foo/bar"') + self.assertInResponse('ACK [2@0] {rm} playlist name is invalid: ' + 'playlist names may not contain slashes, ' + 'newlines or carriage returns') + def test_save(self): self.send_request('save "name"') self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + + def test_save_invalid_name_acks(self): + self.send_request('save "foo/bar"') + self.assertInResponse('ACK [2@0] {save} playlist name is invalid: ' + 'playlist names may not contain slashes, ' + 'newlines or carriage returns')