Merge pull request #1150 from phw/PICARD-1492-fix-vorbis-rating-email-with-invalid-chars

PICARD-1492: Fix saving Vorbis ratings with invalid chars in user e-mail

picard/formats/vorbis.py

@@ -49,6 +49,17 @@ from picard.util import (
 )
 
 
+INVALID_CHARS = re.compile('([^\x20-}]|=)')
+
+
+def sanitize_key(key):
+    """
+    Remove characters from key which are invalid for a Vorbis comment field name.
+    See https://www.xiph.org/vorbis/doc/v-comment.html#vectorformat
+    """
+    return INVALID_CHARS.sub('', key)
+
+
 class VCommentFile(File):
 
     """Generic VComment-based file."""
@@ -94,7 +105,7 @@ class VCommentFile(File):
                         name, email = name.split(':', 1)
                     except ValueError:
                         email = ''
-                    if email != config.setting['rating_user_email']:
+                    if email != sanitize_key(config.setting['rating_user_email']):
                         continue
                     name = '~rating'
                     try:
@@ -186,8 +197,9 @@ class VCommentFile(File):
         for name, value in metadata.items():
             if name == '~rating':
                 # Save rating according to http://code.google.com/p/quodlibet/wiki/Specs_VorbisComments
-                if config.setting['rating_user_email']:
-                    name = 'rating:%s' % config.setting['rating_user_email']
+                user_email = sanitize_key(config.setting['rating_user_email'])
+                if user_email:
+                    name = 'rating:%s' % user_email
                 else:
                     name = 'rating'
                 value = str(float(value) / (config.setting['rating_steps'] - 1))

test/test_formats.py

@@ -19,7 +19,7 @@ from picard.coverart.image import (
     TagCoverArtImage,
 )
 import picard.formats
-from picard.formats import ext_to_format
+from picard.formats import ext_to_format, vorbis
 from picard.metadata import Metadata
 
 settings = {
@@ -289,6 +289,16 @@ class CommonTests:
                 loaded_metadata = save_and_load_metadata(self.filename, metadata)
                 self.assertEqual(int(loaded_metadata['~rating']), rating, '~rating: %r != %r' % (loaded_metadata['~rating'], rating))
 
+        @skipUnlessTestfile
+        def test_invalid_rating_email(self):
+            if not self.supports_ratings:
+                raise unittest.SkipTest("Ratings not supported")
+            metadata = Metadata()
+            metadata['~rating'] = 3
+            config.setting['rating_user_email'] = '{in\tvälid}'
+            loaded_metadata = save_and_load_metadata(self.filename, metadata)
+            self.assertEqual(loaded_metadata['~rating'], metadata['~rating'])
+
         @skipUnlessTestfile
         def test_guess_format(self):
             temp_file = self.copy_of_original_testfile()
@@ -599,6 +609,12 @@ class OptimFROGDUalStreamTest(CommonTests.FormatsTest):
         self.assertEqual(metadata['~format'], 'OptimFROG DualStream Audio')
 
 
+class VorbisUtilTest(PicardTestCase):
+    def test_sanitize_key(self):
+        sanitized = vorbis.sanitize_key(' \x1f=}~')
+        self.assertEqual(sanitized, ' }')
+
+
 cover_settings = {
     'embed_only_one_front_image': True,
 }