Merge pull request #2068 from beetbox/fetchart_mime_extension

Fix fetchart hardcoding the image extension

beetsplug/fetchart.py

@@ -39,9 +39,11 @@ try:
 except ImportError:
     HAVE_ITUNES = False
 
-IMAGE_EXTENSIONS = ['png', 'jpg', 'jpeg']
-CONTENT_TYPES = ('image/jpeg', 'image/png')
-DOWNLOAD_EXTENSION = '.jpg'
+CONTENT_TYPES = {
+    'image/jpeg': [b'jpg', b'jpeg'],
+    'image/png': [b'png']
+}
+IMAGE_EXTENSIONS = [ext for exts in CONTENT_TYPES.values() for ext in exts]
 
 
 class Candidate(object):
@@ -227,8 +229,8 @@ class RemoteArtSource(ArtSource):
         try:
             with closing(self.request(candidate.url, stream=True,
                                       message=u'downloading image')) as resp:
-                if 'Content-Type' not in resp.headers \
-                        or resp.headers['Content-Type'] not in CONTENT_TYPES:
+                ct = resp.headers.get('Content-Type', None)
+                if ct not in CONTENT_TYPES:
                     self._log.debug(
                         u'not a supported image: {}',
                         resp.headers.get('Content-Type') or u'no content type',
@@ -237,7 +239,7 @@ class RemoteArtSource(ArtSource):
                     return
 
                 # Generate a temporary file with the correct extension.
-                with NamedTemporaryFile(suffix=DOWNLOAD_EXTENSION,
+                with NamedTemporaryFile(suffix=b'.' + CONTENT_TYPES[ct][0],
                                         delete=False) as fh:
                     for chunk in resp.iter_content(chunk_size=1024):
                         fh.write(chunk)

docs/changelog.rst

@@ -39,6 +39,10 @@ Fixes:
   connected to a Unix pipe. :bug:`2041`
 * Fix a crash when specifying non-ASCII format strings on the command line
   with the ``-f`` option to many commands. :bug:`2063`
+* :doc:`/plugins/fetchart`: Determine the file extension for downloaded images
+  based on the ``Content-Type`` header instead of hardcoding it to .jpg.
+  Reported in :bug:`2053`, which for now it does not fix due to
+  server-side issues, though.
 
 1.3.18 (May 31, 2016)
 ---------------------

test/test_art.py

@@ -46,7 +46,7 @@ class UseThePlugin(_common.TestCase):
 
 
 class FetchImageTest(UseThePlugin):
-    URL = 'http://example.com'
+    URL = 'http://example.com/test.jpg'
 
     @responses.activate
     def run(self, *args, **kwargs):
@@ -61,18 +61,23 @@ class FetchImageTest(UseThePlugin):
         self.dpath = os.path.join(self.temp_dir, b'arttest')
         self.source = fetchart.RemoteArtSource(logger, self.plugin.config)
         self.extra = {'maxwidth': 0}
+        self.candidate = fetchart.Candidate(logger, url=self.URL)
 
     def test_invalid_type_returns_none(self):
         self.mock_response('image/watercolour')
-        candidate = fetchart.Candidate(logger, url=self.URL)
-        self.source.fetch_image(candidate, self.extra)
-        self.assertEqual(candidate.path, None)
+        self.source.fetch_image(self.candidate, self.extra)
+        self.assertEqual(self.candidate.path, None)
 
     def test_jpeg_type_returns_path(self):
         self.mock_response('image/jpeg')
-        candidate = fetchart.Candidate(logger, url=self.URL)
-        self.source.fetch_image(candidate, self.extra)
-        self.assertNotEqual(candidate.path, None)
+        self.source.fetch_image(self.candidate, self.extra)
+        self.assertNotEqual(self.candidate.path, None)
+
+    def test_extension_set_by_content_type(self):
+        self.mock_response('image/png')
+        self.source.fetch_image(self.candidate, self.extra)
+        self.assertEqual(os.path.splitext(self.candidate.path)[1], b'.png')
+        self.assertExists(self.candidate.path)
 
 
 class FSArtTest(UseThePlugin):