Skip to content

fix: Correct condition for re-encoding avatar-images that include exif-data #7772

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

Closed
72374 wants to merge 2 commits into from
Closed

fix: Correct condition for re-encoding avatar-images that include exif-data #7772

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f43b3b9
fix: Correct condition for re-encoding avatar-images that include exi…
72374 Jan 24, 2026
edea31e
chore: Add a few comments to `check_or_recode_to_size`
72374 Jan 25, 2026
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
25 changes: 12 additions & 13 deletions src/blob.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,19 +409,18 @@ impl<'a> BlobObject<'a> {
// images.
let do_scale = exceeds_max_bytes
|| is_avatar
&& (exceeds_wh
|| exif.is_some() && {
if mem::take(&mut add_white_bg) {
self::add_white_bg(&mut img);
}
encoded_img_exceeds_bytes(
Copy link
Collaborator

@iequidoo iequidoo Jan 25, 2026

Choose a reason for hiding this comment

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

encoded_img_exceeds_bytes() already recodes the image, that's why it takes &mut encoded, this encoded is then passed to BlobObject::create_and_deduplicate_from_bytes(). Maybe rename it to just encode_img_ex() to avoid any wrong assumptions about what it does? Or, if i'm wrong, adding a test for the scenario being fixed will help, w/o tests we always get regressions.

Copy link
Contributor Author

@72374 72374 Jan 25, 2026

Choose a reason for hiding this comment

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

The function does add a file-size-check to encode_img, that returns false or true.

core/src/blob.rs

Lines 653 to 673 in f43b3b9

fn encoded_img_exceeds_bytes(
context: &Context,
img: &DynamicImage,
fmt: ImageOutputFormat,
max_bytes: usize,
encoded: &mut Vec<u8>,
) -> anyhow::Result<bool> {
encode_img(img, fmt, 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)
}

So, i think the name makes sense as it is.

I added a comment that does explain that it actually changes the image (in the resizing-loop, where it is primarily used) to make that easier to notice, and added another comment explaining why the image will be encoded here.

context,
&img,
ofmt.clone(),
max_bytes,
&mut encoded,
)?
});
&& (exceeds_wh || exif.is_some() || {
if mem::take(&mut add_white_bg) {
self::add_white_bg(&mut img);
}
encoded_img_exceeds_bytes(
context,
&img,
ofmt.clone(),
max_bytes,
&mut encoded,
)?
});

if do_scale {
loop {
Expand Down
Loading