Restore Warning instead of Fatal Error in gd_webp.c

[dryabov]

According to the docs (see https://www.php.net/manual/en/function.imagecreatefromwebp.php and https://www.php.net/manual/en/function.imagewebp.php), false should be returned on errors (similar to other functions of the gd extension), but actually all errors result in a Fatal Error. It doesn't look normal when trying to read an empty file or a file in the wrong format causes the program to stop.

The problem seems to be related to a mega-patch that replaced zend_error with zend_error_noreturn almost everywhere.

My patch fixes this behavior by switching from zend_error_noerror to gd_error (i.e. to E_WARNING level). All necessary memory cleanup is already in the code (as it was before the "zend_error_noreturn" patch).

You can use the following code to test:

<?php
$empty_webp = __DIR__ . '/empty.webp';
file_put_contents($empty_webp, '');
$im = imagecreatefromwebp($empty_webp);

Current result:

Fatal error: gd-webp cannot get webp info in ...

Expected result:

Warning: imagecreatefromwebp(): gd-webp cannot get webp info in ...

[cmb69]

Ugh, that is pretty strange, since zend_error(E_ERROR, …) is there since that file has been committed, but apparently nobody had reported this issue before. I shall have a closer look.

[cmb69]

The problem seems to be related to a mega-patch that replaced zend_error with zend_error_noreturn almost everywhere.

No, that is not the issue. You will get the same behavior prior to that commit. The problem is indeed that I had replaced gd_error() calls with zend_error() in the original commit, and I have absolutely no idea why I had done that.

The patch looks fine, but we should probably target PHP-8.2 or maybe only PHP-8.3 (as a compromise), since this is clearly unintended behavior, and is different from what you get with a system libgd. And there should be a test to check the behavior (basically the test code in the OP).

@devnexen, what do think?

@dryabov, would you take care of this?

[devnexen]

Absolutely no objection on my part, however release managers might have a say too.

[cmb69]

Absolutely no objection on my part, however release managers might have a say too.

Ah, right! So asking the PHP 8.2 release managers if they have concerns targeting PHP-8.2 for this bug fix (which is basically emit a warning instead of a fatal error when reading unreadable WebP files): @ramsey, @saundefined, @adoy.

[adoy]

Lgtm. I agree that an error in an image file shouldn't trigger a fatal.

[devnexen]

Thank you !

[Girgias]

I've commented on the commit, but I do think the allocation failures using E_ERROR is actually correct. The rest are fine as warnings.