diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 6de519c10..3506c7891 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -21,6 +21,7 @@ use std::ptr; use std::str::FromStr; use std::time::{Duration, SystemTime}; +use anyhow::Context as _; use async_std::task::{block_on, spawn}; use num_traits::{FromPrimitive, ToPrimitive}; @@ -130,12 +131,14 @@ pub unsafe extern "C" fn dc_set_config( return 0; } let ctx = &*context; - match config::Config::from_str(&to_string_lossy(key)) { - // When ctx.set_config() fails it already logged the error. - // TODO: Context::set_config() should not log this + let key = to_string_lossy(key); + match config::Config::from_str(&key) { Ok(key) => block_on(async move { - ctx.set_config(key, to_opt_string_lossy(value).as_deref()) + let value = to_opt_string_lossy(value); + ctx.set_config(key, value.as_deref()) .await + .with_context(|| format!("Can't set {} to {:?}", key, value)) + .log_err(ctx, "dc_set_config() failed") .is_ok() as libc::c_int }), Err(_) => { diff --git a/src/blob.rs b/src/blob.rs index e52152fc0..7892d17f2 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -1,5 +1,6 @@ //! # Blob directory management +use core::cmp::max; use std::ffi::OsStr; use std::fmt; @@ -7,8 +8,12 @@ use async_std::path::{Path, PathBuf}; use async_std::prelude::*; use async_std::{fs, io}; +use anyhow::format_err; +use anyhow::Context as _; use anyhow::Error; +use image::DynamicImage; use image::GenericImageView; +use image::ImageFormat; use num_traits::FromPrimitive; use thiserror::Error; @@ -380,7 +385,7 @@ impl<'a> BlobObject<'a> { true } - pub async fn recode_to_avatar_size(&self, context: &Context) -> Result<(), BlobError> { + pub async fn recode_to_avatar_size(&mut self, context: &Context) -> Result<(), BlobError> { let blob_abs = self.to_abs_path(); let img_wh = @@ -391,7 +396,15 @@ impl<'a> BlobObject<'a> { MediaQuality::Worse => WORSE_AVATAR_SIZE, }; - self.recode_to_size(context, blob_abs, img_wh).await + // max_bytes is 20_000 bytes: Outlook servers don't allow headers larger than 32k. + // 32 / 4 * 3 = 24k if you account for base64 encoding. To be safe, we reduced this to 20k. + if let Some(new_name) = self + .recode_to_size(context, blob_abs, img_wh, Some(20_000)) + .await? + { + self.name = new_name; + } + Ok(()) } pub async fn recode_to_image_size(&self, context: &Context) -> Result<(), BlobError> { @@ -410,30 +423,69 @@ impl<'a> BlobObject<'a> { MediaQuality::Worse => WORSE_IMAGE_SIZE, }; - self.recode_to_size(context, blob_abs, img_wh).await + if self + .recode_to_size(context, blob_abs, img_wh, None) + .await? + .is_some() + { + return Err(format_err!( + "Internal error: recode_to_size(..., None) shouldn't change the name of the image" + ) + .into()); + } + Ok(()) } async fn recode_to_size( &self, context: &Context, - blob_abs: PathBuf, - img_wh: u32, - ) -> Result<(), BlobError> { + mut blob_abs: PathBuf, + mut img_wh: u32, + max_bytes: Option, + ) -> Result, BlobError> { let mut img = image::open(&blob_abs).map_err(|err| BlobError::RecodeFailure { blobdir: context.get_blobdir().to_path_buf(), blobname: blob_abs.to_str().unwrap_or_default().to_string(), cause: err, })?; let orientation = self.get_exif_orientation(context); + let mut encoded = Vec::new(); + let mut changed_name = None; - let do_scale = img.width() > img_wh || img.height() > img_wh; + fn encode_img(img: &DynamicImage, encoded: &mut Vec) -> anyhow::Result<()> { + encoded.clear(); + img.write_to(encoded, image::ImageFormat::Jpeg)?; + Ok(()) + } + fn encode_img_exceeds_bytes( + context: &Context, + img: &DynamicImage, + max_bytes: Option, + encoded: &mut Vec, + ) -> anyhow::Result { + if let Some(max_bytes) = max_bytes { + encode_img(img, encoded)?; + if encoded.len() > max_bytes { + info!( + context, + "image size {}B ({}x{}px) exceeds {}B, need to scale down", + encoded.len(), + img.width(), + img.height(), + max_bytes, + ); + return Ok(true); + } + } + Ok(false) + } + let exceeds_width = img.width() > img_wh || img.height() > img_wh; + + let do_scale = + exceeds_width || encode_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_scale { - img = img.thumbnail(img_wh, img_wh); - } - if do_rotate { img = match orientation { Ok(90) => img.rotate90(), @@ -443,14 +495,60 @@ impl<'a> BlobObject<'a> { } } - img.save(&blob_abs).map_err(|err| BlobError::WriteFailure { - blobdir: context.get_blobdir().to_path_buf(), - blobname: blob_abs.to_str().unwrap_or_default().to_string(), - cause: err.into(), - })?; + 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 encode_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() + ) + .into()); + } + + img_wh = img_wh * 2 / 3; + } else { + info!( + context, + "Final scaled-down image size: {}B ({}px)", + encoded.len(), + img_wh + ); + break; + } + } + } + + // 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"); + let file_name = blob_abs.file_name().context("No avatar file name (???)")?; + let file_name = file_name.to_str().context("Filename is no UTF-8 (???)")?; + changed_name = Some(format!("$BLOBDIR/{}", file_name)); + } + + if encoded.is_empty() { + encode_img(&img, &mut encoded)?; + } + + fs::write(&blob_abs, &encoded) + .await + .map_err(|err| BlobError::WriteFailure { + blobdir: context.get_blobdir().to_path_buf(), + blobname: blob_abs.to_str().unwrap_or_default().to_string(), + cause: err.into(), + })?; } - Ok(()) + Ok(changed_name) } pub fn get_exif_orientation(&self, context: &Context) -> Result { @@ -522,6 +620,8 @@ pub enum BlobError { #[cfg(test)] mod tests { + use fs::File; + use super::*; use crate::test_utils::TestContext; @@ -715,4 +815,105 @@ mod tests { assert!(!stem.contains('*')); assert!(!stem.contains('?')); } + + #[async_std::test] + async fn test_selfavatar_outside_blobdir() { + let t = TestContext::new().await; + let avatar_src = t.dir.path().join("avatar.jpg"); + let avatar_bytes = include_bytes!("../test-data/image/avatar1000x1000.jpg"); + File::create(&avatar_src) + .await + .unwrap() + .write_all(avatar_bytes) + .await + .unwrap(); + let avatar_blob = t.get_blobdir().join("avatar.jpg"); + assert!(!avatar_blob.exists().await); + t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) + .await + .unwrap(); + assert!(avatar_blob.exists().await); + assert!(std::fs::metadata(&avatar_blob).unwrap().len() < avatar_bytes.len() as u64); + let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap(); + assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string())); + + let img = image::open(avatar_src).unwrap(); + assert_eq!(img.width(), 1000); + assert_eq!(img.height(), 1000); + + let img = image::open(&avatar_blob).unwrap(); + assert_eq!(img.width(), BALANCED_AVATAR_SIZE); + assert_eq!(img.height(), BALANCED_AVATAR_SIZE); + + async fn file_size(path_buf: &PathBuf) -> u64 { + let file = File::open(path_buf).await.unwrap(); + file.metadata().await.unwrap().len() + } + + let blob = BlobObject::new_from_path(&t, &avatar_blob).await.unwrap(); + + blob.recode_to_size(&t, blob.to_abs_path(), 1000, Some(3000)) + .await + .unwrap(); + assert!(file_size(&avatar_blob).await <= 3000); + assert!(file_size(&avatar_blob).await > 2000); + let img = image::open(&avatar_blob).unwrap(); + assert!(img.width() > 130); + assert_eq!(img.width(), img.height()); + } + + #[async_std::test] + async fn test_selfavatar_in_blobdir() { + let t = TestContext::new().await; + let avatar_src = t.get_blobdir().join("avatar.png"); + let avatar_bytes = include_bytes!("../test-data/image/avatar900x900.png"); + File::create(&avatar_src) + .await + .unwrap() + .write_all(avatar_bytes) + .await + .unwrap(); + + let img = image::open(&avatar_src).unwrap(); + assert_eq!(img.width(), 900); + assert_eq!(img.height(), 900); + + t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) + .await + .unwrap(); + let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap().unwrap(); + assert_eq!( + avatar_cfg, + avatar_src.with_extension("jpg").to_str().unwrap() + ); + + let img = image::open(avatar_cfg).unwrap(); + assert_eq!(img.width(), BALANCED_AVATAR_SIZE); + assert_eq!(img.height(), BALANCED_AVATAR_SIZE); + } + + #[async_std::test] + async fn test_selfavatar_copy_without_recode() { + let t = TestContext::new().await; + let avatar_src = t.dir.path().join("avatar.png"); + let avatar_bytes = include_bytes!("../test-data/image/avatar64x64.png"); + File::create(&avatar_src) + .await + .unwrap() + .write_all(avatar_bytes) + .await + .unwrap(); + let avatar_blob = t.get_blobdir().join("avatar.png"); + assert!(!avatar_blob.exists().await); + t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) + .await + .unwrap(); + assert!(avatar_blob.exists().await); + assert_eq!( + std::fs::metadata(&avatar_blob).unwrap().len(), + avatar_bytes.len() as u64 + ); + let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap(); + assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string())); + } } diff --git a/src/chat.rs b/src/chat.rs index 0908eac9c..75aaf638c 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2660,7 +2660,7 @@ pub async fn set_chat_profile_image( msg.param.remove(Param::Arg); msg.text = Some(stock_str::msg_grp_img_deleted(context, DC_CONTACT_ID_SELF as u32).await); } else { - let image_blob = match BlobObject::from_path(context, Path::new(new_image.as_ref())) { + let mut image_blob = match BlobObject::from_path(context, Path::new(new_image.as_ref())) { Ok(blob) => Ok(blob), Err(err) => match err { BlobError::WrongBlobdir { .. } => { diff --git a/src/config.rs b/src/config.rs index 67bcf4ff7..9e16ef387 100644 --- a/src/config.rs +++ b/src/config.rs @@ -249,7 +249,7 @@ impl Context { .await?; match value { Some(value) => { - let blob = BlobObject::new_from_path(self, value).await?; + let mut blob = BlobObject::new_from_path(self, value).await?; blob.recode_to_avatar_size(self).await?; self.sql.set_raw_config(key, Some(blob.as_name())).await?; Ok(()) @@ -331,12 +331,8 @@ mod tests { use std::string::ToString; use crate::constants; - use crate::constants::BALANCED_AVATAR_SIZE; use crate::test_utils::TestContext; - use image::GenericImageView; use num_traits::FromPrimitive; - use std::fs::File; - use std::io::Write; #[test] fn test_to_string() { @@ -350,82 +346,6 @@ mod tests { ); } - #[async_std::test] - async fn test_selfavatar_outside_blobdir() { - let t = TestContext::new().await; - let avatar_src = t.dir.path().join("avatar.jpg"); - let avatar_bytes = include_bytes!("../test-data/image/avatar1000x1000.jpg"); - File::create(&avatar_src) - .unwrap() - .write_all(avatar_bytes) - .unwrap(); - let avatar_blob = t.get_blobdir().join("avatar.jpg"); - assert!(!avatar_blob.exists().await); - t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) - .await - .unwrap(); - assert!(avatar_blob.exists().await); - assert!(std::fs::metadata(&avatar_blob).unwrap().len() < avatar_bytes.len() as u64); - let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap(); - assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string())); - - let img = image::open(avatar_src).unwrap(); - assert_eq!(img.width(), 1000); - assert_eq!(img.height(), 1000); - - let img = image::open(avatar_blob).unwrap(); - assert_eq!(img.width(), BALANCED_AVATAR_SIZE); - assert_eq!(img.height(), BALANCED_AVATAR_SIZE); - } - - #[async_std::test] - async fn test_selfavatar_in_blobdir() { - let t = TestContext::new().await; - let avatar_src = t.get_blobdir().join("avatar.png"); - let avatar_bytes = include_bytes!("../test-data/image/avatar900x900.png"); - File::create(&avatar_src) - .unwrap() - .write_all(avatar_bytes) - .unwrap(); - - let img = image::open(&avatar_src).unwrap(); - assert_eq!(img.width(), 900); - assert_eq!(img.height(), 900); - - t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) - .await - .unwrap(); - let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap(); - assert_eq!(avatar_cfg, avatar_src.to_str().map(|s| s.to_string())); - - let img = image::open(avatar_src).unwrap(); - assert_eq!(img.width(), BALANCED_AVATAR_SIZE); - assert_eq!(img.height(), BALANCED_AVATAR_SIZE); - } - - #[async_std::test] - async fn test_selfavatar_copy_without_recode() { - let t = TestContext::new().await; - let avatar_src = t.dir.path().join("avatar.png"); - let avatar_bytes = include_bytes!("../test-data/image/avatar64x64.png"); - File::create(&avatar_src) - .unwrap() - .write_all(avatar_bytes) - .unwrap(); - let avatar_blob = t.get_blobdir().join("avatar.png"); - assert!(!avatar_blob.exists().await); - t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) - .await - .unwrap(); - assert!(avatar_blob.exists().await); - assert_eq!( - std::fs::metadata(&avatar_blob).unwrap().len(), - avatar_bytes.len() as u64 - ); - let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap(); - assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string())); - } - #[async_std::test] async fn test_media_quality_config_option() { let t = TestContext::new().await; diff --git a/src/sql.rs b/src/sql.rs index 98b2596c2..4b67b3183 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -11,6 +11,7 @@ use anyhow::Context as _; use async_std::prelude::*; use rusqlite::OpenFlags; +use crate::blob::BlobObject; use crate::chat::{add_device_msg, update_device_icon, update_saved_messages_icon}; use crate::config::Config; use crate::constants::{Viewtype, DC_CHAT_ID_TRASH}; @@ -138,7 +139,7 @@ impl Sql { // this should be done before updates that use high-level objects that // rely themselves on the low-level structure. - let (recalc_fingerprints, update_icons, disable_server_delete) = + let (recalc_fingerprints, update_icons, disable_server_delete, recode_avatar) = migrations::run(context, self).await?; // (2) updates that require high-level objects @@ -183,6 +184,23 @@ impl Sql { .await?; } } + + if recode_avatar { + if let Some(avatar) = context.get_config(Config::Selfavatar).await? { + let mut blob = BlobObject::new_from_path(context, &avatar).await?; + match blob.recode_to_avatar_size(context).await { + Ok(()) => { + context + .set_config(Config::Selfavatar, Some(&avatar)) + .await? + } + Err(e) => { + warn!(context, "Migrations can't recode avatar, removing. {:#}", e); + context.set_config(Config::Selfavatar, None).await? + } + } + } + } } info!(context, "Opened {:?}.", dbfile.as_ref()); diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index c26a0bfac..ae40bc38a 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -10,7 +10,7 @@ const DBVERSION: i32 = 68; const VERSION_CFG: &str = "dbversion"; const TABLES: &str = include_str!("./tables.sql"); -pub async fn run(context: &Context, sql: &Sql) -> Result<(bool, bool, bool)> { +pub async fn run(context: &Context, sql: &Sql) -> Result<(bool, bool, bool, bool)> { let mut recalc_fingerprints = false; let mut exists_before_update = false; let mut dbversion_before_update = DBVERSION; @@ -39,6 +39,7 @@ pub async fn run(context: &Context, sql: &Sql) -> Result<(bool, bool, bool)> { let dbversion = dbversion_before_update; let mut update_icons = !exists_before_update; let mut disable_server_delete = false; + let mut recode_avatar = false; if dbversion < 1 { info!(context, "[migration] v1"); @@ -460,8 +461,17 @@ paramsv![] sql.execute_migration("ALTER TABLE msgs ADD COLUMN subject TEXT DEFAULT '';", 76) .await?; } + if dbversion < 77 { + info!(context, "[migration] v77"); + recode_avatar = true; + } - Ok((recalc_fingerprints, update_icons, disable_server_delete)) + Ok(( + recalc_fingerprints, + update_icons, + disable_server_delete, + recode_avatar, + )) } impl Sql {