diff --git a/zerver/lib/thumbnail.py b/zerver/lib/thumbnail.py index ac3ce21ce2..22e9c07d4a 100644 --- a/zerver/lib/thumbnail.py +++ b/zerver/lib/thumbnail.py @@ -93,7 +93,7 @@ def resize_logo(image_data: bytes) -> bytes: def resize_emoji( image_data: bytes, emoji_file_name: str, size: int = DEFAULT_EMOJI_SIZE -) -> Tuple[bytes, bool, Optional[bytes]]: +) -> Tuple[bytes, Optional[bytes]]: if len(image_data) > MAX_EMOJI_GIF_FILE_SIZE_BYTES: raise BadImageError(_("Image size exceeds limit.")) @@ -103,10 +103,9 @@ def resize_emoji( write_file_ext = os.path.splitext(emoji_file_name)[1] assert "[" not in write_file_ext - # This function returns three values: + # This function returns two values: # 1) Emoji image data. - # 2) If the emoji is animated. - # 3) If it is animated, the still image data i.e. first frame of gif. + # 2) If it is animated, the still image data i.e. first frame of gif. with libvips_check_image(image_data) as source_image: if source_image.get_n_pages() == 1: return ( @@ -116,7 +115,6 @@ def resize_emoji( height=size, crop=pyvips.Interesting.CENTRE, ).write_to_buffer(write_file_ext), - False, None, ) first_still = pyvips.Image.thumbnail_buffer( @@ -150,4 +148,4 @@ def resize_emoji( for frame in animated.pagesplit() ] animated = frames[0].pagejoin(frames[1:]) - return (animated.write_to_buffer(write_file_ext), True, first_still) + return (animated.write_to_buffer(write_file_ext), first_still) diff --git a/zerver/lib/upload/__init__.py b/zerver/lib/upload/__init__.py index f159c9ad22..425b530693 100644 --- a/zerver/lib/upload/__init__.py +++ b/zerver/lib/upload/__init__.py @@ -258,20 +258,22 @@ def upload_emoji_image( backend.upload_single_emoji_image( f"{emoji_path}.original", content_type, user_profile, image_data ) - resized_image_data, is_animated, still_image_data = resize_emoji(image_data, emoji_file_name) - if is_animated and len(still_image_data) > MAX_EMOJI_GIF_FILE_SIZE_BYTES: # nocoverage - raise BadImageError(_("Image size exceeds limit")) - if not is_animated and len(image_data) > MAX_EMOJI_GIF_FILE_SIZE_BYTES: # nocoverage + resized_image_data, still_image_data = resize_emoji(image_data, emoji_file_name) + if still_image_data is not None: + if len(still_image_data) > MAX_EMOJI_GIF_FILE_SIZE_BYTES: # nocoverage + raise BadImageError(_("Image size exceeds limit")) + elif len(image_data) > MAX_EMOJI_GIF_FILE_SIZE_BYTES: # nocoverage raise BadImageError(_("Image size exceeds limit")) backend.upload_single_emoji_image(emoji_path, content_type, user_profile, resized_image_data) - if is_animated: - assert still_image_data is not None - still_path = RealmEmoji.STILL_PATH_ID_TEMPLATE.format( - realm_id=user_profile.realm_id, - emoji_filename_without_extension=os.path.splitext(emoji_file_name)[0], - ) - backend.upload_single_emoji_image(still_path, content_type, user_profile, still_image_data) - return is_animated + if still_image_data is None: + return False + + still_path = RealmEmoji.STILL_PATH_ID_TEMPLATE.format( + realm_id=user_profile.realm_id, + emoji_filename_without_extension=os.path.splitext(emoji_file_name)[0], + ) + backend.upload_single_emoji_image(still_path, content_type, user_profile, still_image_data) + return True def get_emoji_file_content( diff --git a/zerver/tests/test_transfer.py b/zerver/tests/test_transfer.py index 33b045e4ae..f91ad22b51 100644 --- a/zerver/tests/test_transfer.py +++ b/zerver/tests/test_transfer.py @@ -103,9 +103,8 @@ class TransferUploadsToS3Test(ZulipTestCase): resized_key = bucket.Object(emoji_path) image_data = read_test_image_file("img.png") - resized_image_data, is_animated, still_image_data = resize_emoji(image_data, "img.png") + resized_image_data, still_image_data = resize_emoji(image_data, "img.png") - self.assertEqual(is_animated, False) self.assertEqual(still_image_data, None) self.assertEqual(image_data, original_key.get()["Body"].read()) self.assertEqual(resized_image_data, resized_key.get()["Body"].read()) @@ -135,11 +134,8 @@ class TransferUploadsToS3Test(ZulipTestCase): ) image_data = read_test_image_file("animated_img.gif") - resized_image_data, is_animated, still_image_data = resize_emoji( - image_data, "animated_img.gif" - ) + resized_image_data, still_image_data = resize_emoji(image_data, "animated_img.gif") - self.assertEqual(is_animated, True) self.assertEqual(type(still_image_data), bytes) self.assertEqual(image_data, original_key.get()["Body"].read()) self.assertEqual(resized_image_data, resized_key.get()["Body"].read()) diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 24464edf11..6449304737 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -1390,10 +1390,9 @@ class EmojiTest(UploadSerializeMixin, ZulipTestCase): # Test unequal width and height of animated GIF image animated_unequal_img_data = read_test_image_file("animated_unequal_img.gif") original_image = pyvips.Image.new_from_buffer(animated_unequal_img_data, "n=-1") - resized_img_data, is_animated, still_img_data = resize_emoji( + resized_img_data, still_img_data = resize_emoji( animated_unequal_img_data, "animated_unequal_img.gif", size=50 ) - self.assertTrue(is_animated) assert still_img_data is not None emoji_image = pyvips.Image.new_from_buffer(resized_img_data, "n=-1") self.assertEqual(emoji_image.get("vips-loader"), "gifload_buffer") @@ -1415,17 +1414,16 @@ class EmojiTest(UploadSerializeMixin, ZulipTestCase): animated_large_img_data = read_test_image_file("animated_large_img.gif") original_image = pyvips.Image.new_from_buffer(animated_large_img_data, "n=-1") - resized_img_data, is_animated, still_img_data = resize_emoji( + resized_img_data, still_img_data = resize_emoji( animated_large_img_data, "animated_large_img.gif", size=50 ) + assert still_img_data is not None emoji_image = pyvips.Image.new_from_buffer(resized_img_data, "n=-1") self.assertEqual(emoji_image.get("vips-loader"), "gifload_buffer") self.assertEqual(emoji_image.get_n_pages(), original_image.get_n_pages()) self.assertEqual(emoji_image.get("page-height"), 50) self.assertEqual(emoji_image.height, 150) self.assertEqual(emoji_image.width, 50) - self.assertTrue(is_animated) - assert still_img_data still_image = pyvips.Image.new_from_buffer(still_img_data, "") self.assertEqual(still_image.get("vips-loader"), "pngload_buffer") self.assertEqual(still_image.get_n_pages(), 1) @@ -1444,7 +1442,7 @@ class EmojiTest(UploadSerializeMixin, ZulipTestCase): # Test a non-animated GIF 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( + resized_img_data, no_still_data = resize_emoji( still_large_img_data, "still_large_img.gif", size=50 ) emoji_image = pyvips.Image.new_from_buffer(resized_img_data, "n=-1") @@ -1452,20 +1450,16 @@ class EmojiTest(UploadSerializeMixin, ZulipTestCase): self.assertEqual(emoji_image.height, 50) self.assertEqual(emoji_image.width, 50) self.assertEqual(emoji_image.get_n_pages(), 1) - self.assertFalse(is_animated) assert no_still_data is None # Test a non-animated and non-animatable image format which needs to be resized still_large_img_data = read_test_image_file("img.jpg") - resized_img_data, is_animated, no_still_data = resize_emoji( - still_large_img_data, "img.jpg", size=50 - ) + resized_img_data, no_still_data = resize_emoji(still_large_img_data, "img.jpg", size=50) emoji_image = pyvips.Image.new_from_buffer(resized_img_data, "") self.assertEqual(emoji_image.get("vips-loader"), "jpegload_buffer") self.assertEqual(emoji_image.height, 50) self.assertEqual(emoji_image.width, 50) self.assertEqual(emoji_image.get_n_pages(), 1) - self.assertFalse(is_animated) assert no_still_data is None