craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.22k stars 627 forks source link

services/Images.php bug in checkMemoryForImage() #6898

Closed khalwat closed 3 years ago

khalwat commented 3 years ago

Description

There's a bug in services/Images.php in the checkMemoryForImage() method:

https://github.com/craftcms/cms/blob/develop/src/services/Images.php#L219

which does:

        // Find out how much memory this image is going to need.
        $imageInfo = getimagesize($filePath);

        $K64 = 65536;
        $tweakFactor = 1.7;
        $bits = $imageInfo['bits'] ?? 8;
        $channels = $imageInfo['channels'] ?? 4;
        $memoryNeeded = round(($imageInfo[0] * $imageInfo[1] * $bits * $channels / 8 + $K64) * $tweakFactor);

The problem here is that getimagesize() can return false if there's some kind of a failure reading in the image (or if the image doesn't exist):

https://www.php.net/manual/en/function.getimagesize.php#refsect1-function.getimagesize-returnvalues

It'll throw an error like this:

2020-09-24 10:41:18 [-][-][7ro05v3itrg23bh40jp8cobrn2][error][yii\base\ErrorException:8] yii\base\ErrorException: Trying to access array offset on value of type bool in 

I'm not sure what behavior you'd like checkMemoryForImage() to have in instances where the passed in image can't be read, but probably it should check for a falsey return value from getimagesize() and not try to access the $imageInfo[] in that case.

Probably I'd expect it to log an error and return false, or it could throw an explicit exception in this case, but up to you. ¯_(ツ)_/¯

andris-sevcenko commented 3 years ago

Curios, but shouldn't this code already return if file didn't exist?

khalwat commented 3 years ago

One would think @andris-sevcenko -- but perhaps the error is a corrupt file or some other issue reading the file (permissions) but it's still able to access the size of the file with filesize ?

sjcallender commented 3 years ago

We're seeing this on a site. Here's the trace

yii\base\ErrorException: Trying to access array offset on value of type bool in /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/craftcms/cms/src/services/Images.php:246
Stack trace:
#0 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/craftcms/cms/src/services/Images.php(246): yii\base\ErrorHandler->handleError(8, 'Trying to acces...', '/var/www/5cb313...', 246, Array)
#1 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/craftcms/cms/src/image/Raster.php(153): craft\services\Images->checkMemoryForImage('/var/www/5cb313...')
#2 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/craftcms/cms/src/services/Images.php(201): craft\image\Raster->loadImage('/var/www/5cb313...')
#3 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/services/Placeholder.php(261): craft\services\Images->loadImage('/var/www/5cb313...')
#4 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/services/Placeholder.php(233): nystudio107\imageoptimize\services\Placeholder->createImageFromPath('/var/www/5cb313...', 300, 93, 75, Array)
#5 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/services/Placeholder.php(211): nystudio107\imageoptimize\services\Placeholder->createImageFromAsset(Object(craft\elements\Asset), 300, 93, 75, Array)
#6 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(433): nystudio107\imageoptimize\services\Placeholder->createTempPlaceholderImage(Object(craft\elements\Asset), 3.2, Array)
#7 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(546): nystudio107\imageoptimize\services\OptimizedImages->generatePlaceholders(Object(craft\elements\Asset), Object(nystudio107\imageoptimize\models\OptimizedImage), 3.2)
#8 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(130): nystudio107\imageoptimize\services\OptimizedImages->addVariantImageToModel(Object(craft\elements\Asset), Object(nystudio107\imageoptimize\models\OptimizedImage), Object(craft\models\AssetTransform), Array, 3.2)
#9 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/services/OptimizedImages.php(248): nystudio107\imageoptimize\services\OptimizedImages->populateOptimizedImageModel(Object(craft\elements\Asset), Array, Object(nystudio107\imageoptimize\models\OptimizedImage), false)
#10 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/nystudio107/craft-imageoptimize/src/jobs/ResaveOptimizedImages.php(98): nystudio107\imageoptimize\services\OptimizedImages->updateOptimizedImageFieldData(Object(nystudio107\imageoptimize\fields\OptimizedImages), Object(craft\elements\Asset), false)
#11 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2-queue/src/Queue.php(246): nystudio107\imageoptimize\jobs\ResaveOptimizedImages->execute(Object(craft\queue\Queue))
#12 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2-queue/src/cli/Queue.php(162): yii\queue\Queue->handleMessage('1779366', 'O:52:"nystudio1...', '1800', '1')
#13 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2-queue/src/cli/Command.php(146): yii\queue\cli\Queue->execute('1779366', 'O:52:"nystudio1...', '1800', '1', NULL)
#14 [internal function]: yii\queue\cli\Command->actionExec('1779366', '1800', '1', '0')
#15 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#16 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2/base/Controller.php(180): yii\base\InlineAction->runWithParams(Array)
#17 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2/console/Controller.php(179): yii\base\Controller->runAction('exec', Array)
#18 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2/base/Module.php(528): yii\console\Controller->runAction('exec', Array)
#19 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2/console/Application.php(180): yii\base\Module->runAction('queue/exec', Array)
#20 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/craftcms/cms/src/console/Application.php(88): yii\console\Application->runAction('queue/exec', Array)
#21 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2/console/Application.php(147): craft\console\Application->runAction('queue/exec', Array)
#22 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/vendor/yiisoft/yii2/base/Application.php(386): yii\console\Application->handleRequest(Object(craft\console\Request))
#23 /var/www/5cb313bb3eb1bc71e949ffb570181ad6d5f57a40/craft(22): yii\base\Application->run()
#24 {main}
andris-sevcenko commented 3 years ago

Thanks! Just fixed this for the next release!

brandonkelly commented 3 years ago

Craft 3.5.16 is out now with that fix.