From 160a70e6dfcd6e7b9e98e6c9bc3d1ea9a0c7fb28 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 25 Apr 2013 21:08:33 +0200 Subject: [PATCH] path: Only accept bytes as paths --- mopidy/utils/path.py | 6 +++++ tests/__init__.py | 4 ++- tests/backends/local/events_test.py | 2 +- tests/backends/local/library_test.py | 2 +- tests/backends/local/playback_test.py | 2 +- tests/backends/local/tracklist_test.py | 2 +- tests/config/types_test.py | 26 +++++++++---------- tests/utils/path_test.py | 36 +++++++++++++++++++------- 8 files changed, 53 insertions(+), 27 deletions(-) diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index 164b114c..3d8bacf8 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -24,6 +24,8 @@ XDG_DIRS = { def get_or_create_dir(dir_path): + if not isinstance(dir_path, bytes): + raise ValueError('Path is not a bytestring.') dir_path = expand_path(dir_path) if os.path.isfile(dir_path): raise OSError( @@ -36,6 +38,8 @@ def get_or_create_dir(dir_path): def get_or_create_file(file_path): + if not isinstance(file_path, bytes): + raise ValueError('Path is not a bytestring.') file_path = expand_path(file_path) get_or_create_dir(os.path.dirname(file_path)) if not os.path.isfile(file_path): @@ -93,6 +97,8 @@ def split_path(path): def expand_path(path): + if not isinstance(path, bytes): + raise ValueError('Path is not a bytestring.') # TODO: expandvars as well? path = string.Template(path).safe_substitute(XDG_DIRS) path = os.path.expanduser(path) diff --git a/tests/__init__.py b/tests/__init__.py index b4e1d283..98df0370 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -10,8 +10,10 @@ else: def path_to_data_dir(name): + if not isinstance(name, bytes): + name = name.encode(sys.getfilesystemencoding()) path = os.path.dirname(__file__) - path = os.path.join(path, 'data') + path = os.path.join(path, b'data') path = os.path.abspath(path) return os.path.join(path, name) diff --git a/tests/backends/local/events_test.py b/tests/backends/local/events_test.py index e09bf4b9..1218afc4 100644 --- a/tests/backends/local/events_test.py +++ b/tests/backends/local/events_test.py @@ -11,7 +11,7 @@ class LocalBackendEventsTest(events.BackendEventsTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('empty_tag_cache'), } } diff --git a/tests/backends/local/library_test.py b/tests/backends/local/library_test.py index 74635b3e..1cb933ce 100644 --- a/tests/backends/local/library_test.py +++ b/tests/backends/local/library_test.py @@ -11,7 +11,7 @@ class LocalLibraryControllerTest(LibraryControllerTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('library_tag_cache'), } } diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index 16592539..7381113a 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -15,7 +15,7 @@ class LocalPlaybackControllerTest(PlaybackControllerTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('empty_tag_cache'), } } diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 2d8e87b6..44e4d004 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -13,7 +13,7 @@ class LocalTracklistControllerTest(TracklistControllerTest, unittest.TestCase): config = { 'local': { 'media_dir': path_to_data_dir(''), - 'playlists_dir': '', + 'playlists_dir': b'', 'tag_cache_file': path_to_data_dir('empty_tag_cache'), } } diff --git a/tests/config/types_test.py b/tests/config/types_test.py index de800e98..6351a0b2 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -324,47 +324,47 @@ class PortTest(unittest.TestCase): class ExpandedPathTest(unittest.TestCase): def test_is_bytes(self): - self.assertIsInstance(types.ExpandedPath('/tmp'), bytes) + self.assertIsInstance(types.ExpandedPath(b'/tmp'), bytes) @mock.patch('mopidy.utils.path.expand_path') def test_defaults_to_expanded(self, expand_path_mock): - expand_path_mock.return_value = 'expanded_path' - self.assertEqual('expanded_path', types.ExpandedPath('~')) + expand_path_mock.return_value = b'expanded_path' + self.assertEqual(b'expanded_path', types.ExpandedPath(b'~')) @mock.patch('mopidy.utils.path.expand_path') def test_orginal_stores_unexpanded(self, expand_path_mock): - self.assertEqual('~', types.ExpandedPath('~').original) + self.assertEqual('~', types.ExpandedPath(b'~').original) class PathTest(unittest.TestCase): def test_deserialize_conversion_success(self): - result = types.Path().deserialize('/foo') + result = types.Path().deserialize(b'/foo') self.assertEqual('/foo', result) self.assertIsInstance(result, types.ExpandedPath) self.assertIsInstance(result, bytes) def test_deserialize_enforces_choices(self): value = types.Path(choices=['/foo', '/bar', '/baz']) - self.assertEqual('/foo', value.deserialize('/foo')) - self.assertRaises(ValueError, value.deserialize, '/foobar') + self.assertEqual('/foo', value.deserialize(b'/foo')) + self.assertRaises(ValueError, value.deserialize, b'/foobar') def test_deserialize_enforces_required(self): value = types.Path() - self.assertRaises(ValueError, value.deserialize, '') + self.assertRaises(ValueError, value.deserialize, b'') def test_deserialize_respects_optional(self): value = types.Path(optional=True) - self.assertIsNone(value.deserialize('')) - self.assertIsNone(value.deserialize(' ')) + self.assertIsNone(value.deserialize(b'')) + self.assertIsNone(value.deserialize(b' ')) @mock.patch('mopidy.utils.path.expand_path') def test_serialize_uses_original(self, expand_path_mock): - expand_path_mock.return_value = 'expanded_path' - path = types.ExpandedPath('original_path') + expand_path_mock.return_value = b'expanded_path' + path = types.ExpandedPath(b'original_path') value = types.Path() self.assertEqual('expanded_path', path) self.assertEqual('original_path', value.serialize(path)) def test_serialize_plain_string(self): value = types.Path() - self.assertEqual('path', value.serialize('path')) + self.assertEqual('path', value.serialize(b'path')) diff --git a/tests/utils/path_test.py b/tests/utils/path_test.py index bd2e794b..13e541fc 100644 --- a/tests/utils/path_test.py +++ b/tests/utils/path_test.py @@ -22,7 +22,7 @@ class GetOrCreateDirTest(unittest.TestCase): shutil.rmtree(self.parent) def test_creating_dir(self): - dir_path = os.path.join(self.parent, 'test') + dir_path = os.path.join(self.parent, b'test') self.assert_(not os.path.exists(dir_path)) created = path.get_or_create_dir(dir_path) self.assert_(os.path.exists(dir_path)) @@ -30,8 +30,8 @@ class GetOrCreateDirTest(unittest.TestCase): self.assertEqual(created, dir_path) def test_creating_nested_dirs(self): - level2_dir = os.path.join(self.parent, 'test') - level3_dir = os.path.join(self.parent, 'test', 'test') + level2_dir = os.path.join(self.parent, b'test') + level3_dir = os.path.join(self.parent, b'test', b'test') self.assert_(not os.path.exists(level2_dir)) self.assert_(not os.path.exists(level3_dir)) created = path.get_or_create_dir(level3_dir) @@ -48,11 +48,20 @@ class GetOrCreateDirTest(unittest.TestCase): self.assertEqual(created, self.parent) def test_create_dir_with_name_of_existing_file_throws_oserror(self): - conflicting_file = os.path.join(self.parent, 'test') + conflicting_file = os.path.join(self.parent, b'test') open(conflicting_file, 'w').close() - dir_path = os.path.join(self.parent, 'test') + dir_path = os.path.join(self.parent, b'test') self.assertRaises(OSError, path.get_or_create_dir, dir_path) + def test_create_dir_with_unicode(self): + with self.assertRaises(ValueError): + dir_path = unicode(os.path.join(self.parent, b'test')) + path.get_or_create_dir(dir_path) + + def test_create_dir_with_none(self): + with self.assertRaises(ValueError): + path.get_or_create_dir(None) + class GetOrCreateFileTest(unittest.TestCase): def setUp(self): @@ -63,7 +72,7 @@ class GetOrCreateFileTest(unittest.TestCase): shutil.rmtree(self.parent) def test_creating_file(self): - file_path = os.path.join(self.parent, 'test') + file_path = os.path.join(self.parent, b'test') self.assert_(not os.path.exists(file_path)) created = path.get_or_create_file(file_path) self.assert_(os.path.exists(file_path)) @@ -71,8 +80,8 @@ class GetOrCreateFileTest(unittest.TestCase): self.assertEqual(created, file_path) def test_creating_nested_file(self): - level2_dir = os.path.join(self.parent, 'test') - file_path = os.path.join(self.parent, 'test', 'test') + level2_dir = os.path.join(self.parent, b'test') + file_path = os.path.join(self.parent, b'test', b'test') self.assert_(not os.path.exists(level2_dir)) self.assert_(not os.path.exists(file_path)) created = path.get_or_create_file(file_path) @@ -83,7 +92,7 @@ class GetOrCreateFileTest(unittest.TestCase): self.assertEqual(created, file_path) def test_creating_existing_file(self): - file_path = os.path.join(self.parent, 'test') + file_path = os.path.join(self.parent, b'test') path.get_or_create_file(file_path) created = path.get_or_create_file(file_path) self.assert_(os.path.exists(file_path)) @@ -94,6 +103,15 @@ class GetOrCreateFileTest(unittest.TestCase): conflicting_dir = os.path.join(self.parent) self.assertRaises(IOError, path.get_or_create_file, conflicting_dir) + def test_create_dir_with_unicode(self): + with self.assertRaises(ValueError): + file_path = unicode(os.path.join(self.parent, b'test')) + path.get_or_create_file(file_path) + + def test_create_dir_with_none(self): + with self.assertRaises(ValueError): + path.get_or_create_file(None) + class PathToFileURITest(unittest.TestCase): def test_simple_path(self):