cleanImage method bloats up animated GIF

[souvikdg]

Description

We have got a highly optimised animated GIF, but the size is bloating up whenever uploaded through Craft admin panel.

Steps to reproduce

Upload this image file (670kb) through the Craft Asset Panel and the uploaded file will grow to nearly 1mb.

Additional info

• Craft version: 2.6.3012
• PHP version: 7.1.17
• imagick: 3.4.3

[souvikdg]

Would be good to have a configuration like sanitizeSvgUploads that would skip animated GIFs from being cleansed. We had earlier faced an issue with animated GIFs (due to host constraint / old version of imagick) where the images were losing animation. Such a configuration can be handy in trusted environments.

[brandonkelly]

@andris-sevcenko thoughts?

[andris-sevcenko]

@brandonkelly If the setting by default sanitizes GIFs and users have to explicitly turn it off (consciously disabling a security feature), I feel like it's a good idea.

[brandonkelly]

Should it just be GIFs, or maybe a way to disable all image sanitization (at least when uploaded via the Control Panel)?

[andris-sevcenko]

Even though GIF gets hit the hardest, probably should stick to one setting for all sanitization - too granular config settings often get confusing.

[souvikdg]

The two possible aspects are the image formats and upload sources (Control Panel or site front-end). You could provide two different settings for the two aspects, but I think it would be wise to lift the security feature for Control Panel uploads only. As far as image formats go, instead of a blanket disable, one approach could be to accept a list of formats/types/extensions that will be skipped.

[brandonkelly]

Agree with all of that @souvikdg. Maybe we could even roll SVGs into this, and deprecate the sanitizeSvgs config setting.

[andris-sevcenko]

Love this.

[brandonkelly]

@souvikdg One Config Setting to Rule Them All ended up not being as simple of a solution as we thought it would, so we’re going with a transformGifs config setting instead, which will disable all gif transforms – which will have the additional benifit of giving servers without ImageMagick a way of supporting animated gifs.

Already implemented in Craft 3, and we will also port it to Craft 2.

[carlcs]

It seems like there’s quite many people wanting to have this config setting for other image formats as well. See #3287 #3060

As a workaround I override the Images service, but would love to see a config option.

<?php

namespace modules\craft;

class Images extends \craft\services\Images {

    /**
     * @inheritdoc
     */
    public function cleanImage(string $filePath)
    {
        // no cleansing
    }
}

[brandonkelly]

@carlcs Let’s move the discussion over to #3287