Skip to content

fix: Prevent DivisionByZeroError in BaseHandler::reproportion() with float 0.0#10319

Open
gr8man wants to merge 3 commits into
codeigniter4:developfrom
gr8man:fix-reproportion-division-by-zero
Open

fix: Prevent DivisionByZeroError in BaseHandler::reproportion() with float 0.0#10319
gr8man wants to merge 3 commits into
codeigniter4:developfrom
gr8man:fix-reproportion-division-by-zero

Conversation

@gr8man

@gr8man gr8man commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Description
This PR fixes a potential DivisionByZeroError inside BaseHandler::reproportion() when processing image resizing operations where dimensions (origWidth or origHeight) might evaluate to 0.0 (float) or when provided dimensions are invalid.

Previously, the strict comparison check === 0 on float 0.0 allowed the execution to pass through the initial guard clause, causing a crash during proportion calculations.

Changes:

  • Refactored reproportion() to safely cast inputs to int and centralize dimension validation.
  • Added strict checks via is_numeric() to ensure both provided and original dimensions are mathematically valid.
  • Introduced testReproportionWithFloatZero in BaseHandlerTest utilizing Reflection to directly test the protected method's early return behavior without side effects.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdoc fully completed
  • Unit testing, with >80% coverage
  • User guide updated (if applicable)

Comment thread system/Images/Handlers/BaseHandler.php Outdated
$w = (int) $this->width;
$h = (int) $this->height;

if (! is_numeric($this->width) || ! is_numeric($this->height) || $origW === 0 || $origH === 0 || ($w === 0 && $h === 0)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_numeric() is much broader than ctype_digit(). 0.0 should be checked explicitly.

Comment thread tests/system/Images/BaseHandlerTest.php Outdated
$expectedWidth = $handler->getWidth();
$expectedHeight = $handler->getHeight();

$method = new ReflectionMethod($handler::class, 'reproportion');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalsn michalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Jun 18, 2026
@gr8man gr8man requested a review from michalsn June 18, 2026 19:22
@michalsn

Copy link
Copy Markdown
Member

I should have checked this earlier, but it looks like the PHPDoc for Image::$origWidth and Image::$origHeight is too broad. These values are populated from getimagesize(), and PHP implementation reads image dimensions as integer values: https://github.com/php/php-src/blob/master/ext/standard/image.c

Since getimagesize() returns integer width and height values, Image::$origWidth and Image::$origHeight should probably be documented as int rather than float|int. I don't think we should add defensive 0.0 handling to reproportion().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants