sanitize path components from plugins (#315)

beets/library.py

@@ -854,9 +854,12 @@ class Library(BaseLibrary):
             mapping['artist'] = mapping['albumartist']
         if not mapping['albumartist']:
             mapping['albumartist'] = mapping['artist']
+
+        # Get values from plugins.
+        for key, value in plugins.template_values(item).iteritems():
+            mapping[key] = util.sanitize_for_path(value, pathmod, key)
         
         # Perform substitution.
-        mapping.update(plugins.template_values(item))
         funcs = dict(TEMPLATE_FUNCTIONS)
         funcs.update(plugins.template_funcs())
         subpath = subpath_tmpl.substitute(mapping, funcs)

docs/changelog.rst

@@ -15,6 +15,8 @@ Changelog
   instead of 24.
 * The autotagger now also tolerates tracks whose track artists tags are set
   to "Various Artists".
+* Plugin-supplied template values, such as those created by ``rewrite``, are now
+  properly sanitized (for example, ``AC/DC`` properly becomes ``AC_DC``).
 * The ``inline`` plugin now prints a more comprehensible error when exceptions
   occur in Python snippets.
 * A reasonable error message is now shown when the import log file cannot be

test/test_db.py

@@ -26,6 +26,7 @@ import _common
 from _common import item
 import beets.library
 from beets import util
+from beets import plugins
 
 def lib():
     return beets.library.Library(os.path.join(_common.RSRC, 'test.blb'))
@@ -408,7 +409,8 @@ class DestinationFunctionTest(unittest.TestCase):
     def _setf(self, fmt):
         self.lib.path_formats.insert(0, ('default', fmt))
     def _assert_dest(self, dest):
-        self.assertEqual(self.lib.destination(self.i), dest)
+        self.assertEqual(self.lib.destination(self.i, pathmod=posixpath),
+                         dest)
 
     def test_upper_case_literal(self):
         self._setf(u'%upper{foo}')
@@ -449,6 +451,47 @@ class DestinationFunctionTest(unittest.TestCase):
     def test_nonexistent_function(self):
         self._setf(u'%foo{bar}')
         self._assert_dest('/base/%foo{bar}')
+
+class PluginDestinationTest(unittest.TestCase):
+    # Mock the plugins.template_values(item) function.
+    def _template_values(self, item):
+        return self._tv_map
+    def setUp(self):
+        self._tv_map = {}
+        self.old_template_values = plugins.template_values
+        plugins.template_values = self._template_values
+
+        self.lib = beets.library.Library(':memory:')
+        self.lib.directory = '/base'
+        self.lib.path_formats = [('default', u'$artist $foo')]
+        self.i = item()
+    def tearDown(self):
+        plugins.template_values = self.old_template_values
+
+    def _assert_dest(self, dest):
+        self.assertEqual(self.lib.destination(self.i, pathmod=posixpath),
+                         '/base/' + dest)
+
+    def test_undefined_value_not_substituted(self):
+        self._assert_dest('the artist $foo')
+    
+    def test_plugin_value_not_substituted(self):
+        self._tv_map = {
+            'foo': 'bar',
+        }
+        self._assert_dest('the artist bar')
+    
+    def test_plugin_value_overrides_attribute(self):
+        self._tv_map = {
+            'artist': 'bar',
+        }
+        self._assert_dest('bar $foo')
+
+    def test_plugin_value_sanitized(self):
+        self._tv_map = {
+            'foo': 'bar/baz',
+        }
+        self._assert_dest('the artist bar_baz')
     
 class MigrationTest(unittest.TestCase):
     """Tests the ability to change the database schema between