more characters escaped on Windows

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)

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