emoji: Only resize custom emoji that need it.

This additional logic to prevent resizing is certain circumstances
(file size, dimensions) is necessary because the pillow gif handling
code seems to be rather flaky with regards to handling gif color
palletes, causing broken gifs after resizing.  The workaround is to
only resize when absolutely necessary (e.g. because the file is larger
than 128x128 or 128KB).

Fixes zulip#10351.

zerver/lib/upload.py

@@ -36,6 +36,12 @@
 MEDIUM_AVATAR_SIZE = 500
 DEFAULT_EMOJI_SIZE = 64
 
+# These sizes were selected based on looking at the maximum common
+# sizes in a library of animated custom emoji, balanced against the
+# network cost of very large emoji images.
+MAX_EMOJI_GIF_SIZE = 128
+MAX_EMOJI_GIF_FILE_SIZE_BYTES = 128 * 1024 * 1024  # 128 kb
+
 # Performance Note:
 #
 # For writing files to S3, the file could either be stored in RAM
@@ -154,12 +160,22 @@ def resize_gif(im: GifImageFile, size: int=DEFAULT_EMOJI_SIZE) -> bytes:
                    loop=loop)
     return out.getvalue()
 
+
 def resize_emoji(image_data: bytes, size: int=DEFAULT_EMOJI_SIZE) -> bytes:
     try:
         im = Image.open(io.BytesIO(image_data))
         image_format = im.format
         if image_format == "GIF":
-            return resize_gif(im, size)
+            # There are a number of bugs in Pillow.GifImagePlugin which cause
+            # results in resized gifs being broken. To work around this we
+            # only resize under certain conditions to minimize the chance of
+            # creating ugly gifs.
+            should_resize = any((
+                im.size[0] != im.size[1],                            # not square
+                im.size[0] > MAX_EMOJI_GIF_SIZE,                     # dimensions too large
+                len(image_data) > MAX_EMOJI_GIF_FILE_SIZE_BYTES,     # filesize too large
+            ))
+            return resize_gif(im, size) if should_resize else image_data
         else:
             im = exif_rotate(im)
             im = ImageOps.fit(im, (size, size), Image.ANTIALIAS)

zerver/tests/test_upload.py

@@ -2,6 +2,7 @@
 from django.conf import settings
 from django.test import TestCase
 from unittest import skip
+from unittest.mock import patch
 
 from zerver.lib.avatar import (
     avatar_url,
@@ -1096,17 +1097,32 @@ def test_resize_emoji(self) -> None:
         im = Image.open(io.BytesIO(resized_img_data))
         self.assertEqual((50, 50), im.size)
 
-        # Test for large animated image (128x128)
-        animated_large_img_data = get_test_image_file('animated_large_img.gif').read()
-        resized_img_data = resize_emoji(animated_large_img_data, size=50)
-        im = Image.open(io.BytesIO(resized_img_data))
-        self.assertEqual((50, 50), im.size)
-
         # Test corrupt image exception
         corrupted_img_data = get_test_image_file('corrupt.gif').read()
         with self.assertRaises(BadImageError):
             resize_emoji(corrupted_img_data)
 
+        # Test an image larger than max is resized
+        with patch('zerver.lib.upload.MAX_EMOJI_GIF_SIZE', 128):
+            animated_large_img_data = get_test_image_file('animated_large_img.gif').read()
+            resized_img_data = resize_emoji(animated_large_img_data, size=50)
+            im = Image.open(io.BytesIO(resized_img_data))
+            self.assertEqual((50, 50), im.size)
+
+        # Test an image file larger than max is resized
+        with patch('zerver.lib.upload.MAX_EMOJI_GIF_FILE_SIZE_BYTES', 3 * 1024 * 1024):
+            animated_large_img_data = get_test_image_file('animated_large_img.gif').read()
+            resized_img_data = resize_emoji(animated_large_img_data, size=50)
+            im = Image.open(io.BytesIO(resized_img_data))
+            self.assertEqual((50, 50), im.size)
+
+        # Test an image smaller than max and smaller than file size max is not resized
+        with patch('zerver.lib.upload.MAX_EMOJI_GIF_SIZE', 512):
+            animated_large_img_data = get_test_image_file('animated_large_img.gif').read()
+            resized_img_data = resize_emoji(animated_large_img_data, size=50)
+            im = Image.open(io.BytesIO(resized_img_data))
+            self.assertEqual((256, 256), im.size)
+
     def tearDown(self) -> None:
         destroy_uploads()