Skip to content

fix: Treat and send images that can't be decoded as Viewtype::File #6904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 8, 2025
Merged

fix: Treat and send images that can't be decoded as Viewtype::File #6904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b19b6e3
fix: Treat "tgs" as Viewtype::File
iequidoo Jun 6, 2025
aa46391
refactor: build_body_file(): Remove guessing mimetype by file extension
iequidoo Jun 6, 2025
a2e261e
fix: Treat and send images that can't be decoded as Viewtype::File
iequidoo Jun 6, 2025
21affed
fix: Decide on filename used for sending depending on the original Vi…
iequidoo Jul 8, 2025
0b6154c
feat: Check images passed as `File` before making them `Image`
iequidoo Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 36 additions & 31 deletions src/blob.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::constants::{self, MediaQuality};
use crate::context::Context;
use crate::events::EventType;
use crate::log::{LogExt, error, info, warn};
use crate::message::Viewtype;
use crate::tools::sanitize_filename;

/// Represents a file in the blob directory.
Expand Down Expand Up @@ -263,32 +264,30 @@ impl<'a> BlobObject<'a> {
}
};

let maybe_sticker = &mut false;
let viewtype = &mut Viewtype::Image;
let is_avatar = true;
self.recode_to_size(
context,
None, // The name of an avatar doesn't matter
maybe_sticker,
img_wh,
max_bytes,
is_avatar,
self.check_or_recode_to_size(
context, None, // The name of an avatar doesn't matter
viewtype, img_wh, max_bytes, is_avatar,
)?;

Ok(())
}

/// Recodes an image pointed by a [BlobObject] so that it fits into limits on the image width,
/// height and file size specified by the config.
/// 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.
///
/// On some platforms images are passed to the core as [`crate::message::Viewtype::Sticker`] in
/// which case `maybe_sticker` flag should be set. We recheck if an image is a true sticker
/// assuming that it must have at least one fully transparent corner, otherwise this flag is
/// reset.
pub async fn recode_to_image_size(
/// 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`].
pub async fn check_or_recode_image(
&mut self,
context: &Context,
name: Option<String>,
maybe_sticker: &mut bool,
viewtype: &mut Viewtype,
) -> Result<String> {
let (img_wh, max_bytes) =
match MediaQuality::from_i32(context.get_config_int(Config::MediaQuality).await?)
Expand All @@ -301,13 +300,10 @@ impl<'a> BlobObject<'a> {
MediaQuality::Worse => (constants::WORSE_IMAGE_SIZE, constants::WORSE_IMAGE_BYTES),
};
let is_avatar = false;
let new_name =
self.recode_to_size(context, name, maybe_sticker, img_wh, max_bytes, is_avatar)?;

Ok(new_name)
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.
Expand All @@ -318,11 +314,11 @@ 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>,
maybe_sticker: &mut bool,
viewtype: &mut Viewtype,
mut img_wh: u32,
max_bytes: usize,
is_avatar: bool,
Expand All @@ -333,6 +329,7 @@ impl<'a> BlobObject<'a> {
let no_exif_ref = &mut no_exif;
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)?;
Expand All @@ -351,21 +348,28 @@ impl<'a> BlobObject<'a> {
)
}
};
let fmt = imgreader.format().context("No format??")?;
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();

if *maybe_sticker {
if *vt == Viewtype::Sticker {
let x_max = img.width().saturating_sub(1);
let y_max = img.height().saturating_sub(1);
*maybe_sticker = img.in_bounds(x_max, y_max)
&& (img.get_pixel(0, 0).0[3] == 0
if !img.in_bounds(x_max, y_max)
|| !(img.get_pixel(0, 0).0[3] == 0
|| img.get_pixel(x_max, 0).0[3] == 0
|| img.get_pixel(0, y_max).0[3] == 0
|| img.get_pixel(x_max, y_max).0[3] == 0);
|| img.get_pixel(x_max, y_max).0[3] == 0)
{
*vt = Viewtype::Image;
}
}
if *maybe_sticker && exif.is_none() {
if *vt == Viewtype::Sticker && exif.is_none() {
return Ok(name);
}

Expand Down Expand Up @@ -500,10 +504,11 @@ impl<'a> BlobObject<'a> {
Ok(_) => res,
Err(err) => {
if !is_avatar && no_exif {
warn!(
error!(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error! messages are displayed as a toast in the UI, do we really want it here? Not being able to decode an image is not an exceptional state, users may drop HEIF images into the UI and we cannot prevent this.

Copy link
Collaborator Author

@iequidoo iequidoo Jul 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, this error is not translated, but i wanted to warn the user that the original data will be sent which may contain sensitive metadata, and the message may be huge.

EDIT: no_exif is false by default, so metadata isn't a problem. But the original file name would preserve.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed another commit, fix: Decide on filename used for sending depending on the original Viewtype, fixing this problem. Still i'd prefer this to be error! to warn the user that the message may be huge

context,
"Cannot recode image, using original data: {err:#}.",
"Cannot check/recode image, using original data: {err:#}.",
);
*viewtype = Viewtype::File;
Ok(original_name)
} else {
Err(err)
Expand Down
10 changes: 5 additions & 5 deletions src/blob/blob_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ async fn test_add_white_bg() {

let mut blob = BlobObject::create_and_deduplicate(&t, &avatar_src, &avatar_src).unwrap();
let img_wh = 128;
let maybe_sticker = &mut false;
let viewtype = &mut Viewtype::Image;
let strict_limits = true;
blob.recode_to_size(&t, None, maybe_sticker, 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())
Expand Down Expand Up @@ -188,9 +188,9 @@ async fn test_selfavatar_outside_blobdir() {
);

let mut blob = BlobObject::create_and_deduplicate(&t, avatar_path, avatar_path).unwrap();
let maybe_sticker = &mut false;
let viewtype = &mut Viewtype::Image;
let strict_limits = true;
blob.recode_to_size(&t, None, maybe_sticker, 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);
Expand Down Expand Up @@ -399,7 +399,7 @@ async fn test_recode_image_balanced_png() {
.await
.unwrap();

// This will be sent as Image, see [`BlobObject::maybe_sticker`] for explanation.
// This will be sent as Image, see [`BlobObject::check_or_recode_image()`] for explanation.
SendImageCheckMediaquality {
viewtype: Viewtype::Sticker,
media_quality_config: "0",
Expand Down
109 changes: 75 additions & 34 deletions src/chat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::str::FromStr;
use std::time::Duration;

use anyhow::{Context as _, Result, anyhow, bail, ensure};
use chrono::TimeZone;
use deltachat_contact_tools::{ContactAddress, sanitize_bidi_characters, sanitize_single_line};
use deltachat_derive::{FromSql, ToSql};
use mail_builder::mime::MimePart;
Expand Down Expand Up @@ -1958,7 +1959,6 @@ impl Chat {
context: &Context,
msg: &mut Message,
update_msg_id: Option<MsgId>,
timestamp: i64,
) -> Result<MsgId> {
let mut to_id = 0;
let mut location_id = 0;
Expand Down Expand Up @@ -1990,7 +1990,7 @@ impl Chat {
msg.param.set_int(Param::AttachGroupImage, 1);
self.param
.remove(Param::Unpromoted)
.set_i64(Param::GroupNameTimestamp, timestamp);
.set_i64(Param::GroupNameTimestamp, msg.timestamp_sort);
self.update_param(context).await?;
// TODO: Remove this compat code needed because Core <= v1.143:
// - doesn't accept synchronization of QR code tokens for unpromoted groups, so we also
Expand Down Expand Up @@ -2085,7 +2085,7 @@ impl Chat {
(timestamp,from_id,chat_id, latitude,longitude,independent)\
VALUES (?,?,?, ?,?,1);",
(
timestamp,
msg.timestamp_sort,
ContactId::SELF,
self.id,
msg.param.get_float(Param::SetLatitude).unwrap_or_default(),
Expand Down Expand Up @@ -2141,7 +2141,6 @@ impl Chat {

msg.chat_id = self.id;
msg.from_id = ContactId::SELF;
msg.timestamp_sort = timestamp;

// add message to the database
if let Some(update_msg_id) = update_msg_id {
Expand Down Expand Up @@ -2682,11 +2681,12 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
if msg.viewtype == Viewtype::Text || msg.viewtype == Viewtype::VideochatInvitation {
// the caller should check if the message text is empty
} else if msg.viewtype.has_file() {
let viewtype_orig = msg.viewtype;
let mut blob = msg
.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
Expand All @@ -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())
Expand All @@ -2722,31 +2724,79 @@ 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?;
}

let mut maybe_sticker = msg.viewtype == Viewtype::Sticker;
if !send_as_is
&& (msg.viewtype == Viewtype::Image
|| maybe_sticker && !msg.param.exists(Param::ForceSticker))
if msg.viewtype == Viewtype::File && maybe_image
|| msg.viewtype == Viewtype::Image
|| msg.viewtype == Viewtype::Sticker && !msg.param.exists(Param::ForceSticker)
{
let new_name = blob
.recode_to_image_size(context, msg.get_filename(), &mut maybe_sticker)
.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());

if !maybe_sticker {
msg.viewtype = Viewtype::Image;
}
}

if !msg.param.exists(Param::MimeType) {
if let Some((_, mime)) = message::guess_msgtype_from_suffix(msg) {
if let Some((viewtype, mime)) = message::guess_msgtype_from_suffix(msg) {
// If we unexpectedly didn't recognize the file as image, don't send it as such,
// either the format is unsupported or the image is corrupted.
let mime = match viewtype != Viewtype::Image
|| matches!(msg.viewtype, Viewtype::Image | Viewtype::Sticker)
{
true => mime,
false => "application/octet-stream",
};
msg.param.set(Param::MimeType, mime);
}
}

msg.try_calc_and_set_dimensions(context).await?;

let filename = msg.get_filename().context("msg has no file")?;
let suffix = Path::new(&filename)
.extension()
.and_then(|e| e.to_str())
.unwrap_or("dat");
// Get file name to use for sending. For privacy purposes, we do not transfer the original
// filenames e.g. for images; these names are normally not needed and contain timestamps,
// running numbers, etc.
let filename: String = match viewtype_orig {
Viewtype::Voice => format!(
"voice-messsage_{}.{}",
chrono::Utc
.timestamp_opt(msg.timestamp_sort, 0)
.single()
.map_or_else(
|| "YY-mm-dd_hh:mm:ss".to_string(),
|ts| ts.format("%Y-%m-%d_%H-%M-%S").to_string()
),
&suffix
),
Viewtype::Image | Viewtype::Gif => format!(
"image_{}.{}",
chrono::Utc
.timestamp_opt(msg.timestamp_sort, 0)
.single()
.map_or_else(
|| "YY-mm-dd_hh:mm:ss".to_string(),
|ts| ts.format("%Y-%m-%d_%H-%M-%S").to_string(),
),
&suffix,
),
Viewtype::Video => format!(
"video_{}.{}",
chrono::Utc
.timestamp_opt(msg.timestamp_sort, 0)
.single()
.map_or_else(
|| "YY-mm-dd_hh:mm:ss".to_string(),
|ts| ts.format("%Y-%m-%d_%H-%M-%S").to_string()
),
&suffix
),
_ => filename,
};
msg.param.set(Param::Filename, filename);

info!(
context,
"Attaching \"{}\" for message type #{}.",
Expand Down Expand Up @@ -2899,18 +2949,12 @@ async fn prepare_send_msg(
// ... then change the MessageState in the message object
msg.state = MessageState::OutPending;

msg.timestamp_sort = create_smeared_timestamp(context);
prepare_msg_blob(context, msg).await?;
if !msg.hidden {
chat_id.unarchive_if_not_muted(context, msg.state).await?;
}
msg.id = chat
.prepare_msg_raw(
context,
msg,
update_msg_id,
create_smeared_timestamp(context),
)
.await?;
msg.id = chat.prepare_msg_raw(context, msg, update_msg_id).await?;
msg.chat_id = chat_id;

let row_ids = create_send_msg_jobs(context, msg)
Expand Down Expand Up @@ -4305,9 +4349,8 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId)

msg.state = MessageState::OutPending;
msg.rfc724_mid = create_outgoing_rfc724_mid();
let new_msg_id = chat
.prepare_msg_raw(context, &mut msg, None, curr_timestamp)
.await?;
msg.timestamp_sort = curr_timestamp;
let new_msg_id = chat.prepare_msg_raw(context, &mut msg, None).await?;

curr_timestamp += 1;
if !create_send_msg_jobs(context, &mut msg).await?.is_empty() {
Expand Down Expand Up @@ -4557,19 +4600,17 @@ pub async fn add_device_msg_with_importance(
chat_id = ChatId::get_for_contact(context, ContactId::DEVICE).await?;

let rfc724_mid = create_outgoing_rfc724_mid();
prepare_msg_blob(context, msg).await?;

let timestamp_sent = create_smeared_timestamp(context);

// makes sure, the added message is the last one,
// even if the date is wrong (useful esp. when warning about bad dates)
let mut timestamp_sort = timestamp_sent;
msg.timestamp_sort = timestamp_sent;
if let Some(last_msg_time) = chat_id.get_timestamp(context).await? {
if timestamp_sort <= last_msg_time {
timestamp_sort = last_msg_time + 1;
if msg.timestamp_sort <= last_msg_time {
msg.timestamp_sort = last_msg_time + 1;
}
}

prepare_msg_blob(context, msg).await?;
let state = MessageState::InFresh;
let row_id = context
.sql
Expand All @@ -4591,7 +4632,7 @@ pub async fn add_device_msg_with_importance(
chat_id,
ContactId::DEVICE,
ContactId::SELF,
timestamp_sort,
msg.timestamp_sort,
timestamp_sent,
timestamp_sent, // timestamp_sent equals timestamp_rcvd
msg.viewtype,
Expand Down
Loading
Loading