From 075c16b4103e7368e304496f3016d3b826b3ee92 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 9 Mar 2021 07:37:09 -0500 Subject: [PATCH] Handle image transparency better when thumbnailing. (#9473) Properly uses RGBA mode for 1- and 8-bit images with transparency (instead of RBG mode). --- changelog.d/9473.bugfix | 1 + synapse/rest/media/v1/thumbnailer.py | 11 ++++++--- tests/rest/media/v1/test_media_storage.py | 29 ++++++++++++++++------- 3 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 changelog.d/9473.bugfix diff --git a/changelog.d/9473.bugfix b/changelog.d/9473.bugfix new file mode 100644 index 000000000..71fb487cf --- /dev/null +++ b/changelog.d/9473.bugfix @@ -0,0 +1 @@ +Fix long-standing bug when generating thumbnails for some images with transparency: `TypeError: cannot unpack non-iterable int object`. diff --git a/synapse/rest/media/v1/thumbnailer.py b/synapse/rest/media/v1/thumbnailer.py index 07903e401..988f52c78 100644 --- a/synapse/rest/media/v1/thumbnailer.py +++ b/synapse/rest/media/v1/thumbnailer.py @@ -96,9 +96,14 @@ class Thumbnailer: def _resize(self, width: int, height: int) -> Image: # 1-bit or 8-bit color palette images need converting to RGB # otherwise they will be scaled using nearest neighbour which - # looks awful - if self.image.mode in ["1", "P"]: - self.image = self.image.convert("RGB") + # looks awful. + # + # If the image has transparency, use RGBA instead. + if self.image.mode in ["1", "L", "P"]: + mode = "RGB" + if self.image.info.get("transparency", None) is not None: + mode = "RGBA" + self.image = self.image.convert(mode) return self.image.resize((width, height), Image.ANTIALIAS) def scale(self, width: int, height: int, output_type: str) -> BytesIO: diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 36d1e6bc4..9f77125fd 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -105,7 +105,7 @@ class MediaStorageTests(unittest.HomeserverTestCase): self.assertEqual(test_body, body) -@attr.s +@attr.s(slots=True, frozen=True) class _TestImage: """An image for testing thumbnailing with the expected results @@ -117,13 +117,15 @@ class _TestImage: test should just check for success. expected_scaled: The expected bytes from scaled thumbnailing, or None if test should just check for a valid image returned. + expected_found: True if the file should exist on the server, or False if + a 404 is expected. """ data = attr.ib(type=bytes) content_type = attr.ib(type=bytes) extension = attr.ib(type=bytes) - expected_cropped = attr.ib(type=Optional[bytes]) - expected_scaled = attr.ib(type=Optional[bytes]) + expected_cropped = attr.ib(type=Optional[bytes], default=None) + expected_scaled = attr.ib(type=Optional[bytes], default=None) expected_found = attr.ib(default=True, type=bool) @@ -153,6 +155,21 @@ class _TestImage: ), ), ), + # small png with transparency. + ( + _TestImage( + unhexlify( + b"89504e470d0a1a0a0000000d49484452000000010000000101000" + b"00000376ef9240000000274524e5300010194fdae0000000a4944" + b"4154789c636800000082008177cd72b60000000049454e44ae426" + b"082" + ), + b"image/png", + b".png", + # Note that we don't check the output since it varies across + # different versions of Pillow. + ), + ), # small lossless webp ( _TestImage( @@ -162,8 +179,6 @@ class _TestImage: ), b"image/webp", b".webp", - None, - None, ), ), # an empty file @@ -172,9 +187,7 @@ class _TestImage: b"", b"image/gif", b".gif", - None, - None, - False, + expected_found=False, ), ), ],