ext/gd: imagewebp/imageavif/imagepng/imagejpeg stricter checks quality/speed. #14485
Conversation
devnexen
force-pushed
the
gd_image_exceptions
branch
from
91e9210 to
925dcc3
Compare
devnexen
changed the title
ext/gd/gd.c
Outdated
| gdImageJpegCtx(im, ctx, (int) quality); | ||
| break; | ||
| #endif | ||
| #ifdef HAVE_GD_WEBP | ||
| case PHP_GDIMG_TYPE_WEBP: | ||
| if (quality == -1) { | ||
| if (quality < -1) { | ||
| zend_argument_value_error(3, "must be at least -1"); |
There was a problem hiding this comment.
The "greater than or equal to" style is more commonly used with "at least" having only a few hits in the codebase.
ext/gd/gd.c
Outdated
| zend_argument_value_error(3, "must be at least -1"); | ||
| ctx->gd_free(ctx); | ||
| RETURN_THROWS(); | ||
| } else if (quality == -1) { |
There was a problem hiding this comment.
This "else if" branch is actually unnecessary:
php-src/ext/gd/libgd/gd_webp.c
Lines 120 to 122 in c7366cf
There was a problem hiding this comment.
yes saw that but was wondering if this is a "recent" addition was wondering why it is in place here then.
There was a problem hiding this comment.
It seems that the code previously used virtual functions so that might've been harder to see what the possible call targets are and therefore harder to see what's necessary and not. I'm fine with keeping this of course.
It's been in the library part since 2015: 00ba7e6
There was a problem hiding this comment.
I see. happy to remove anyway :)
devnexen
force-pushed
the
gd_image_exceptions
branch
from
925dcc3 to
d7b4eb9
Compare
|
@nielsdos I'd like to point out that the default quality of 80 that was dropped in this PR was only added to system gd from version 2.2.0, while we require 2.1.0 as minimum version, so this can theoretically break installations linking to system libgd 2.1 (no idea if any exists). |
|
Thanks for pointing this out. |
|
Definitively, 2.1.0 is 10 years+ old it seems. |
Then feel free to just bump it. |
No description provided.