Remove metadata from avatars and JPEG images before sending (#4027)

iequidoo · iequidoo · commit 3c7a285e28f7 · 2023-03-11T11:26:48.000-03:00
If there's an Exif, rewrite the file to remove it. This implies recoding now though.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@
 - Run `cargo-deny` in CI. #4101
 - Check provider database with CI. #4099 
 - Switch to DEFERRED transactions #4100
+- Remove metadata from avatars and JPEG images before sending #4037
 
 ### Fixes
 - Do not block async task executor while decrypting the messages. #4079
diff --git a/src/blob.rs b/src/blob.rs
@@ -368,61 +368,64 @@ impl<'a> BlobObject<'a> {
     ) -> Result<Option<String>> {
         tokio::task::block_in_place(move || {
             let mut img = image::open(&blob_abs).context("image recode failure")?;
-            let orientation = self.get_exif_orientation(context);
+            let exif = self.get_exif().ok();
+            let orientation = exif.as_ref().map(|exif| exif_orientation(exif, context));
             let mut encoded = Vec::new();
             let mut changed_name = None;
 
             let exceeds_width = img.width() > img_wh || img.height() > img_wh;
 
             let do_scale =
                 exceeds_width || encoded_img_exceeds_bytes(context, &img, max_bytes, &mut encoded)?;
-            let do_rotate = matches!(orientation, Ok(90) | Ok(180) | Ok(270));
-
-            if do_scale || do_rotate {
-                if do_rotate {
-                    img = match orientation {
-                        Ok(90) => img.rotate90(),
-                        Ok(180) => img.rotate180(),
-                        Ok(270) => img.rotate270(),
-                        _ => img,
-                    }
+            let do_rotate = matches!(orientation, Some(90) | Some(180) | Some(270));
+
+            if do_rotate {
+                img = match orientation {
+                    Some(90) => img.rotate90(),
+                    Some(180) => img.rotate180(),
+                    Some(270) => img.rotate270(),
+                    _ => img,
                 }
+            }
 
-                if do_scale {
-                    if !exceeds_width {
-                        // The image is already smaller than img_wh, but exceeds max_bytes
-                        // We can directly start with trying to scale down to 2/3 of its current width
-                        img_wh = max(img.width(), img.height()) * 2 / 3
-                    }
+            if do_scale {
+                if !exceeds_width {
+                    // The image is already smaller than img_wh, but exceeds max_bytes
+                    // We can directly start with trying to scale down to 2/3 of its current width
+                    img_wh = max(img.width(), img.height()) * 2 / 3
+                }
+
+                loop {
+                    let new_img = img.thumbnail(img_wh, img_wh);
+
+                    if encoded_img_exceeds_bytes(context, &new_img, max_bytes, &mut encoded)? {
+                        if img_wh < 20 {
+                            return Err(format_err!(
+                                "Failed to scale image to below {}B",
+                                max_bytes.unwrap_or_default()
+                            ));
+                        }
 
-                    loop {
-                        let new_img = img.thumbnail(img_wh, img_wh);
-
-                        if encoded_img_exceeds_bytes(context, &new_img, max_bytes, &mut encoded)? {
-                            if img_wh < 20 {
-                                return Err(format_err!(
-                                    "Failed to scale image to below {}B",
-                                    max_bytes.unwrap_or_default()
-                                ));
-                            }
-
-                            img_wh = img_wh * 2 / 3;
-                        } else {
-                            if encoded.is_empty() {
-                                encode_img(&new_img, &mut encoded)?;
-                            }
-
-                            info!(
-                                context,
-                                "Final scaled-down image size: {}B ({}px)",
-                                encoded.len(),
-                                img_wh
-                            );
-                            break;
+                        img_wh = img_wh * 2 / 3;
+                    } else {
+                        if encoded.is_empty() {
+                            encode_img(&new_img, &mut encoded)?;
                         }
+
+                        info!(
+                            context,
+                            "Final scaled-down image size: {}B ({}px)",
+                            encoded.len(),
+                            img_wh
+                        );
+                        break;
                     }
                 }
+            }
 
+            // We also need to rewrite the file to remove metadata such as location, camera model,
+            // etc. if any
+            if do_rotate || do_scale || exif.is_some() {
                 // The file format is JPEG now, we may have to change the file extension
                 if !matches!(ImageFormat::from_path(&blob_abs), Ok(ImageFormat::Jpeg)) {
                     blob_abs = blob_abs.with_extension("jpg");
@@ -443,23 +446,26 @@ impl<'a> BlobObject<'a> {
         })
     }
 
-    pub fn get_exif_orientation(&self, context: &Context) -> Result<i32> {
+    pub fn get_exif(&self) -> Result<exif::Exif> {
         let file = std::fs::File::open(self.to_abs_path())?;
         let mut bufreader = std::io::BufReader::new(&file);
         let exifreader = exif::Reader::new();
-        let exif = exifreader.read_from_container(&mut bufreader)?;
-        if let Some(orientation) = exif.get_field(exif::Tag::Orientation, exif::In::PRIMARY) {
-            // possible orientation values are described at http://sylvana.net/jpegcrop/exif_orientation.html
-            // we only use rotation, in practise, flipping is not used.
-            match orientation.value.get_uint(0) {
-                Some(3) => return Ok(180),
-                Some(6) => return Ok(90),
-                Some(8) => return Ok(270),
-                other => warn!(context, "exif orientation value ignored: {:?}", other),
-            }
+        Ok(exifreader.read_from_container(&mut bufreader)?)
+    }
+}
+
+fn exif_orientation(exif: &exif::Exif, context: &Context) -> i32 {
+    if let Some(orientation) = exif.get_field(exif::Tag::Orientation, exif::In::PRIMARY) {
+        // possible orientation values are described at http://sylvana.net/jpegcrop/exif_orientation.html
+        // we only use rotation, in practise, flipping is not used.
+        match orientation.value.get_uint(0) {
+            Some(3) => return 180,
+            Some(6) => return 90,
+            Some(8) => return 270,
+            other => warn!(context, "exif orientation value ignored: {:?}", other),
         }
-        Ok(0)
     }
+    0
 }
 
 impl<'a> fmt::Display for BlobObject<'a> {
@@ -796,12 +802,22 @@ mod tests {
     async fn test_recode_image_1() {
         let bytes = include_bytes!("../test-data/image/avatar1000x1000.jpg");
         // BALANCED_IMAGE_SIZE > 1000, the original image size, so the image is not scaled down:
-        send_image_check_mediaquality(Some("0"), bytes, 1000, 1000, 0, 1000, 1000)
-            .await
-            .unwrap();
+        send_image_check_mediaquality(
+            Some("0"),
+            bytes,
+            true, // has Exif
+            1000,
+            1000,
+            0,
+            1000,
+            1000,
+        )
+        .await
+        .unwrap();
         send_image_check_mediaquality(
             Some("1"),
             bytes,
+            true, // has Exif
             1000,
             1000,
             0,
@@ -819,6 +835,7 @@ mod tests {
         let img_rotated = send_image_check_mediaquality(
             Some("0"),
             bytes,
+            true, // has Exif
             2000,
             1800,
             270,
@@ -842,6 +859,7 @@ mod tests {
             let img_rotated = send_image_check_mediaquality(
                 Some("0"),
                 &bytes2,
+                false, // no Exif
                 BALANCED_IMAGE_SIZE * 1800 / 2000,
                 BALANCED_IMAGE_SIZE,
                 0,
@@ -856,6 +874,7 @@ mod tests {
         let img_rotated = send_image_check_mediaquality(
             Some("1"),
             &bytes,
+            false, // no Exif
             BALANCED_IMAGE_SIZE * 1800 / 2000,
             BALANCED_IMAGE_SIZE,
             0,
@@ -872,15 +891,33 @@ mod tests {
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_recode_image_3() {
         let bytes = include_bytes!("../test-data/image/rectangle200x180-rotated.jpg");
-        let img_rotated = send_image_check_mediaquality(Some("0"), bytes, 200, 180, 270, 180, 200)
-            .await
-            .unwrap();
+        let img_rotated = send_image_check_mediaquality(
+            Some("0"),
+            bytes,
+            true, // has Exif
+            200,
+            180,
+            270,
+            180,
+            200,
+        )
+        .await
+        .unwrap();
         assert_correct_rotation(&img_rotated);
 
         let bytes = include_bytes!("../test-data/image/rectangle200x180-rotated.jpg");
-        let img_rotated = send_image_check_mediaquality(Some("1"), bytes, 200, 180, 270, 180, 200)
-            .await
-            .unwrap();
+        let img_rotated = send_image_check_mediaquality(
+            Some("1"),
+            bytes,
+            true, // has Exif
+            200,
+            180,
+            270,
+            180,
+            200,
+        )
+        .await
+        .unwrap();
         assert_correct_rotation(&img_rotated);
     }
 
@@ -901,9 +938,11 @@ mod tests {
         assert_eq!(luma, 0);
     }
 
+    #[allow(clippy::too_many_arguments)]
     async fn send_image_check_mediaquality(
         media_quality_config: Option<&str>,
         bytes: &[u8],
+        has_exif: bool,
         original_width: u32,
         original_height: u32,
         orientation: i32,
@@ -923,7 +962,13 @@ mod tests {
         check_image_size(&file, original_width, original_height);
 
         let blob = BlobObject::new_from_path(&alice, &file).await?;
-        assert_eq!(blob.get_exif_orientation(&alice).unwrap_or(0), orientation);
+        let exif = blob.get_exif();
+        if has_exif {
+            let exif = exif.unwrap();
+            assert_eq!(exif_orientation(&exif, &alice), orientation);
+        } else {
+            assert!(exif.is_err());
+        }
 
         let mut msg = Message::new(Viewtype::Image);
         msg.set_file(file.to_str().unwrap(), None);
@@ -944,7 +989,7 @@ mod tests {
         let file = bob_msg.get_file(&bob).unwrap();
 
         let blob = BlobObject::new_from_path(&bob, &file).await?;
-        assert_eq!(blob.get_exif_orientation(&bob).unwrap_or(0), 0);
+        assert!(blob.get_exif().is_err());
 
         let img = check_image_size(file, compressed_width, compressed_height);
         Ok(img)
diff --git a/test-data/image/avatar1000x1000.jpg b/test-data/image/avatar1000x1000.jpg
