diff --git a/docs/changelog.rst b/docs/changelog.rst index ee2ddf8c..067e59fb 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,6 +26,10 @@ Bug fix release. - Core: Avoid endless loop if all tracks in the tracklist are unplayable and consume mode is off. (Fixes: :issue:`1221`, :issue:`1454`, PR: :issue:`1455`) +- File: Ensure path comparision is done between bytestrings only. Fixes crash + where a :confval:`file/media_dirs` path contained non-ASCII characters. + (Fixes: :issue:`1345`, PR: :issue:`1493`) + v2.0.0 (2016-02-15) =================== diff --git a/mopidy/file/library.py b/mopidy/file/library.py index 09fa2cf1..10182a38 100644 --- a/mopidy/file/library.py +++ b/mopidy/file/library.py @@ -132,5 +132,6 @@ class FileLibraryProvider(backend.LibraryProvider): def _is_in_basedir(self, local_path): return any( - path.is_path_inside_base_dir(local_path, media_dir['path']) + path.is_path_inside_base_dir( + local_path, media_dir['path'].encode('utf-8')) for media_dir in self._media_dirs) diff --git a/mopidy/internal/path.py b/mopidy/internal/path.py index 498b3016..1c10736f 100644 --- a/mopidy/internal/path.py +++ b/mopidy/internal/path.py @@ -196,6 +196,11 @@ def find_mtimes(root, follow=False): def is_path_inside_base_dir(path, base_path): + if not isinstance(path, bytes): + raise ValueError('path is not a bytestring') + if not isinstance(base_path, bytes): + raise ValueError('base_path is not a bytestring') + if path.endswith(os.sep): raise ValueError('Path %s cannot end with a path separator' % path) diff --git a/tests/internal/test_path.py b/tests/internal/test_path.py index 9e09c39a..6eebaaa3 100644 --- a/tests/internal/test_path.py +++ b/tests/internal/test_path.py @@ -7,6 +7,8 @@ import shutil import tempfile import unittest +import pytest + from mopidy import compat, exceptions from mopidy.internal import path from mopidy.internal.gi import GLib @@ -392,6 +394,30 @@ class FindMTimesTest(unittest.TestCase): self.assertEqual(errors, {}) +class TestIsPathInsideBaseDir(object): + def test_when_inside(self): + assert path.is_path_inside_base_dir( + '/æ/øå'.encode('utf-8'), + '/æ'.encode('utf-8')) + + def test_when_outside(self): + assert not path.is_path_inside_base_dir( + '/æ/øå'.encode('utf-8'), + '/ø'.encode('utf-8')) + + def test_byte_inside_str_fails(self): + with pytest.raises(ValueError): + path.is_path_inside_base_dir('/æ/øå'.encode('utf-8'), '/æ') + + def test_str_inside_byte_fails(self): + with pytest.raises(ValueError): + path.is_path_inside_base_dir('/æ/øå', '/æ'.encode('utf-8')) + + def test_str_inside_str_fails(self): + with pytest.raises(ValueError): + path.is_path_inside_base_dir('/æ/øå', '/æ') + + # TODO: kill this in favour of just os.path.getmtime + mocks class MtimeTest(unittest.TestCase):