Problem/Motivation

An image style using the aspect switcher effect may cause a division by zero PHP error if the source image is corrupted. Marked as major because this bug took down an important production site without warning.

The website encountered an unexpected error. Please try again later.
DivisionByZeroError: Division by zero in Drupal\image_effects\Plugin\ImageEffect\AspectSwitcherImageEffect->getChildImageStyleToExecute() (line 200 of modules/contrib/image_effects/src/Plugin/ImageEffect/AspectSwitcherImageEffect.php).

Steps to reproduce

  1. Create an image style with at least one aspect switcher effect
  2. Apply the image style to an image field display formatter (i.e. on a content type)
  3. Upload a corrupted image to the image field and save
  4. Attempt to view the image field via the display formatter

An example corrupted image is attached. It's a plain text file with a JPEG extension.

Proposed resolution

Verify the image toolkit was able to get the image dimensions before attempting to apply transformations.

CommentFileSizeAuthor
corrupted.jpg16 bytesbobooon

Issue fork image_effects-3357133

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

robphillips created an issue. See original summary.

robphillips opened merge request !32

bobooon’s picture

bobooon commented
Status: Active » Needs review

Patch from MR. If either width or height is not a positive number do not return a style name.

https://git.drupalcode.org/project/image_effects/-/merge_requests/32/dif...

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks.

1) instead of returning in getChildImageStyleToExecute, I'd rather avoid to call it in applyEffect and transformDimensions when the image is invalid.
2) an automated test would help prevent regressions.

aporie’s picture

aporie
Primary language French
commented

There is a duplicated issue #3136292: Division by zero in AspectSwitcherImageEffect which patch caused us 

Error: Xdebug has detected a possible infinite loop, and aborted your script with a stack depth of '256' frames in is_int() (line 331 of core/lib/Drupal/Component/Utility/NestedArray.php).

This patch is fine though (and seems an easier approach) than the attached duplicated issue.

aporie’s picture

aporie
Primary language French
commented
Related issues: +#3136292: Division by zero in AspectSwitcherImageEffect
mvonfrie’s picture

mvonfrie commented

MR !32 works, but this is only half of the truth. In my case the image was not corrupted but the thumbnail of a document media entity embedded in a body field via CKeditor.

Steps to reproduce

  1. Create an image style with at least one aspect switcher effect.
  2. Enable Drupal Media with the Document media type including thumbnails.
  3. Configure a text format using CKeditor5 to allow adding Drupal media documents.
  4. Configure media documents default view mode to include the thumbnail image using the image style created before.
  5. Upload a document via Drupal media and ensure it has a valid, non-corrupted thumbnail image.
  6. Edit the body of any content using the configured text format, add the uploaded document via the Drupal Media button.
  7. Observe an error message in the CKeditor preview.
  8. Save and attempt to view the content with the included document.
bobooon’s picture

bobooon commented
Version: 8.x-3.x-dev » 4.0.x-dev

robbiehobby changed the visibility of the branch 8.x-3.x to hidden.

robbiehobby opened merge request !65

robbiehobby closed merge request !32