Skip to content
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.

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

Craft thinks some images are editable when they are not #2408

Closed
khalwat opened this issue Feb 7, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@khalwat
Copy link
Contributor

commented Feb 7, 2018

Craft CMS 3 RC 9, to determine whether an image can be edited/manipulated, Craft calls canManipulateAsImage() in craft\helpers\Image:

/**
 * Returns whether an image extension is considered manipulatable.
 *
 * @param string $extension
 *
 * @return bool
 */
public static function canManipulateAsImage(string $extension): bool
{
    $formats = Craft::$app->getImages()->getSupportedImageFormats();

    $alwaysManipulatable = ['svg'];
    $neverManipulatable = ['pdf'];

    $formats = array_merge($formats, $alwaysManipulatable);
    $formats = array_diff($formats, $neverManipulatable);

    return in_array(strtolower($extension), $formats);
}

Which in turn calls getSupportedImageFormats() in craft\services\Images:

    /**
     * Returns a list of all supported image formats.
     *
     * @return array
     */
    public function getSupportedImageFormats(): array
    {
        if ($this->getIsImagick()) {
            return array_map('strtolower', Imagick::queryFormats());
        }

The problem happens if Imagick is installed, because Imagick::queryFormats() returns a massive list of "supported" file formats:

http://phpimagick.com/Imagick/queryFormats

This means, for instance, that Craft will think it can edit an .mp4 (even with no crazy plugins installed) when in reality, if you try to open it up in the Image Editor, it understandably can't handle it, and you end up with a blank image editor.

@khalwat

This comment has been minimized.

Copy link
Contributor Author

commented Feb 7, 2018

Looks like the list of files that Imagick returns via ::queryFormats() maybe can't be relied upon:

https://github.com/thoughtbot/paperclip/wiki/Video-thumbnails

Note: ImageMagick does not handle the thumbnail extraction directly. It delegates the extraction to the ffmeg library. If ffmpeg is not available paperclip will raise Validation failed: Asset Paperclip::Errors::NotIdentifiedByImageMagickError.

So apparently the list of formats it's returning isn't reflective of what will actually work, apparently.

@andris-sevcenko

This comment has been minimized.

Copy link
Member

commented Feb 7, 2018

As a counterpoint - if Imagick says it can handle it, then Craft has no reason to not trust it.

Maybe a config setting of file types that can be opened in the image editor could be a solution here, since that seems to be the core issue here?

@brandonkelly

This comment has been minimized.

Copy link
Member

commented Feb 7, 2018

@andris-sevcenko Is there a quick & low memory way we can attempt to generate an image with the given extension from canManipulateAsImage(), to ensure that the format is actually safe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.