Fixed XF 2.1.7 - imPecl - Imagick resizing problem

[Rasmus Vind]

Affected version

2.1.7

I have a problem where the Imagick drivers fails to downscale an image. I have a feeling it has something to do with the foreach loop, but I have no idea why. The reason I made the foreach example is because XenForo does the same thing. After calling thumbnailimage the image returns false on isValid() / valid(). Do we need some sort of conditional on when to foreach and when not to? Because the Imagick object supports thumbnailimage without doing a foreach on its frames.

PHP version:

Ramabopro:Hive-XF2 ralle$ docker-compose exec php php -v
PHP 7.0.33-0+deb9u7 (cli) (built: Feb 16 2020 15:11:40) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.0.33-0+deb9u7, Copyright (c) 1999-2017, by Zend Technologies

PHP Imagick version:

root@ef0019441df3:/# php -i | grep imagem
Imagick compiled with ImageMagick version => ImageMagick 6.9.7-4 Q16 x86_64 20170114 http://www.imagemagick.org
Imagick using ImageMagick library version => ImageMagick 6.9.7-4 Q16 x86_64 20170114 http://www.imagemagick.org

Test script:

<?php
$phpVersion = phpversion();
if (version_compare($phpVersion, '5.6.0', '<')) {
    die("PHP 5.6.0 or newer is required. $phpVersion does not meet this requirement. Please ask your host to upgrade PHP.");
}
$dir = __DIR__;
require $dir.'/src/XF.php';
XF::start($dir);
$app = \XF::app();
$file = '/var/www/html/data/medal/619_1582990307l.png';
echo "File: $file\n";
echo 'Exists: ', (file_exists($file) ? 'true' : 'false'), "\n";
echo "\n";

echo "Imagick:direct\n";
$image = new \Imagick($file);
echo 'Valid: ', ($image->valid() ? 'true' : 'false'), "\n";
$image->thumbnailImage(20, 20, false, false);
echo 'Valid: ', ($image->valid() ? 'true' : 'false'), "\n";
echo "\n";

echo "Imagick:foreach\n";
$image = new \Imagick($file);
echo 'Valid: ', ($image->valid() ? 'true' : 'false'), "\n";
foreach ($image as $frame) {
    $frame->thumbnailImage(20, 20, false, true);
}
echo 'Valid: ', ($image->valid() ? 'true' : 'false'), "\n";
echo "\n";

echo "XenForo ImageManager\n";
$image = $app->imageManager()->imageFromFile($file, 'imPecl');
echo 'Valid: ', ($image->isValid() ? 'true' : 'false'), "\n";
$image->resizeTo(20, 20);
echo 'Valid: ', ($image->isValid() ? 'true' : 'false'), "\n";

Test script output:

Ramabopro:Hive-XF2 ralle$ docker-compose exec php php /var/www/html/test.php
File: /var/www/html/data/medal/619_1582990307l.png
Exists: true

Imagick:direct
Valid: true
Valid: true

Imagick:foreach
Valid: true
Valid: false

XenForo ImageManager
Valid: true
Valid: false

An example image that fails on my system.



A snippet from my Dockerfile to reproduce:

FROM debian:stretch
RUN true \
&& apt-get update \
&& DEBIAN_FRONTEND=noninteractive apt-get install -y \
        imagemagick \
        php-fpm \
        php-cli \
        php-memcached \
        php-gd \
        php-imagick \
        php-xml \
        php-zip \
        php-mysql \
        php-curl \
        php-mbstring

This is the snippet of the XenForo code that causes problems for me:

File: XF\Image\Imagick.php

    public function resizeTo($width, $height)
    {
        $scaleUp = ($width > $this->width || $height > $this->height);
        try
        {
            foreach ($this->imagick AS $frame)
            {
                if ($scaleUp)
                {
                    $frame->resizeImage($width, $height, \Imagick::FILTER_QUADRATIC, .5, true);
                }
                else if ($this->isOldImagick())
                {
                    $frame->thumbnailImage($width, $height, false);
                }
                else
                {
                    $frame->thumbnailImage($width, $height, false, true);
                }
                $frame->setImagePage($width, $height, 0, 0);
            }
            $this->updateDimensions();
        }
        catch (\Exception $e) {}
        return $this;
    }

I understand it could be a regression in ImageMagick or something, but I'd dearly want more skilled eyes on this.


EDIT. I just tested with a handful of other PNGs and get the same result.

EDIT. I am working to use the official PHP Docker images now as I can specify exactly which PHP version I want considering PHP 7.0 is actually out of support. I think this could be the fix. I will report back.

 

[Rasmus Vind]

Update.

I am now on PHP 7.2 and XF 2.1.7 and getting the same wrong response.

I am on this Dockerfile:

FROM php:7.2-fpm-buster
RUN true \
    && apt-get update \
    && DEBIAN_FRONTEND=noninteractive apt-get install -y \
    # For the imagick extension
    imagemagick \
    libmagickwand-dev \
    # For the gd extension
    libpng-dev \
    # For running 32 bit binaries
    lib32z1 \
    # For composer fallback
    git \
    # For downloading composer
    curl \
    # For the xml extension
    libxml2-dev \
    # For the curl extension
    libzip-dev \
    libcurl4-gnutls-dev \
    # For the memcached extension
    libmemcached-dev zlib1g-dev \
    # Clean up
    && apt-get clean \
    && apt-get autoclean \
    && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
RUN true \
    # gd extension
    && docker-php-ext-configure gd --with-freetype-dir=/usr/include/ --with-jpeg-dir=/usr/include/ \
    && docker-php-ext-install gd \
    # zip extension
    && docker-php-ext-install zip \
    # mysql extension
    && docker-php-ext-install mysqli \
    # imagick extension
    && pecl install imagick-3.4.4 \
    && docker-php-ext-enable imagick \
    # memcached extension
    && pecl install memcached-3.1.4 \
    && docker-php-ext-enable memcached

And downscaling with imagick is still broken.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.0 RC2).

Change log:

Fix false negative response to XF\Image\Imagick::isValid after an image action has been returned

There may be a delay before changes are rolled out to the XenForo Community.

 

[Mike]

I spent a while debugging this, but I think it's ultimately a false negative because after iterating the image, it looks like the current iterator must be false/invalid internally. The internal call within Imagick::valid then fails.

The fix is really just to put $this->imagick->setFirstIterator(); at the start of our isValid() method to ensure we reset this (which is exactly what the foreach does implicitly when it starts).