BKWLD / croppa

Image thumbnail creation through specially formatted URLs for Laravel.
MIT License
492 stars 91 forks source link

exif_read_data error with PHP 8 #196

Closed toyi closed 2 years ago

toyi commented 3 years ago

I recently encountered an error while cropping some images with PHP 8.0. I'm not sure why some images seems to be more affected than others, but here is what I found:

Croppa uses weotch/phpthumb.

In this package, in jpg_rotate.inc.php at line 35:

// Exif couldn't do it's thing.  Suppressing errors on it cause it
// does a non-exception error if it can't read the image.
$exif = @exif_read_data($this->parentInstance->getFilename());

The @ operator used here to suppress errors is not working anymore, since it doesn't suppress fatal errors in PHP 8 (https://php.watch/versions/8.0/fatal-error-suppression). As a consequence, an exception is thrown:

exif_read_data(): Argument #1 ($file) must not contain any null bytes at /var/www/agmvision.com/api/vendor/weotch/phpthumb/src/thumb_plugins/jpg_rotate.inc.php:35)

Disabling the exif extension fixed this problem since a check is done earlier in the code to ensure exif_read_data is callable. However, this is obviously not a good fix... or maybe I am missing something ?

I posted here since the issues are disabled on the weotch/phpthumb repo. I don't have the time to make a pr for now, but it should be pretty easy to fix. @weotch you might be interested ! :)

weotch commented 3 years ago

Thanks for the heads up! Do you know if that error is try/catch-able?

toyi commented 3 years ago

It is, a ValueError is thrown and is try/catch-able (more about this error here https://php.watch/versions/8.0/ValueError).

I finally made this PR https://github.com/BKWLD/croppa/pull/197 that mimics the old @ behaviour (on the Croppa side) :)

weotch commented 3 years ago

Closed by #197