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

src/blob.rs

@@ -329,15 +329,17 @@ 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)? * 2;
             // It's strange that BufReader modifies a file position while it takes a non-mut
             // reference. Ok, just rewind it.
             file.rewind()?;
@@ -390,7 +392,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 +504,15 @@ 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:#}.",
                     );
+                    // NB: If the image can't be decoded, but the user had chosen `Viewtype::File`
+                    // initially, we wouldn't have gotten here and would have ended up with
+                    // `Viewtype::Image`. So choosing `Viewtype::File` here looks inconsistent, but
+                    // it's unlikely that otherwise the image would be displayed by UIs.
                     *viewtype = Viewtype::File;
                     Ok(original_name)
                 } else {