From d9d5827697441130bc766e7a64af149545bc85a2 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sat, 2 Nov 2019 16:40:17 +0100 Subject: [PATCH 1/3] Deprecate the $num_points parameter of php_imagepolygon That parameter is mostly useless in practise, and likely has been directly ported from the underlying `gdImagePolygon()` and friends, which require that parameter since the number of elements of the point array would otherwise be unknown. Typical usages of `imagepolygon()`, `imageopenpolygon()` and `imagefilledpolygon()` pass `count($points)/2` or hard-code this value as literal. Since explicitly specifying this parameter is annoying and error-prone, we offer the possibility to omit it, in which case the `$points` array must have an even number of elements, and the number of points is calculated as `count($points)/2`. We deprecate explicitly passing the `$num_points` parameter, but keep the sloppy behavior of the functions in this case (for instance, having an odd number of elements in `$points`). --- ext/gd/gd.c | 14 ++++++++++++-- ext/gd/gd.stub.php | 6 +++--- ext/gd/gd_arginfo.h | 4 ++-- ext/gd/tests/bug43073.phpt | 2 +- ext/gd/tests/bug53504.phpt | 2 +- ext/gd/tests/bug55005.phpt | 7 +++++-- ext/gd/tests/bug64641.phpt | 4 ++-- ext/gd/tests/imagefilledpolygon_basic.phpt | 2 +- ext/gd/tests/imageopenpolygon_basic.phpt | 8 ++++---- ext/gd/tests/imagepolygon_aa.phpt | 2 +- ext/gd/tests/imagepolygon_basic.phpt | 1 - ext/gd/tests/libgd00100.phpt | 16 ++++++++-------- 12 files changed, 40 insertions(+), 28 deletions(-) diff --git a/ext/gd/gd.c b/ext/gd/gd.c index cc7472742040c..52b46c948fae1 100644 --- a/ext/gd/gd.c +++ b/ext/gd/gd.c @@ -2735,8 +2735,18 @@ static void php_imagepolygon(INTERNAL_FUNCTION_PARAMETERS, int filled) gdPointPtr points; int npoints, col, nelem, i; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oall", &IM, gd_image_ce, &POINTS, &NPOINTS, &COL) == FAILURE) { - return; + if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "Oall", &IM, gd_image_ce, &POINTS, &NPOINTS, &COL) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oal", &IM, gd_image_ce, &POINTS, &COL) == FAILURE) { + return; + } + NPOINTS = zend_hash_num_elements(Z_ARRVAL_P(POINTS)); + if (NPOINTS % 2 != 0) { + zend_value_error("Points array must have an even number of elements"); + return; + } + NPOINTS /= 2; + } else { + php_error_docref(NULL, E_DEPRECATED, "the $num_points parameter is deprecated"); } im = php_gd_libgdimageptr_from_zval_p(IM); diff --git a/ext/gd/gd.stub.php b/ext/gd/gd.stub.php index 7b639f9371037..2f5be5339d89c 100644 --- a/ext/gd/gd.stub.php +++ b/ext/gd/gd.stub.php @@ -199,11 +199,11 @@ function imagecolortransparent(GdImage $im, int $col = UNKNOWN): ?int {} function imageinterlace(GdImage $im, int $interlace = UNKNOWN): ?int {} -function imagepolygon(GdImage $im, array $points, int $num_pos, int $col): bool {} +function imagepolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} -function imageopenpolygon(GdImage $im, array $points, int $num_pos, int $col): bool {} +function imageopenpolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} -function imagefilledpolygon(GdImage $im, array $points, int $num_pos, int $col): bool {} +function imagefilledpolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} function imagefontwidth(int $font): int {} diff --git a/ext/gd/gd_arginfo.h b/ext/gd/gd_arginfo.h index 1f036c8ea96b5..8e14930ba0f53 100644 --- a/ext/gd/gd_arginfo.h +++ b/ext/gd/gd_arginfo.h @@ -365,10 +365,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_imageinterlace, 0, 1, IS_LONG, 1 ZEND_ARG_TYPE_INFO(0, interlace, IS_LONG, 0) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_imagepolygon, 0, 4, _IS_BOOL, 0) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_imagepolygon, 0, 3, _IS_BOOL, 0) ZEND_ARG_OBJ_INFO(0, im, GdImage, 0) ZEND_ARG_TYPE_INFO(0, points, IS_ARRAY, 0) - ZEND_ARG_TYPE_INFO(0, num_pos, IS_LONG, 0) + ZEND_ARG_TYPE_INFO(0, num_points, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, col, IS_LONG, 0) ZEND_END_ARG_INFO() diff --git a/ext/gd/tests/bug43073.phpt b/ext/gd/tests/bug43073.phpt index a8027338ef36b..c52bdad5bedf5 100644 --- a/ext/gd/tests/bug43073.phpt +++ b/ext/gd/tests/bug43073.phpt @@ -38,7 +38,7 @@ $cos_t = cos(deg2rad($delta_t)); $sin_t = sin(deg2rad($delta_t)); for ($angle = 0.0, $i = 0; $angle < 360.0; $angle += $delta_t, $i++) { $bbox = imagettftext($g, 24, $angle, 400+$x, 400+$y, $black, $font, 'ABCDEF'); - imagepolygon($g, $bbox, 4, $red); + imagepolygon($g, $bbox, $red); printf("%2d: ", $i); for ($j = 0; $j < 8; $j++) { if ($bbox[$j] >= $exp[$i][$j] - 1 && $bbox[$j] <= $exp[$i][$j] + 1) { diff --git a/ext/gd/tests/bug53504.phpt b/ext/gd/tests/bug53504.phpt index 3b2ce4c7eb3f0..c44e4ef0f81b2 100644 --- a/ext/gd/tests/bug53504.phpt +++ b/ext/gd/tests/bug53504.phpt @@ -72,7 +72,7 @@ foreach ($tests as $testnum => $test) { } // draw bounding box: - imagepolygon($g, $bboxDrawn, 4, $red); + imagepolygon($g, $bboxDrawn, $red); // draw baseline: $width = sqrt(pow($bboxDrawn[2] - $bboxDrawn[0], 2) + pow($bboxDrawn[3] - $bboxDrawn[1], 2)); diff --git a/ext/gd/tests/bug55005.phpt b/ext/gd/tests/bug55005.phpt index a48d92441c21b..793774729afcf 100644 --- a/ext/gd/tests/bug55005.phpt +++ b/ext/gd/tests/bug55005.phpt @@ -16,6 +16,9 @@ trycatch_dump( fn () => imagepolygon($g, array(200,10, 200,100, 280,100), 2, $fgnd) ); ?> ---EXPECT-- -!! [ValueError] Polygon must have at least 3 points +--EXPECTF-- +Deprecated: imagefilledpolygon(): the $num_points parameter is deprecated in %s on line %d !! [ValueError] Polygon must have at least 3 points + +Deprecated: imagepolygon(): the $num_points parameter is deprecated in %s on line %d +!! [ValueError] Polygon must have at least 3 points \ No newline at end of file diff --git a/ext/gd/tests/bug64641.phpt b/ext/gd/tests/bug64641.phpt index 713daaf85e361..2d5768eece80a 100644 --- a/ext/gd/tests/bug64641.phpt +++ b/ext/gd/tests/bug64641.phpt @@ -18,14 +18,14 @@ $points = array( 100, 200, 100, 300 ); -imagefilledpolygon($im, $points, 3, 0xFFFF00); +imagefilledpolygon($im, $points, 0xFFFF00); $points = array( 300, 200, 400, 200, 500, 200 ); -imagefilledpolygon($im, $points, 3, 0xFFFF00); +imagefilledpolygon($im, $points, 0xFFFF00); $ex = imagecreatefrompng(__DIR__ . '/bug64641.png'); if (($diss = calc_image_dissimilarity($ex, $im)) < 1e-5) { diff --git a/ext/gd/tests/imagefilledpolygon_basic.phpt b/ext/gd/tests/imagefilledpolygon_basic.phpt index 0f5f9cf335cd7..0bede9e6f991b 100644 --- a/ext/gd/tests/imagefilledpolygon_basic.phpt +++ b/ext/gd/tests/imagefilledpolygon_basic.phpt @@ -36,7 +36,7 @@ $bg = imagecolorallocate($image, 0, 0, 0); $col_poly = imagecolorallocate($image, 0, 255, 0); // draw the polygon -imagefilledpolygon($image, $points, count($points)/2, $col_poly); +imagefilledpolygon($image, $points, $col_poly); // output the picture to a file imagepng($image, $dest); diff --git a/ext/gd/tests/imageopenpolygon_basic.phpt b/ext/gd/tests/imageopenpolygon_basic.phpt index 0109cf015eca0..f21125519c035 100644 --- a/ext/gd/tests/imageopenpolygon_basic.phpt +++ b/ext/gd/tests/imageopenpolygon_basic.phpt @@ -16,10 +16,10 @@ $green = imagecolorallocate($im, 0, 128, 0); $blue = imagecolorallocate($im, 0, 0, 255); imagefilledrectangle($im, 0,0, 99,99, $white); -imageopenpolygon($im, [10,10, 49,89, 89,10], 3, $black); -imageopenpolygon($im, [10,89, 35,10, 60,89, 85,10], 4, $red); -imageopenpolygon($im, [10,49, 30,89, 50,10, 70,89, 90,10], 5, $green); -imageopenpolygon($im, [10,50, 25,10, 40,89, 55,10, 80,89, 90,50], 6, $blue); +imageopenpolygon($im, [10,10, 49,89, 89,10], $black); +imageopenpolygon($im, [10,89, 35,10, 60,89, 85,10], $red); +imageopenpolygon($im, [10,49, 30,89, 50,10, 70,89, 90,10], $green); +imageopenpolygon($im, [10,50, 25,10, 40,89, 55,10, 80,89, 90,50], $blue); test_image_equals_file(__DIR__ . DIRECTORY_SEPARATOR . 'imageopenpolygon.png', $im); ?> diff --git a/ext/gd/tests/imagepolygon_aa.phpt b/ext/gd/tests/imagepolygon_aa.phpt index 9725bdabe8042..a92fbfdb78e4d 100644 --- a/ext/gd/tests/imagepolygon_aa.phpt +++ b/ext/gd/tests/imagepolygon_aa.phpt @@ -14,7 +14,7 @@ $black = imagecolorallocate($im, 0, 0, 0); imagefilledrectangle($im, 0,0, 99,99, $white); imageantialias($im, true); -imagepolygon($im, [10,10, 49,89, 89,49], 3, $black); +imagepolygon($im, [10,10, 49,89, 89,49], $black); test_image_equals_file(__DIR__ . DIRECTORY_SEPARATOR . 'imagepolygon_aa.png', $im); ?> diff --git a/ext/gd/tests/imagepolygon_basic.phpt b/ext/gd/tests/imagepolygon_basic.phpt index 5dfecfbf19b13..4880f3ebe7b38 100644 --- a/ext/gd/tests/imagepolygon_basic.phpt +++ b/ext/gd/tests/imagepolygon_basic.phpt @@ -33,7 +33,6 @@ imagepolygon($image, array ( 100, 200, 300, 200 ), - 3, $col_poly); // output the picture to a file diff --git a/ext/gd/tests/libgd00100.phpt b/ext/gd/tests/libgd00100.phpt index a0ad74a5f53f1..c6fa6dd30e5cc 100644 --- a/ext/gd/tests/libgd00100.phpt +++ b/ext/gd/tests/libgd00100.phpt @@ -33,7 +33,7 @@ $points = array( $x+$d, ($top+$bot)/2, $x, $bot ); -imagefilledpolygon($im, $points, 5, $yellow); +imagefilledpolygon($im, $points, $yellow); // left-facing M not on baseline $top = 40; @@ -47,7 +47,7 @@ $points = array( $left, $bot, ($left+$right)/2, ($top+$bot)/2 ); -imagefilledpolygon($im, $points, 5, $purple); +imagefilledpolygon($im, $points, $purple); // left-facing M on baseline $top = 240; @@ -61,7 +61,7 @@ $points = array( $left, $bot, ($left+$right)/2, ($top+$bot)/2 ); -imagefilledpolygon($im, $points, 5, $magenta); +imagefilledpolygon($im, $points, $magenta); // left-facing M on ceiling $top = -15; @@ -75,23 +75,23 @@ $points = array( $left, $bot, ($left+$right)/2, ($top+$bot)/2 ); -imagefilledpolygon($im, $points, 5, $blue); +imagefilledpolygon($im, $points, $blue); $d = 30; $x = 150; $y = 150; $diamond = array($x-$d, $y, $x, $y+$d, $x+$d, $y, $x, $y-$d); -imagefilledpolygon($im, $diamond, 4, $green); +imagefilledpolygon($im, $diamond, $green); $x = 180; $y = 225; $diamond = array($x-$d, $y, $x, $y+$d, $x+$d, $y, $x, $y-$d); -imagefilledpolygon($im, $diamond, 4, $red); +imagefilledpolygon($im, $diamond, $red); $x = 225; $y = 255; $diamond = array($x-$d, $y, $x, $y+$d, $x+$d, $y, $x, $y-$d); -imagefilledpolygon($im, $diamond, 4, $cyan); +imagefilledpolygon($im, $diamond, $cyan); // M (bridge) not touching bottom boundary $top = 100; @@ -104,7 +104,7 @@ $points = array( $x+$d, ($top+$bot)/2, $x, $bot ); -imagefilledpolygon($im, $points, 5, $black); +imagefilledpolygon($im, $points, $black); include_once __DIR__ . '/func.inc'; test_image_equals_file(__DIR__ . '/libgd00100.png', $im); From 968cf8e1d7161d49e9515305b70338692c698f19 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 4 Nov 2019 09:18:23 +0100 Subject: [PATCH 2/3] Refactor --- ext/gd/gd.c | 9 +++++---- ext/gd/gd.stub.php | 6 +++--- ext/gd/gd_arginfo.h | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ext/gd/gd.c b/ext/gd/gd.c index 52b46c948fae1..f9d910de6f4e5 100644 --- a/ext/gd/gd.c +++ b/ext/gd/gd.c @@ -2735,10 +2735,11 @@ static void php_imagepolygon(INTERNAL_FUNCTION_PARAMETERS, int filled) gdPointPtr points; int npoints, col, nelem, i; - if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "Oall", &IM, gd_image_ce, &POINTS, &NPOINTS, &COL) == FAILURE) { - if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oal", &IM, gd_image_ce, &POINTS, &COL) == FAILURE) { - return; - } + if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oal|l", &IM, gd_image_ce, &POINTS, &NPOINTS, &COL) == FAILURE) { + return; + } + if (ZEND_NUM_ARGS() == 3) { + COL = NPOINTS; NPOINTS = zend_hash_num_elements(Z_ARRVAL_P(POINTS)); if (NPOINTS % 2 != 0) { zend_value_error("Points array must have an even number of elements"); diff --git a/ext/gd/gd.stub.php b/ext/gd/gd.stub.php index 2f5be5339d89c..ef7997ebff4ca 100644 --- a/ext/gd/gd.stub.php +++ b/ext/gd/gd.stub.php @@ -199,11 +199,11 @@ function imagecolortransparent(GdImage $im, int $col = UNKNOWN): ?int {} function imageinterlace(GdImage $im, int $interlace = UNKNOWN): ?int {} -function imagepolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} +function imagepolygon(GdImage $im, array $points, int $num_points_or_col, int $col = UNKNOWN): bool {} -function imageopenpolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} +function imageopenpolygon(GdImage $im, array $points, int $num_points_or_col, int $col = UNKNOWN): bool {} -function imagefilledpolygon(GdImage $im, array $points, int $num_points, int $col = UNKNOWN): bool {} +function imagefilledpolygon(GdImage $im, array $points, int $num_points_or_col, int $col = UNKNOWN): bool {} function imagefontwidth(int $font): int {} diff --git a/ext/gd/gd_arginfo.h b/ext/gd/gd_arginfo.h index 8e14930ba0f53..1f764c59dfe50 100644 --- a/ext/gd/gd_arginfo.h +++ b/ext/gd/gd_arginfo.h @@ -368,7 +368,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_imagepolygon, 0, 3, _IS_BOOL, 0) ZEND_ARG_OBJ_INFO(0, im, GdImage, 0) ZEND_ARG_TYPE_INFO(0, points, IS_ARRAY, 0) - ZEND_ARG_TYPE_INFO(0, num_points, IS_LONG, 0) + ZEND_ARG_TYPE_INFO(0, num_points_or_col, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, col, IS_LONG, 0) ZEND_END_ARG_INFO() From e922ebce0df2c05a078a017e7112985cbf219d82 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 14 Nov 2019 09:46:43 +0100 Subject: [PATCH 3/3] Postpone deprecation --- ext/gd/gd.c | 2 -- ext/gd/tests/bug55005.phpt | 5 +---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/ext/gd/gd.c b/ext/gd/gd.c index f9d910de6f4e5..543e08466c0ac 100644 --- a/ext/gd/gd.c +++ b/ext/gd/gd.c @@ -2746,8 +2746,6 @@ static void php_imagepolygon(INTERNAL_FUNCTION_PARAMETERS, int filled) return; } NPOINTS /= 2; - } else { - php_error_docref(NULL, E_DEPRECATED, "the $num_points parameter is deprecated"); } im = php_gd_libgdimageptr_from_zval_p(IM); diff --git a/ext/gd/tests/bug55005.phpt b/ext/gd/tests/bug55005.phpt index 793774729afcf..38fb25db07787 100644 --- a/ext/gd/tests/bug55005.phpt +++ b/ext/gd/tests/bug55005.phpt @@ -17,8 +17,5 @@ trycatch_dump( ); ?> --EXPECTF-- -Deprecated: imagefilledpolygon(): the $num_points parameter is deprecated in %s on line %d !! [ValueError] Polygon must have at least 3 points - -Deprecated: imagepolygon(): the $num_points parameter is deprecated in %s on line %d -!! [ValueError] Polygon must have at least 3 points \ No newline at end of file +!! [ValueError] Polygon must have at least 3 points