Skip to content

Commit

Permalink
fix: Don't remove file extension when recoding avatars
Browse files Browse the repository at this point in the history
There was a bug that file extensions were removed when recoding an
avatar. The problem was that `recode_avatar` used `name` to check for
the extension, but some functions passed an empty string.
There even were two tests from before the decision to keep the
extensions that tested for the faulty behavior.
  • Loading branch information
Hocuri committed Jan 27, 2025
1 parent 60f8b68 commit 28e3fbf
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 29 deletions.
34 changes: 10 additions & 24 deletions src/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ impl<'a> BlobObject<'a> {
// 32 / 4 * 3 = 24k if you account for base64 encoding. To be safe, we reduced this to 20k.
self.recode_to_size(
context,
"".to_string(), // The name of an avatar doesn't matter
None, // The name of an avatar doesn't matter
maybe_sticker,
img_wh,
20_000,
Expand All @@ -459,7 +459,7 @@ impl<'a> BlobObject<'a> {
pub async fn recode_to_image_size(
&mut self,
context: &Context,
name: String,
name: Option<String>,
maybe_sticker: &mut bool,
) -> Result<String> {
let (img_wh, max_bytes) =
Expand Down Expand Up @@ -494,11 +494,10 @@ 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".
/// Pass an empty string if you don't care.
fn recode_to_size(
&mut self,
context: &Context,
mut name: String,
name: Option<String>,
maybe_sticker: &mut bool,
mut img_wh: u32,
max_bytes: usize,
Expand All @@ -508,6 +507,7 @@ impl<'a> BlobObject<'a> {
let mut add_white_bg = img_wh <= constants::BALANCED_AVATAR_SIZE;
let mut no_exif = false;
let no_exif_ref = &mut no_exif;
let mut name = name.unwrap_or_else(|| self.name.clone());
let original_name = name.clone();
let res: Result<String> = tokio::task::block_in_place(move || {
let mut file = std::fs::File::open(self.to_abs_path())?;
Expand Down Expand Up @@ -1103,15 +1103,8 @@ mod tests {
let img_wh = 128;
let maybe_sticker = &mut false;
let strict_limits = true;
blob.recode_to_size(
&t,
"avatar.png".to_string(),
maybe_sticker,
img_wh,
20_000,
strict_limits,
)
.unwrap();
blob.recode_to_size(&t, None, maybe_sticker, img_wh, 20_000, strict_limits)
.unwrap();
tokio::task::block_in_place(move || {
let img = ImageReader::open(blob.to_abs_path())
.unwrap()
Expand Down Expand Up @@ -1142,7 +1135,7 @@ mod tests {
let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
let avatar_path = Path::new(&avatar_blob);
assert!(
avatar_blob.ends_with("d98cd30ed8f2129bf3968420208849d"),
avatar_blob.ends_with("d98cd30ed8f2129bf3968420208849d.jpg"),
"The avatar filename should be its hash, put instead it's {avatar_blob}"
);
let scaled_avatar_size = file_size(avatar_path).await;
Expand All @@ -1158,15 +1151,8 @@ mod tests {
let mut blob = BlobObject::new_from_path(&t, avatar_path).await.unwrap();
let maybe_sticker = &mut false;
let strict_limits = true;
blob.recode_to_size(
&t,
"avatar.jpg".to_string(),
maybe_sticker,
1000,
3000,
strict_limits,
)
.unwrap();
blob.recode_to_size(&t, None, maybe_sticker, 1000, 3000, strict_limits)
.unwrap();
let new_file_size = file_size(&blob.to_abs_path()).await;
assert!(new_file_size <= 3000);
assert!(new_file_size > 2000);
Expand Down Expand Up @@ -1201,7 +1187,7 @@ mod tests {
.unwrap();
let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
assert!(
avatar_cfg.ends_with("9e7f409ac5c92b942cc4f31cee2770a"),
avatar_cfg.ends_with("9e7f409ac5c92b942cc4f31cee2770a.png"),
"Avatar file name {avatar_cfg} should end with its hash"
);

Expand Down
6 changes: 1 addition & 5 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2782,11 +2782,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
|| maybe_sticker && !msg.param.exists(Param::ForceSticker))
{
let new_name = blob
.recode_to_image_size(
context,
msg.get_filename().unwrap_or_else(|| "file".to_string()),
&mut maybe_sticker,
)
.recode_to_image_size(context, msg.get_filename(), &mut maybe_sticker)
.await?;
msg.param.set(Param::Filename, new_name);

Expand Down

0 comments on commit 28e3fbf

Please sign in to comment.