feat: Hard-limit original image byte size to 2 * max_bytes if recoding fails (#8296)

src/blob.rs

@@ -280,10 +280,18 @@ impl<'a> BlobObject<'a> {
     }
 
     /// Checks or recodes an image pointed by the [BlobObject] so that it fits into limits on the
-    /// image width, height and file size specified by the config.
+    /// image width, height and file size specified by the config, and has Exif removed.
     ///
-    /// Recoding is only done for [`Viewtype::Image`]. For [`Viewtype::File`], if it's a correct
-    /// image, `*viewtype` is set to [`Viewtype::Image`].
+    /// Recoding is only done for [`Viewtype::Image`]. For [`Viewtype::File`], if the image format
+    /// can be guessed from the content or file path, `*viewtype` is set to [`Viewtype::Image`]. NB:
+    /// Decoding isn't performed in this case to save CPU, so the image may be invalid.
+    ///
+    /// If the image needs to be recoded, but that fails, `*viewtype` downgrades to
+    /// [`Viewtype::File`] if the image doesn't contain Exif and doesn't exceed the configured limit
+    /// (see [`Config::MediaQuality`]) multiplied by [`constants::IMAGE_BYTES_TOLERANCE`]. NB: If
+    /// the user chooses [`Viewtype::File`] initially, we end up with [`Viewtype::Image`] in this
+    /// case. This looks inconsistent, but it's unlikely that a failed-to-recode image would be
+    /// displayed by UIs.
     pub async fn check_or_recode_image(
         &mut self,
         context: &Context,
@@ -329,15 +337,20 @@ impl<'a> BlobObject<'a> {
     ) -> Result<String> {
         // Add white background only to avatars to spare the CPU.
         let mut add_white_bg = is_avatar;
-        let mut no_exif = false;
-        let no_exif_ref = &mut no_exif;
+        let mut can_use_original = false;
+        let can_use_original_ref = &mut can_use_original;
         let mut name = name.unwrap_or_else(|| self.name.clone());
         let original_name = name.clone();
         let vt = &mut *viewtype;
         let res: Result<String> = tokio::task::block_in_place(move || {
             let mut file = std::fs::File::open(self.to_abs_path())?;
             let (nr_bytes, exif) = image_metadata(&file)?;
-            *no_exif_ref = exif.is_none();
+            // `max_bytes` isn't a strict limit for non-avatars, still, we don't want to send a
+            // gigabyte if recoding fails.
+            *can_use_original_ref = exif.is_none()
+                && nr_bytes
+                    <= u64::try_from(max_bytes)?
+                        .saturating_mul(constants::IMAGE_BYTES_TOLERANCE.try_into()?);
             // It's strange that BufReader modifies a file position while it takes a non-mut
             // reference. Ok, just rewind it.
             file.rewind()?;
@@ -390,7 +403,7 @@ impl<'a> BlobObject<'a> {
             } else {
                 max(img.width(), img.height())
             };
-            let exceeds_max_bytes = nr_bytes > max_bytes as u64;
+            let exceeds_max_bytes = nr_bytes > u64::try_from(max_bytes)?;
 
             let jpeg_quality = 75;
             let ofmt = match fmt {
@@ -502,11 +515,12 @@ impl<'a> BlobObject<'a> {
         match res {
             Ok(_) => res,
             Err(err) => {
-                if !is_avatar && no_exif {
+                if !is_avatar && can_use_original {
                     error!(
                         context,
                         "Cannot check/recode image, using original data: {err:#}.",
                     );
+                    // See `check_or_recode_image()` docs for reasoning.
                     *viewtype = Viewtype::File;
                     Ok(original_name)
                 } else {

src/constants.rs

@@ -181,9 +181,14 @@ pub const DC_LP_AUTH_NORMAL: i32 = 0x4;
 /// if none of these flags are set, the default is chosen
 pub const DC_LP_AUTH_FLAGS: i32 = DC_LP_AUTH_OAUTH2 | DC_LP_AUTH_NORMAL;
 
-// max. weight of images to send w/o recoding
+// Max. weight of images to send w/o recoding. However, these aren't strict limits for sending: if
+// after recoding an image is bigger, it's still sent. We don't expect to exceed these limits too
+// much though because we use fixed JPEG quality. However, if recoding fails, the original image is
+// sent if it doesn't contain Exif and doesn't exceed the configured limit multiplied by
+// IMAGE_BYTES_TOLERANCE.
 pub const BALANCED_IMAGE_BYTES: usize = 500_000;
 pub const WORSE_IMAGE_BYTES: usize = 130_000;
+pub const IMAGE_BYTES_TOLERANCE: usize = 2;
 
 // max. width/height and bytes of an avatar
 pub(crate) const BALANCED_AVATAR_SIZE: u32 = 512;