diff --git a/beets/library.py b/beets/library.py index 978589062..47abf59f9 100644 --- a/beets/library.py +++ b/beets/library.py @@ -20,6 +20,7 @@ import shutil import sys from string import Template import logging +import platform from beets.mediafile import MediaFile, UnreadableFileError, FileTypeError MAX_FILENAME_LENGTH = 200 @@ -130,6 +131,31 @@ def _unicode_path(path): return path return path.decode(sys.getfilesystemencoding()) +# Note: POSIX actually supports \ and : -- I just think they're +# a pain. +SPECIAL_CHARS_UNIX = re.compile(r'[\\/:]|^\.') +SPECIAL_CHARS_WINDOWS = re.compile('[\\/:"\*\?<>\|]|^\.|\.$') +SANITIZE_CHAR = u'_' +def _sanitize_path(path, plat=None): + """Takes a path and makes sure that it is legal for the specified + platform (as returned by platform.system()). Returns a new path. + """ + plat = plat or platform.system() + comps = _components(path) + for i, comp in enumerate(comps): + # Replace special characters. + if plat == 'Windows': + regex = SPECIAL_CHARS_WINDOWS + else: + regex = SPECIAL_CHARS_UNIX + comp = regex.sub(SANITIZE_CHAR, comp) + + # Truncate each component. + if len(comp) > MAX_FILENAME_LENGTH: + comp = comp[:MAX_FILENAME_LENGTH] + + comps[i] = comp + return os.path.join(*comps) @@ -680,8 +706,7 @@ class Library(BaseLibrary): # sanitize the value for inclusion in a path: # replace / and leading . with _ if isinstance(value, basestring): - value.replace(os.sep, '_') - value = re.sub(r'[\\/:]|^\.', '_', value) + value = value.replace(os.sep, '_') elif key in ('track', 'tracktotal', 'disc', 'disctotal'): # pad with zeros value = '%02i' % value @@ -692,12 +717,8 @@ class Library(BaseLibrary): # Perform substitution. subpath = subpath_tmpl.substitute(mapping) - # Truncate path components. - comps = _components(subpath) - for i, comp in enumerate(comps): - if len(comp) > MAX_FILENAME_LENGTH: - comps[i] = comp[:MAX_FILENAME_LENGTH] - subpath = os.path.join(*comps) + # Truncate components and remove forbidden characters. + subpath = _sanitize_path(subpath) # Preserve extension. _, extension = os.path.splitext(item.path) diff --git a/test/test_db.py b/test/test_db.py index 4106c61f9..c7e4c4354 100644 --- a/test/test_db.py +++ b/test/test_db.py @@ -198,6 +198,18 @@ class DestinationTest(unittest.TestCase): self.assertTrue('two' in dest) self.assertFalse('one/two' in dest) + def test_destination_escapes_leading_dot(self): + self.i.album = '.something' + dest = self.lib.destination(self.i) + self.assertTrue('something' in dest) + self.assertFalse('/.' in dest) + + def test_destination_preserves_legitimate_slashes(self): + self.i.artist = 'one' + self.i.album = 'two' + dest = self.lib.destination(self.i) + self.assertTrue(os.path.join('one', 'two') in dest) + def test_destination_long_names_truncated(self): self.i.title = 'X'*300 self.i.artist = 'Y'*300 @@ -209,6 +221,24 @@ class DestinationTest(unittest.TestCase): self.i.path = 'something.extn' dest = self.lib.destination(self.i) self.assertEqual(dest[-5:], '.extn') + + def test_sanitize_unix_replaces_leading_dot(self): + p = beets.library._sanitize_path('one/.two/three', 'Darwin') + self.assertFalse('.' in p) + + def test_sanitize_windows_replaces_trailing_dot(self): + p = beets.library._sanitize_path('one/two./three', 'Windows') + self.assertFalse('.' in p) + + def test_sanitize_windows_replaces_illegal_chars(self): + p = beets.library._sanitize_path(':*?"<>|', 'Windows') + self.assertFalse(':' in p) + self.assertFalse('*' in p) + self.assertFalse('?' in p) + self.assertFalse('"' in p) + self.assertFalse('<' in p) + self.assertFalse('>' in p) + self.assertFalse('|' in p) class MigrationTest(unittest.TestCase): """Tests the ability to change the database schema between