emoji: Pass down content-type, rather than guessing from extension.

zerver/actions/realm_emoji.py

@@ -21,7 +21,7 @@ def notify_realm_emoji(realm: Realm, realm_emoji: Dict[str, EmojiInfo]) -> None:
 
 
 def check_add_realm_emoji(
-    realm: Realm, name: str, author: UserProfile, image_file: IO[bytes]
+    realm: Realm, name: str, author: UserProfile, image_file: IO[bytes], content_type: str
 ) -> RealmEmoji:
     try:
         realm_emoji = RealmEmoji(realm=realm, name=name, author=author)
@@ -36,7 +36,7 @@ def check_add_realm_emoji(
     emoji_uploaded_successfully = False
     is_animated = False
     try:
-        is_animated = upload_emoji_image(image_file, emoji_file_name, author)
+        is_animated = upload_emoji_image(image_file, emoji_file_name, author, content_type)
         emoji_uploaded_successfully = True
     finally:
         if not emoji_uploaded_successfully:

zerver/lib/transfer.py

@@ -10,6 +10,7 @@ from django.db import connection
 
 from zerver.lib.avatar_hash import user_avatar_path
 from zerver.lib.mime_types import guess_type
+from zerver.lib.thumbnail import BadImageError
 from zerver.lib.upload import upload_emoji_image, write_avatar_images
 from zerver.lib.upload.s3 import S3UploadBackend, upload_image_to_s3
 from zerver.models import Attachment, RealmEmoji, UserProfile
@@ -113,13 +114,21 @@ def _transfer_emoji_to_s3(realm_emoji: RealmEmoji) -> None:
     )
     assert settings.LOCAL_UPLOADS_DIR is not None
     assert settings.LOCAL_AVATARS_DIR is not None
+    content_type = guess_type(emoji_path)[0]
     emoji_path = os.path.join(settings.LOCAL_AVATARS_DIR, emoji_path) + ".original"
+    if content_type is None:  # nocoverage
+        logging.error("Emoji %d has no recognizable file extension", realm_emoji.id)
+        return
     try:
         with open(emoji_path, "rb") as f:
-            upload_emoji_image(f, realm_emoji.file_name, realm_emoji.author, backend=s3backend)
+            upload_emoji_image(
+                f, realm_emoji.file_name, realm_emoji.author, content_type, backend=s3backend
+            )
             logging.info("Uploaded emoji file in path %s", emoji_path)
     except FileNotFoundError:  # nocoverage
         logging.error("Emoji %d could not be loaded from local disk", realm_emoji.id)
+    except BadImageError as e:
+        logging.error("Emoji %d is invalid: %s", realm_emoji.id, e)
 
 
 def transfer_emoji_to_s3(processes: int) -> None:

zerver/lib/upload/__init__.py

@@ -334,11 +334,12 @@ def upload_emoji_image(
     emoji_file: IO[bytes],
     emoji_file_name: str,
     user_profile: UserProfile,
+    content_type: str,
     backend: Optional[ZulipUploadBackend] = None,
 ) -> bool:
     if backend is None:
         backend = upload_backend
-    content_type = guess_type(emoji_file_name)[0]
+
     emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format(
         realm_id=user_profile.realm_id,
         emoji_file_name=emoji_file_name,
@@ -368,21 +369,21 @@ def upload_emoji_image(
 
 def get_emoji_file_content(
     session: OutgoingSession, emoji_url: str, emoji_id: int, logger: logging.Logger
-) -> bytes:  # nocoverage
+) -> Tuple[bytes, str]:  # nocoverage
     original_emoji_url = emoji_url + ".original"
 
     logger.info("Downloading %s", original_emoji_url)
     response = session.get(original_emoji_url)
     if response.status_code == 200:
         assert isinstance(response.content, bytes)
-        return response.content
+        return response.content, response.headers["Content-Type"]
 
     logger.info("Error fetching emoji from URL %s", original_emoji_url)
     logger.info("Trying %s instead", emoji_url)
     response = session.get(emoji_url)
     if response.status_code == 200:
         assert isinstance(response.content, bytes)
-        return response.content
+        return response.content, response.headers["Content-Type"]
     logger.info("Error fetching emoji from URL %s", emoji_url)
     logger.error("Could not fetch emoji %s", emoji_id)
     raise AssertionError(f"Could not fetch emoji {emoji_id}")
@@ -403,7 +404,9 @@ def handle_reupload_emojis_event(realm: Realm, logger: logging.Logger) -> None:
         if emoji_url.startswith("/"):
             emoji_url = urljoin(realm_emoji.realm.url, emoji_url)
 
-        emoji_file_content = get_emoji_file_content(session, emoji_url, realm_emoji.id, logger)
+        emoji_file_content, content_type = get_emoji_file_content(
+            session, emoji_url, realm_emoji.id, logger
+        )
 
         emoji_bytes_io = io.BytesIO(emoji_file_content)
 
@@ -412,7 +415,9 @@ def handle_reupload_emojis_event(realm: Realm, logger: logging.Logger) -> None:
         assert user_profile is not None
 
         logger.info("Reuploading emoji %s", realm_emoji.id)
-        realm_emoji.is_animated = upload_emoji_image(emoji_bytes_io, emoji_filename, user_profile)
+        realm_emoji.is_animated = upload_emoji_image(
+            emoji_bytes_io, emoji_filename, user_profile, content_type
+        )
         realm_emoji.save(update_fields=["is_animated"])
 
 

zerver/tests/test_audit_log.py

@@ -1029,7 +1029,11 @@ class TestRealmAuditLog(ZulipTestCase):
             # check in upload_emoji, we need to make this request via
             # that helper rather than via the API.
             realm_emoji = check_add_realm_emoji(
-                realm=user.realm, name="test_emoji", author=user, image_file=img_file
+                realm=user.realm,
+                name="test_emoji",
+                author=user,
+                image_file=img_file,
+                content_type="image/png",
             )
 
         added_emoji = EmojiInfo(

zerver/tests/test_events.py

@@ -2665,7 +2665,9 @@ class NormalActionsTest(BaseAction):
         author = self.example_user("iago")
         with get_test_image_file("img.png") as img_file:
             with self.verify_action() as events:
-                check_add_realm_emoji(self.user_profile.realm, "my_emoji", author, img_file)
+                check_add_realm_emoji(
+                    self.user_profile.realm, "my_emoji", author, img_file, "image/png"
+                )
 
         check_realm_emoji_update("events[0]", events[0])
 

zerver/tests/test_import_export.py

@@ -181,7 +181,7 @@ class ExportFile(ZulipTestCase):
         realm = user_profile.realm
 
         with get_test_image_file("img.png") as img_file:
-            check_add_realm_emoji(realm, emoji_name, user_profile, img_file)
+            check_add_realm_emoji(realm, emoji_name, user_profile, img_file, "image/png")
 
     def upload_files_for_realm(self, user_profile: UserProfile) -> None:
         realm = user_profile.realm
@@ -775,7 +775,11 @@ class RealmImportExportTest(ExportFile):
 
         with get_test_image_file("img.png") as img_file:
             realm_emoji = check_add_realm_emoji(
-                realm=hamlet.realm, name="hawaii", author=hamlet, image_file=img_file
+                realm=hamlet.realm,
+                name="hawaii",
+                author=hamlet,
+                image_file=img_file,
+                content_type="image/png",
             )
             self.assertEqual(realm_emoji.name, "hawaii")
 
@@ -931,7 +935,11 @@ class RealmImportExportTest(ExportFile):
         # to test that upon import that gets fixed.
         with get_test_image_file("img.png") as img_file:
             new_realm_emoji = check_add_realm_emoji(
-                realm=hamlet.realm, name="hawaii2", author=hamlet, image_file=img_file
+                realm=hamlet.realm,
+                name="hawaii2",
+                author=hamlet,
+                image_file=img_file,
+                content_type="image/png",
             )
             assert new_realm_emoji is not None
         original_realm_emoji_count = RealmEmoji.objects.count()

zerver/tests/test_realm_emoji.py

@@ -17,7 +17,11 @@ class RealmEmojiTest(ZulipTestCase):
     def create_test_emoji(self, name: str, author: UserProfile) -> RealmEmoji:
         with get_test_image_file("img.png") as img_file:
             realm_emoji = check_add_realm_emoji(
-                realm=author.realm, name=name, author=author, image_file=img_file
+                realm=author.realm,
+                name=name,
+                author=author,
+                image_file=img_file,
+                content_type="image/png",
             )
             if realm_emoji is None:
                 raise Exception("Error creating test emoji.")  # nocoverage
@@ -395,7 +399,11 @@ class RealmEmojiTest(ZulipTestCase):
             # check in upload_emoji, we need to make this request via
             # that helper rather than via the API.
             check_add_realm_emoji(
-                realm=emoji_author.realm, name=emoji_name, author=emoji_author, image_file=img_file
+                realm=emoji_author.realm,
+                name=emoji_name,
+                author=emoji_author,
+                image_file=img_file,
+                content_type="image/png",
             )
             with self.assertRaises(JsonableError):
                 check_add_realm_emoji(
@@ -403,4 +411,5 @@ class RealmEmojiTest(ZulipTestCase):
                     name=emoji_name,
                     author=emoji_author,
                     image_file=img_file,
+                    content_type="image/png",
                 )

zerver/tests/test_transfer.py

@@ -88,7 +88,9 @@ class TransferUploadsToS3Test(ZulipTestCase):
         emoji_name = "emoji.png"
 
         with get_test_image_file("img.png") as image_file:
-            emoji = check_add_realm_emoji(othello.realm, emoji_name, othello, image_file)
+            emoji = check_add_realm_emoji(
+                othello.realm, emoji_name, othello, image_file, "image/png"
+            )
 
         emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format(
             realm_id=othello.realm_id,
@@ -112,7 +114,9 @@ class TransferUploadsToS3Test(ZulipTestCase):
         emoji_name = "emoji2.png"
 
         with get_test_image_file("animated_img.gif") as image_file:
-            emoji = check_add_realm_emoji(othello.realm, emoji_name, othello, image_file)
+            emoji = check_add_realm_emoji(
+                othello.realm, emoji_name, othello, image_file, "image/gif"
+            )
 
         emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format(
             realm_id=othello.realm_id,

zerver/tests/test_upload_local.py

@@ -174,7 +174,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase):
         file_name = "emoji.png"
 
         with get_test_image_file("img.png") as image_file:
-            upload_emoji_image(image_file, file_name, user_profile)
+            upload_emoji_image(image_file, file_name, user_profile, "image/png")
         url = zerver.lib.upload.upload_backend.get_emoji_url(file_name, user_profile.realm_id)
 
         emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format(
@@ -186,7 +186,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase):
 
         file_name = "emoji.gif"
         with get_test_image_file("animated_img.gif") as image_file:
-            upload_emoji_image(image_file, file_name, user_profile)
+            upload_emoji_image(image_file, file_name, user_profile, "image/png")
         url = zerver.lib.upload.upload_backend.get_emoji_url(file_name, user_profile.realm_id)
         still_url = zerver.lib.upload.upload_backend.get_emoji_url(
             file_name, user_profile.realm_id, still=True
@@ -211,7 +211,7 @@ class LocalStorageTest(UploadSerializeMixin, ZulipTestCase):
         file_name = "emoji.png"
 
         with get_test_image_file("img.png") as image_file:
-            upload_emoji_image(image_file, file_name, user_profile)
+            upload_emoji_image(image_file, file_name, user_profile, "image/png")
 
         emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format(
             realm_id=user_profile.realm_id,

zerver/tests/test_upload_s3.py

@@ -478,7 +478,7 @@ class S3Test(ZulipTestCase):
         user_profile = self.example_user("hamlet")
         emoji_name = "emoji.png"
         with get_test_image_file("img.png") as image_file:
-            zerver.lib.upload.upload_emoji_image(image_file, emoji_name, user_profile)
+            zerver.lib.upload.upload_emoji_image(image_file, emoji_name, user_profile, "image/png")
 
         emoji_path = RealmEmoji.PATH_ID_TEMPLATE.format(
             realm_id=user_profile.realm_id,

zerver/views/realm_emoji.py

@@ -9,6 +9,7 @@ from zerver.lib.emoji import check_remove_custom_emoji, check_valid_emoji_name,
 from zerver.lib.exceptions import JsonableError, ResourceNotFoundError
 from zerver.lib.request import REQ, has_request_variables
 from zerver.lib.response import json_success
+from zerver.lib.upload import get_file_info
 from zerver.models import RealmEmoji, UserProfile
 from zerver.models.realm_emoji import get_all_custom_emoji_for_realm
 
@@ -51,7 +52,8 @@ def upload_emoji(
             )
         )
 
-    check_add_realm_emoji(user_profile.realm, emoji_name, user_profile, emoji_file)
+    _filename, content_type = get_file_info(emoji_file)
+    check_add_realm_emoji(user_profile.realm, emoji_name, user_profile, emoji_file, content_type)
     return json_success(request)
 
 

zilencer/management/commands/populate_db.py

@@ -913,7 +913,9 @@ class Command(ZulipBaseCommand):
         # Create a test realm emoji.
         IMAGE_FILE_PATH = static_path("images/test-images/checkbox.png")
         with open(IMAGE_FILE_PATH, "rb") as fp:
-            check_add_realm_emoji(zulip_realm, "green_tick", iago, File(fp, name="checkbox.png"))
+            check_add_realm_emoji(
+                zulip_realm, "green_tick", iago, File(fp, name="checkbox.png"), "image/png"
+            )
 
         if not options["test_suite"]:
             # Populate users with some bar data