Skip to content

Keep at least one pixel when resizing by width or height - #334

Merged
freekmurze merged 1 commit into
spatie:mainfrom
patrickweh:fix/extreme-aspect-ratio-width-height
Oct 5, 2026
Merged

freekmurze merged 1 commit into
spatie:mainfrom
patrickweh:fix/extreme-aspect-ratio-width-height

Conversation

@patrickweh

Copy link
Copy Markdown
Contributor

width() and height() compute the other dimension with (int) round(...). For images with an extreme aspect ratio that rounds to 0, for example a 588x1 divider line from an email signature resized with width(100). Size::resize() then turns the size into 0x0 and the next aspectRatio() call throws:

DivisionByZeroError: Division by zero
src/Size.php:20 Spatie\Image\Size::aspectRatio
src/Size.php:74 Spatie\Image\Size::resizeWidth
src/Size.php:43 Spatie\Image\Size::resize
src/Drivers/Gd/GdDriver.php:676 GdDriver::resize
src/Drivers/Gd/GdDriver.php:687 GdDriver::width

With Imagick the same input ends in a ValueError. In laravel-medialibrary this fails the whole PerformConversionsJob as soon as such an image is attached.

This clamps the computed dimension to at least 1 pixel in all three drivers, the same way Size::resizeWidth() and Size::resizeHeight() already do. Tests cover a 588x1 image with width(100) and a 1x588 image with height(100).

width() and height() compute the other dimension with round(), which
yields 0 for images with an extreme aspect ratio (e.g. a 588x1 divider
resized to width 100). Size::resize() then divides by zero in
aspectRatio(). Clamp to 1, as resizeWidth()/resizeHeight() already do.

@freekmurze freekmurze left a comment

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.

Looks good. The regression tests cover all three drivers, and CI is green.

@freekmurze
freekmurze merged commit b80b9fa into spatie:main Oct 5, 2026
5 checks passed
@freekmurze

Copy link
Copy Markdown
Member

Thanks @patrickweh for the fix and regression tests! Merged.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants