feat: Check images passed as File before making them Image

We don't want images having unsupported format or corrupted ones to be sent as `Image` and appear in
the "Images" tab in UIs because they can't be displayed correctly.

src/blob.rs

@@ -266,7 +266,7 @@ impl<'a> BlobObject<'a> {
 
         let viewtype = &mut Viewtype::Image;
         let is_avatar = true;
-        self.recode_to_size(
+        self.check_or_recode_to_size(
             context, None, // The name of an avatar doesn't matter
             viewtype, img_wh, max_bytes, is_avatar,
         )?;
@@ -274,13 +274,16 @@ impl<'a> BlobObject<'a> {
         Ok(())
     }
 
-    /// Recodes an image pointed by a [BlobObject] so that it fits into limits on the image width,
+    /// Checks or recodes an image pointed by the [BlobObject] so that it fits into limits on the
-    /// height and file size specified by the config.
+    /// image width, height and file size specified by the config.
+    ///
+    /// Recoding is only done for [`Viewtype::Image`]. For [`Viewtype::File`], if it's a correct
+    /// image, `*viewtype` is set to [`Viewtype::Image`].
     ///
     /// On some platforms images are passed to Core as [`Viewtype::Sticker`]. We recheck if the
     /// image is a true sticker assuming that it must have at least one fully transparent corner,
-    /// otherwise `*viewtype` is set to `Viewtype::Image`.
+    /// otherwise `*viewtype` is set to [`Viewtype::Image`].
-    pub async fn recode_to_image_size(
+    pub async fn check_or_recode_image(
         &mut self,
         context: &Context,
         name: Option<String>,
@@ -297,10 +300,10 @@ impl<'a> BlobObject<'a> {
                 MediaQuality::Worse => (constants::WORSE_IMAGE_SIZE, constants::WORSE_IMAGE_BYTES),
             };
         let is_avatar = false;
-        self.recode_to_size(context, name, viewtype, img_wh, max_bytes, is_avatar)
+        self.check_or_recode_to_size(context, name, viewtype, img_wh, max_bytes, is_avatar)
     }
 
-    /// Recodes the image so that it fits into limits on width/height and byte size.
+    /// Checks or recodes the image so that it fits into limits on width/height and byte size.
     ///
     /// If `!is_avatar`, then if `max_bytes` is exceeded, reduces the image to `img_wh` and proceeds
     /// with the result without rechecking.
@@ -311,7 +314,7 @@ impl<'a> BlobObject<'a> {
     /// then the updated user-visible filename will be returned;
     /// this may be necessary because the format may be changed to JPG,
     /// i.e. "image.png" -> "image.jpg".
-    fn recode_to_size(
+    fn check_or_recode_to_size(
         &mut self,
         context: &Context,
         name: Option<String>,
@@ -346,6 +349,10 @@ impl<'a> BlobObject<'a> {
                 }
             };
             let fmt = imgreader.format().context("Unknown format")?;
+            if *vt == Viewtype::File {
+                *vt = Viewtype::Image;
+                return Ok(name);
+            }
             let mut img = imgreader.decode().context("image decode failure")?;
             let orientation = exif.as_ref().map(|exif| exif_orientation(exif, context));
             let mut encoded = Vec::new();
@@ -499,7 +506,7 @@ impl<'a> BlobObject<'a> {
                 if !is_avatar && no_exif {
                     error!(
                         context,
-                        "Cannot recode image, using original data: {err:#}.",
+                        "Cannot check/recode image, using original data: {err:#}.",
                     );
                     *viewtype = Viewtype::File;
                     Ok(original_name)

src/blob/blob_tests.rs

@@ -142,7 +142,7 @@ async fn test_add_white_bg() {
         let img_wh = 128;
         let viewtype = &mut Viewtype::Image;
         let strict_limits = true;
-        blob.recode_to_size(&t, None, viewtype, img_wh, 20_000, strict_limits)
+        blob.check_or_recode_to_size(&t, None, viewtype, img_wh, 20_000, strict_limits)
             .unwrap();
         tokio::task::block_in_place(move || {
             let img = ImageReader::open(blob.to_abs_path())
@@ -190,7 +190,7 @@ async fn test_selfavatar_outside_blobdir() {
     let mut blob = BlobObject::create_and_deduplicate(&t, avatar_path, avatar_path).unwrap();
     let viewtype = &mut Viewtype::Image;
     let strict_limits = true;
-    blob.recode_to_size(&t, None, viewtype, 1000, 3000, strict_limits)
+    blob.check_or_recode_to_size(&t, None, viewtype, 1000, 3000, strict_limits)
         .unwrap();
     let new_file_size = file_size(&blob.to_abs_path()).await;
     assert!(new_file_size <= 3000);
@@ -399,7 +399,7 @@ async fn test_recode_image_balanced_png() {
     .await
     .unwrap();
 
-    // This will be sent as Image, see [`BlobObject::recode_to_image_size()`] for explanation.
+    // This will be sent as Image, see [`BlobObject::check_or_recode_image()`] for explanation.
     SendImageCheckMediaquality {
         viewtype: Viewtype::Sticker,
         media_quality_config: "0",

src/chat.rs

@@ -2686,7 +2686,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
             .param
             .get_file_blob(context)?
             .with_context(|| format!("attachment missing for message of type #{}", msg.viewtype))?;
-        let send_as_is = msg.viewtype == Viewtype::File;
+        let mut maybe_image = false;
 
         if msg.viewtype == Viewtype::File
             || msg.viewtype == Viewtype::Image
@@ -2704,6 +2704,8 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
                         // UIs don't want conversions of `Sticker` to anything other than `Image`.
                         msg.param.set_int(Param::ForceSticker, 1);
                     }
+                } else if better_type == Viewtype::Image {
+                    maybe_image = true;
                 } else if better_type != Viewtype::Webxdc
                     || context
                         .ensure_sendable_webxdc_file(&blob.to_abs_path())
@@ -2722,12 +2724,12 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
         if msg.viewtype == Viewtype::Vcard {
             msg.try_set_vcard(context, &blob.to_abs_path()).await?;
         }
-        if !send_as_is
+        if msg.viewtype == Viewtype::File && maybe_image
-            && (msg.viewtype == Viewtype::Image
+            || msg.viewtype == Viewtype::Image
-                || msg.viewtype == Viewtype::Sticker && !msg.param.exists(Param::ForceSticker))
+            || msg.viewtype == Viewtype::Sticker && !msg.param.exists(Param::ForceSticker)
         {
             let new_name = blob
-                .recode_to_image_size(context, msg.get_filename(), &mut msg.viewtype)
+                .check_or_recode_image(context, msg.get_filename(), &mut msg.viewtype)
                 .await?;
             msg.param.set(Param::Filename, new_name);
             msg.param.set(Param::File, blob.as_name());

src/chat/chat_tests.rs

@@ -3727,18 +3727,26 @@ async fn test_nonimage_with_png_ext() -> Result<()> {
 
     let bytes = include_bytes!("../../test-data/message/thunderbird_with_autocrypt.eml");
     let file = alice.get_blobdir().join("screenshot.png");
-    tokio::fs::write(&file, bytes).await?;
 
-    let mut msg = Message::new(Viewtype::Image);
+    for vt in [Viewtype::Image, Viewtype::File] {
-    msg.set_file_and_deduplicate(alice, &file, Some("screenshot.png"), None)?;
+        tokio::fs::write(&file, bytes).await?;
-    let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await;
+        let mut msg = Message::new(vt);
-    assert_eq!(msg.viewtype, Viewtype::File);
+        msg.set_file_and_deduplicate(alice, &file, Some("screenshot.png"), None)?;
-    assert_eq!(msg.get_filemime().unwrap(), "application/octet-stream");
+        let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await;
-    assert!(!msg.get_filename().unwrap().contains("screenshot"));
+        assert_eq!(msg.viewtype, Viewtype::File);
-    let msg_bob = bob.recv_msg(&sent_msg).await;
+        assert_eq!(msg.get_filemime().unwrap(), "application/octet-stream");
-    assert_eq!(msg_bob.viewtype, Viewtype::File);
+        assert_eq!(
-    assert_eq!(msg_bob.get_filemime().unwrap(), "application/octet-stream");
+            msg.get_filename().unwrap().contains("screenshot"),
-    assert!(!msg_bob.get_filename().unwrap().contains("screenshot"));
+            vt == Viewtype::File
+        );
+        let msg_bob = bob.recv_msg(&sent_msg).await;
+        assert_eq!(msg_bob.viewtype, Viewtype::File);
+        assert_eq!(msg_bob.get_filemime().unwrap(), "application/octet-stream");
+        assert_eq!(
+            msg_bob.get_filename().unwrap().contains("screenshot"),
+            vt == Viewtype::File
+        );
+    }
     Ok(())
 }