upload: Fix resizing non-animated images.

5dab6e9d31 began honoring the list of disposals for every frame.
Unfortunately, passing a list of disposals for a non-animated image
raises an exception:
```
  File "zerver/lib/upload.py", line 212, in resize_emoji
    image_data = resize_gif(im, size)
  File "zerver/lib/upload.py", line 165, in resize_gif
    frames[0].save(
  File "[...]/PIL/Image.py", line 2212, in save
    save_handler(self, fp, filename)
  File "[...]/PIL/GifImagePlugin.py", line 605, in _save
    _write_single_frame(im, fp, palette)
  File "[...]/PIL/GifImagePlugin.py", line 506, in _write_single_frame
    _write_local_header(fp, im, (0, 0), flags)
  File "[...]/PIL/GifImagePlugin.py", line 647, in _write_local_header
    disposal = int(im.encoderinfo.get("disposal", 0))
TypeError: int() argument must be a string, a bytes-like object or a
number, not 'list'
```

`check_add_realm_emoji` calls this as:

```
    try:
        is_animated = upload_emoji_image(image_file, emoji_file_name, a
uthor)
        emoji_uploaded_successfully = True
    finally:
        if not emoji_uploaded_successfully:
            realm_emoji.delete()
            return None
        # ...
```

This is equivalent to dropping _all_ exceptions silently.  As such,
Zulip has silently rejected all non-animated images larger than 64x64
since 5dab6e9d31.

Adjust to only pass a single disposal if there are no additional
frames.  Add a test for non-animated images, which requires also
fixing the incidental bug that all GIF images were being recorded as
animated, regardless of if they had more than 1 frame or not.
This commit is contained in:
Alex Vandiver 2022-02-16 22:26:31 +00:00 committed by Tim Abbott
parent c396c2f7a4
commit 96a5fa9d78
4 changed files with 25 additions and 2 deletions

View File

@ -169,7 +169,7 @@ def resize_gif(im: GifImageFile, size: int = DEFAULT_EMOJI_SIZE) -> bytes:
format="GIF", format="GIF",
append_images=frames[1:], append_images=frames[1:],
duration=duration_info, duration=duration_info,
disposal=disposals, disposal=disposals if len(frames) > 1 else disposals[0],
loop=loop, loop=loop,
) )
return out.getvalue() return out.getvalue()
@ -211,7 +211,10 @@ def resize_emoji(
if should_resize: if should_resize:
image_data = resize_gif(im, size) image_data = resize_gif(im, size)
return image_data, True, still_image_data if im.n_frames > 1:
return image_data, True, still_image_data
else:
return image_data, False, None
else: else:
# Note that this is essentially duplicated in the # Note that this is essentially duplicated in the
# still_image code path, above. # still_image code path, above.

Binary file not shown.

After

Width:  |  Height:  |  Size: 2.8 KiB

View File

@ -271,6 +271,18 @@ class RealmEmojiTest(ZulipTestCase):
result = self.client_post("/json/realm/emoji/my_emoji", {"f1": fp1, "f2": fp2}) result = self.client_post("/json/realm/emoji/my_emoji", {"f1": fp1, "f2": fp2})
self.assert_json_error(result, "You must upload exactly one file.") self.assert_json_error(result, "You must upload exactly one file.")
def test_emoji_upload_success(self) -> None:
self.login("iago")
with get_test_image_file("img.gif") as fp:
result = self.client_post("/json/realm/emoji/my_emoji", {"file": fp})
self.assert_json_success(result)
def test_emoji_upload_resize_success(self) -> None:
self.login("iago")
with get_test_image_file("still_large_img.gif") as fp:
result = self.client_post("/json/realm/emoji/my_emoji", {"file": fp})
self.assert_json_success(result)
def test_emoji_upload_file_size_error(self) -> None: def test_emoji_upload_file_size_error(self) -> None:
self.login("iago") self.login("iago")
with get_test_image_file("img.png") as fp: with get_test_image_file("img.png") as fp:

View File

@ -1343,6 +1343,14 @@ class EmojiTest(UploadSerializeMixin, ZulipTestCase):
still_image = Image.open(io.BytesIO(still_img_data)) still_image = Image.open(io.BytesIO(still_img_data))
self.assertEqual((50, 50), still_image.size) self.assertEqual((50, 50), still_image.size)
# Test a non-animated image which does need to be resized
still_large_img_data = read_test_image_file("still_large_img.gif")
resized_img_data, is_animated, no_still_data = resize_emoji(still_large_img_data, size=50)
im = Image.open(io.BytesIO(resized_img_data))
self.assertEqual((50, 50), im.size)
self.assertFalse(is_animated)
assert no_still_data is None
def tearDown(self) -> None: def tearDown(self) -> None:
destroy_uploads() destroy_uploads()
super().tearDown() super().tearDown()