From a8e34667c7630c038e9357b749f6bec05e9b7b38 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 23 Feb 2021 09:29:43 -0500 Subject: [PATCH 1/6] Handle odd transparency images when thumbnailing. --- synapse/rest/media/v1/thumbnailer.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/synapse/rest/media/v1/thumbnailer.py b/synapse/rest/media/v1/thumbnailer.py index 07903e401761..ebba5205255f 100644 --- a/synapse/rest/media/v1/thumbnailer.py +++ b/synapse/rest/media/v1/thumbnailer.py @@ -96,9 +96,14 @@ def aspect(self, max_width: int, max_height: int) -> Tuple[int, int]: 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): + 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: From 50cb945826112870108d0755d8880e1217d37533 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 23 Feb 2021 10:15:26 -0500 Subject: [PATCH 2/6] Add tests. --- tests/rest/media/v1/test_media_storage.py | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 36d1e6bc4a4c..7da1065573d1 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -153,6 +153,30 @@ class _TestImage: ), ), ), + # small png with transparency. + ( + _TestImage( + unhexlify( + b"89504e470d0a1a0a0000000d49484452000000010000000101000" + b"00000376ef9240000000274524e5300010194fdae0000000a4944" + b"4154789c636800000082008177cd72b60000000049454e44ae426" + b"082" + ), + b"image/png", + b".png", + unhexlify( + b"89504e470d0a1a0a0000000d49484452000000200000002008060" + b"00000737a7af40000002f49444154789cedce3101000008c3b081" + b"7fcf43064f6aa099b6cd63fb39070000000000000000000000000" + b"0004892034d88043c4abd9d150000000049454e44ae426082", + ), + unhexlify( + b"89504e470d0a1a0a0000000d49484452000000010000000108060" + b"000001f15c4890000000d49444154789c63f8ffffff7f0009fb03" + b"fd2a86e38a0000000049454e44ae426082" + ), + ), + ), # small lossless webp ( _TestImage( From bd086ff1d5bfd53aff4f0c59523fbb6d42595e7e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 23 Feb 2021 10:18:58 -0500 Subject: [PATCH 3/6] Compare to None. --- synapse/rest/media/v1/thumbnailer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/media/v1/thumbnailer.py b/synapse/rest/media/v1/thumbnailer.py index ebba5205255f..988f52c78f73 100644 --- a/synapse/rest/media/v1/thumbnailer.py +++ b/synapse/rest/media/v1/thumbnailer.py @@ -101,7 +101,7 @@ def _resize(self, width: int, height: int) -> Image: # 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): + 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) From 5ca699e93ca8c6096c4dcf93925614063ff031e5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 23 Feb 2021 10:20:26 -0500 Subject: [PATCH 4/6] Newsfragment --- changelog.d/9473.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9473.bugfix diff --git a/changelog.d/9473.bugfix b/changelog.d/9473.bugfix new file mode 100644 index 000000000000..71fb487cf2c0 --- /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`. From 6415f48e4ed8c5c24aad450f98135fcad6c23400 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 8 Mar 2021 13:46:32 -0500 Subject: [PATCH 5/6] Do not check the output since it changes in Pillow v6.0.0. --- tests/rest/media/v1/test_media_storage.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 7da1065573d1..ba96c404327a 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -164,17 +164,10 @@ class _TestImage: ), b"image/png", b".png", - unhexlify( - b"89504e470d0a1a0a0000000d49484452000000200000002008060" - b"00000737a7af40000002f49444154789cedce3101000008c3b081" - b"7fcf43064f6aa099b6cd63fb39070000000000000000000000000" - b"0004892034d88043c4abd9d150000000049454e44ae426082", - ), - unhexlify( - b"89504e470d0a1a0a0000000d49484452000000010000000108060" - b"000001f15c4890000000d49444154789c63f8ffffff7f0009fb03" - b"fd2a86e38a0000000049454e44ae426082" - ), + # Note that we don't check the output since it varies across + # different versions of Pillow. + None, + None, ), ), # small lossless webp From 42b7379e9ef3e09f28735e32a2b44216e96056ad Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 8 Mar 2021 14:00:33 -0500 Subject: [PATCH 6/6] Don't provide empty properties. --- tests/rest/media/v1/test_media_storage.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index ba96c404327a..9f77125fd445 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -105,7 +105,7 @@ def test_ensure_media_is_in_local_cache(self): 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) @@ -166,8 +168,6 @@ class _TestImage: b".png", # Note that we don't check the output since it varies across # different versions of Pillow. - None, - None, ), ), # small lossless webp @@ -179,8 +179,6 @@ class _TestImage: ), b"image/webp", b".webp", - None, - None, ), ), # an empty file @@ -189,9 +187,7 @@ class _TestImage: b"", b"image/gif", b".gif", - None, - None, - False, + expected_found=False, ), ), ],