Skip to content

Commit

Permalink
ext/gd: imagewebp/imageavif/imagepng/imagejpeg stricter checks qualit…
Browse files Browse the repository at this point in the history
…y/speed.

close GH-14485
  • Loading branch information
devnexen committed Jun 5, 2024
1 parent 924e7fc commit 7b2ca07
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 8 deletions.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ PHP NEWS
- GD:
. Fix parameter numbers and missing alpha check for imagecolorset().
(Giovanni Giacobbi)
. imagepng/imagejpeg/imagewep/imageavif now throw an exception on
invalid quality parameter. (David Carlier)

- Gettext:
. bind_textdomain_codeset, textdomain and d(*)gettext functions
Expand Down
5 changes: 5 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,11 @@ PHP 8.4 UPGRADE NOTES
. DOMDocument::registerNodeClass() now has a tentative return type of true.
Previously, the return type was bool but only true could be returned in practice.

- GD:
. imagejpeg/imagewebp/imagepng/imageavif throws an exception if an invalid
quality parameter value is passed. In addition, imageavif will throw an exception
if an invalid speed parameter value is passed.

- Gettext:
. bind_textdomain_codeset, textdomain and d(*)gettext functions now throw an exception
if the domain argument is empty.
Expand Down
27 changes: 24 additions & 3 deletions ext/gd/gd.c
Original file line number Diff line number Diff line change
Expand Up @@ -4108,27 +4108,48 @@ static void _php_image_output_ctx(INTERNAL_FUNCTION_PARAMETERS, int image_type,
switch (image_type) {
#ifdef HAVE_GD_JPG
case PHP_GDIMG_TYPE_JPG:
if (quality < -1 || quality > 100) {
zend_argument_value_error(3, "must be at between -1 and 100");
ctx->gd_free(ctx);
RETURN_THROWS();
}
gdImageJpegCtx(im, ctx, (int) quality);
break;
#endif
#ifdef HAVE_GD_WEBP
case PHP_GDIMG_TYPE_WEBP:
if (quality == -1) {
quality = 80;
if (quality < -1) {
zend_argument_value_error(3, "must be greater than or equal to -1");
ctx->gd_free(ctx);
RETURN_THROWS();
}
gdImageWebpCtx(im, ctx, (int) quality);
break;
#endif
#ifdef HAVE_GD_AVIF
case PHP_GDIMG_TYPE_AVIF:
if (speed == -1) {
if (quality < -1 || quality > 100) {
zend_argument_value_error(3, "must be between -1 and 100");
ctx->gd_free(ctx);
RETURN_THROWS();
}
if (speed < -1 || speed > 10) {
zend_argument_value_error(4, "must be between -1 and 10");
ctx->gd_free(ctx);
RETURN_THROWS();
} else if (speed == -1) {
speed = 6;
}
gdImageAvifCtx(im, ctx, (int) quality, (int) speed);
break;
#endif
#ifdef HAVE_GD_PNG
case PHP_GDIMG_TYPE_PNG:
if (quality < -1 || quality > 9) {
zend_argument_value_error(3, "must be between -1 and 9");
ctx->gd_free(ctx);
RETURN_THROWS();
}
#ifdef HAVE_GD_BUNDLED
gdImagePngCtxEx(im, ctx, (int) quality, (int) basefilter);
#else
Expand Down
17 changes: 13 additions & 4 deletions ext/gd/tests/avif_decode_encode.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,19 @@ gd
echo_status(imageavif($img, $outfile, -1));

echo 'Encoding AVIF with illegal quality: ';
echo_status(imageavif($img, $outfile, 1234));
try {
imageavif($img, $outfile, 1234);
} catch (\ValueError $e) {
echo $e->getMessage() . PHP_EOL;
}

echo 'Encoding AVIF with illegal speed: ';
echo_status(imageavif($img, $outfile, 70, 1234));

try {
imageavif($img, $outfile, 70, 1234);
} catch (\ValueError $e) {
echo $e->getMessage() . PHP_EOL;
}

echo 'Encoding AVIF losslessly... ';
echo_status(imageavif($img, $outfile, 100, 0));
Expand All @@ -66,8 +75,8 @@ Default AVIF encoding: ok
Encoding AVIF at quality 70: ok
Encoding AVIF at quality 70 with speed 5: ok
Encoding AVIF with default quality: ok
Encoding AVIF with illegal quality: ok
Encoding AVIF with illegal speed: ok
Encoding AVIF with illegal quality: imageavif(): Argument #3 ($quality) must be between -1 and 100
Encoding AVIF with illegal speed: imageavif(): Argument #4 ($speed) must be between -1 and 10
Encoding AVIF losslessly... ok
Decoding the AVIF we just wrote...
How many pixels are different in the two images? 0
8 changes: 8 additions & 0 deletions ext/gd/tests/imageresolution_jpeg.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ imageresolution($exp, 71, 299);
imagejpeg($exp, $filename);
$act = imagecreatefromjpeg($filename);
var_dump(imageresolution($act));
imageresolution($exp, 71, 299);

try {
imagejpeg($exp, $filename, 101);
} catch (\ValueError $e) {
echo $e->getMessage();
}
?>
--EXPECT--
array(2) {
Expand All @@ -36,6 +43,7 @@ array(2) {
[1]=>
int(299)
}
imagejpeg(): Argument #3 ($quality) must be at between -1 and 100
--CLEAN--
<?php
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'imageresolution_jpeg.jpeg');
Expand Down
14 changes: 13 additions & 1 deletion ext/gd/tests/pngcomp.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,20 @@ gd
<?php
$cwd = __DIR__;

echo "PNG compression test: ";

$im = imagecreatetruecolor(20,20);
imagefilledrectangle($im, 5,5, 10,10, 0xffffff);
try {
imagepng($im, $cwd . '/test_pngcomp.png', -2);
} catch (\ValueError $e) {
echo $e->getMessage() . PHP_EOL;
}
try {
imagepng($im, $cwd . '/test_pngcomp.png', 10);
} catch (\ValueError $e) {
echo $e->getMessage() . PHP_EOL;
}
echo "PNG compression test: ";
imagepng($im, $cwd . '/test_pngcomp.png', 9);

$im2 = imagecreatefrompng($cwd . '/test_pngcomp.png');
Expand All @@ -27,4 +37,6 @@ gd
@unlink($cwd . "/test_pngcomp.png");
?>
--EXPECT--
imagepng(): Argument #3 ($quality) must be between -1 and 9
imagepng(): Argument #3 ($quality) must be between -1 and 9
PNG compression test: ok
7 changes: 7 additions & 0 deletions ext/gd/tests/webp_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ $im_lossless = imagecreatefromwebp($filename);
echo 'Does lossless conversion work? ';
var_dump(calc_image_dissimilarity($im1, $im_lossless) == 0);

try {
imagewebp($im1, $filename, -10);
} catch (\ValueError $e) {
echo $e->getMessage();
}

?>
--CLEAN--
<?php
Expand All @@ -45,3 +51,4 @@ var_dump(calc_image_dissimilarity($im1, $im_lossless) == 0);
--EXPECT--
Is lossy conversion close enough? bool(true)
Does lossless conversion work? bool(true)
imagewebp(): Argument #3 ($quality) must be greater than or equal to -1

0 comments on commit 7b2ca07

Please sign in to comment.