From 8c9a3d6df232faf1f61f2aeb6cf827f506e50743 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 1 Nov 2012 12:46:29 +0100 Subject: [PATCH] Slugify local playlist names to make them safe to use in paths (#217) --- mopidy/backends/local/stored_playlists.py | 23 +++++++++--- tests/backends/base/stored_playlists.py | 16 ++++----- tests/backends/local/stored_playlists_test.py | 36 ++++++++++++++----- 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/mopidy/backends/local/stored_playlists.py b/mopidy/backends/local/stored_playlists.py index 139cfac8..3d488655 100644 --- a/mopidy/backends/local/stored_playlists.py +++ b/mopidy/backends/local/stored_playlists.py @@ -1,7 +1,9 @@ import glob import logging import os +import re import shutil +import unicodedata from mopidy import settings from mopidy.backends import base @@ -20,9 +22,8 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): self._folder = settings.LOCAL_PLAYLIST_PATH self.refresh() - # TODO playlist names needs safer handling using a slug function - def create(self, name): + name = self._slugify(name) file_path = os.path.join(self._folder, name + '.m3u') uri = path.path_to_uri(file_path) playlist = Playlist(uri=uri, name=name) @@ -68,10 +69,11 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): old_playlist = self.lookup(playlist.uri) if old_playlist and playlist.name != old_playlist.name: + new_name = self._slugify(playlist.name) src = os.path.join(self._folder, old_playlist.name + '.m3u') - dst = os.path.join(self._folder, playlist.name + '.m3u') + dst = os.path.join(self._folder, new_name + '.m3u') shutil.move(src, dst) - playlist = playlist.copy(uri=path.path_to_uri(dst)) + playlist = playlist.copy(uri=path.path_to_uri(dst), name=new_name) self._save_m3u(playlist) @@ -97,3 +99,16 @@ class LocalStoredPlaylistsProvider(base.BaseStoredPlaylistsProvider): file_path = playlist.uri[len('file://'):] if os.path.exists(file_path): os.remove(file_path) + + def _slugify(self, value): + """ + Converts to lowercase, removes non-word characters (alphanumerics and + underscores) and converts spaces to hyphens. Also strips leading and + trailing whitespace. + + This function is based on Django's slugify implementation. + """ + value = unicodedata.normalize('NFKD', value) + value = value.encode('ascii', 'ignore').decode('ascii') + value = re.sub('[^\w\s-]', '', value).strip().lower() + return re.sub('[-\s]+', '-', value) diff --git a/tests/backends/base/stored_playlists.py b/tests/backends/base/stored_playlists.py index 2b5469ac..267a025c 100644 --- a/tests/backends/base/stored_playlists.py +++ b/tests/backends/base/stored_playlists.py @@ -31,15 +31,15 @@ class StoredPlaylistsControllerTest(object): settings.runtime.clear() def test_create_returns_playlist_with_name_set(self): - playlist = self.stored.create('test') + playlist = self.stored.create(u'test') self.assertEqual(playlist.name, 'test') def test_create_returns_playlist_with_uri_set(self): - playlist = self.stored.create('test') + playlist = self.stored.create(u'test') self.assert_(playlist.uri) def test_create_adds_playlist_to_playlists_collection(self): - playlist = self.stored.create('test') + playlist = self.stored.create(u'test') self.assert_(self.stored.playlists) self.assertIn(playlist, self.stored.playlists) @@ -50,7 +50,7 @@ class StoredPlaylistsControllerTest(object): self.stored.delete('file:///unknown/playlist') def test_delete_playlist_removes_it_from_the_collection(self): - playlist = self.stored.create('test') + playlist = self.stored.create(u'test') self.assertIn(playlist, self.stored.playlists) self.stored.delete(playlist.uri) @@ -66,7 +66,7 @@ class StoredPlaylistsControllerTest(object): self.assertRaises(LookupError, test) def test_get_with_right_criteria(self): - playlist1 = self.stored.create('test') + playlist1 = self.stored.create(u'test') playlist2 = self.stored.get(name='test') self.assertEqual(playlist1, playlist2) @@ -96,7 +96,7 @@ class StoredPlaylistsControllerTest(object): self.assertEqual(u'"name=c" match no playlists', e[0]) def test_lookup_finds_playlist_by_uri(self): - original_playlist = self.stored.create('test') + original_playlist = self.stored.create(u'test') looked_up_playlist = self.stored.lookup(original_playlist.uri) @@ -107,10 +107,10 @@ class StoredPlaylistsControllerTest(object): pass def test_save_replaces_stored_playlist_with_updated_playlist(self): - playlist1 = self.stored.create('test1') + playlist1 = self.stored.create(u'test1') self.assertIn(playlist1, self.stored.playlists) - playlist2 = playlist1.copy(name='test2') + playlist2 = playlist1.copy(name=u'test2') playlist2 = self.stored.save(playlist2) self.assertNotIn(playlist1, self.stored.playlists) self.assertIn(playlist2, self.stored.playlists) diff --git a/tests/backends/local/stored_playlists_test.py b/tests/backends/local/stored_playlists_test.py index 987c0788..cd1ecd3c 100644 --- a/tests/backends/local/stored_playlists_test.py +++ b/tests/backends/local/stored_playlists_test.py @@ -18,22 +18,40 @@ class LocalStoredPlaylistsControllerTest( def test_created_playlist_is_persisted(self): path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u') - self.assert_(not os.path.exists(path)) - self.stored.create('test') - self.assert_(os.path.exists(path)) + self.assertFalse(os.path.exists(path)) + + self.stored.create(u'test') + self.assertTrue(os.path.exists(path)) + + def test_create_slugifies_playlist_name(self): + path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test-foo-bar.m3u') + self.assertFalse(os.path.exists(path)) + + playlist = self.stored.create(u'test FOO baR') + self.assertEqual(u'test-foo-bar', playlist.name) + self.assertTrue(os.path.exists(path)) + + def test_create_slugifies_names_which_tries_to_change_directory(self): + path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test-foo-bar.m3u') + self.assertFalse(os.path.exists(path)) + + playlist = self.stored.create(u'../../test FOO baR') + self.assertEqual(u'test-foo-bar', playlist.name) + self.assertTrue(os.path.exists(path)) def test_saved_playlist_is_persisted(self): path1 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test1.m3u') - path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2.m3u') + path2 = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test2-foo-bar.m3u') - playlist = self.stored.create('test1') + playlist = self.stored.create(u'test1') self.assertTrue(os.path.exists(path1)) self.assertFalse(os.path.exists(path2)) - playlist = playlist.copy(name='test2') + playlist = playlist.copy(name=u'test2 FOO baR') playlist = self.stored.save(playlist) + self.assertEqual(u'test2-foo-bar', playlist.name) self.assertFalse(os.path.exists(path1)) self.assertTrue(os.path.exists(path2)) @@ -41,7 +59,7 @@ class LocalStoredPlaylistsControllerTest( path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u') self.assertFalse(os.path.exists(path)) - playlist = self.stored.create('test') + playlist = self.stored.create(u'test') self.assertTrue(os.path.exists(path)) self.stored.delete(playlist.uri) @@ -50,7 +68,7 @@ class LocalStoredPlaylistsControllerTest( def test_playlist_contents_is_written_to_disk(self): track = Track(uri=generate_song(1)) track_path = track.uri[len('file://'):] - playlist = self.stored.create('test') + playlist = self.stored.create(u'test') playlist_path = playlist.uri[len('file://'):] playlist = playlist.copy(tracks=[track]) playlist = self.stored.save(playlist) @@ -64,7 +82,7 @@ class LocalStoredPlaylistsControllerTest( playlist_path = os.path.join(settings.LOCAL_PLAYLIST_PATH, 'test.m3u') track = Track(uri=path_to_uri(path_to_data_dir('uri2'))) - playlist = self.stored.create('test') + playlist = self.stored.create(u'test') playlist = playlist.copy(tracks=[track]) playlist = self.stored.save(playlist)