autumn icon indicating copy to clipboard operation
autumn copied to clipboard

bug: "thumbnail-ificiation" skipped on decode error

Open GDWR opened this issue 1 year ago • 2 comments

What happened?

Low priority issue tbh, but here we go.

It was noticed when adding an animated .webp for a channel icon, sometimes it would be animated and other times not. Even though the same method of conversion (some ffmpeg commands) from .gif to .webp was used between the different uploads.

I spent some time looking into what was happening and noticed that the assets that would be received as an animated .webp would not be correctly resized by Autumn. For example, look at these as a comparison.

Working as expected:

  • https://autumn.revolt.chat/icons/UwEqmLzBLkMUs1kjb5iR4gFRmM8o9JjFtFtvEK3Mlu
  • https://autumn.revolt.chat/icons/UwEqmLzBLkMUs1kjb5iR4gFRmM8o9JjFtFtvEK3Mlu?max_side=48

No working as expected:

  • https://autumn.revolt.chat/icons/kVXj2K9pTpERDnZlwIsH30WAz5DTb_kB9Qyyeeu99F
  • https://autumn.revolt.chat/icons/kVXj2K9pTpERDnZlwIsH30WAz5DTb_kB9Qyyeeu99F?max_side=48

This is literally caused by a decode error in the try_resize function, Error decoding Err(Decoding(DecodingError { format: Exact(WebP), underlying: Some(ChunkHeaderInvalid([82, 73, 70, 70])) })) and with the current implementation, if this resizing fails it sends the unaltered file.

This is literally the definition of a bug that is a feature :laughing: This is an upstream issue, so might be fixed in the future but this does raise a question on error handling for clients and intended behaviour. I would recommend failing harder (500) which is more expressive to the user that something has gone wrong and would reduce the chances of somebody pulling their hair out as some work and some don't seemingly randomly. I've been playing around with an implementation, but it really is dependent on what behaviour you'd expect in this scenario, 500 or make exceptions for animated content allowing for animated channel icons etc.

Thanks, GDWR - Griff#1126

GDWR avatar Jun 24 '23 12:06 GDWR