Problem/Motivation

Version: ImageMagick 6.9.10-68 Q16 x86_64
When generating images on our project, performing a conversion to WEBP, we've encountered that during the creation of some images, the following errors/warnings were thrown.

Warning : Undefined array key 1 dans Drupal\imagemagick\Plugin\FileMetadata\ImagemagickIdentify->identify() (/web/modules/contrib/imagemagick/src/Plugin/FileMetadata/ImagemagickIdentify.php ligne 208)

TypeError : trim(): Argument #1 ($string) must be of type string, null given dans trim() (ligne 214 de /web/modules/contrib/imagemagick/src/Plugin/FileMetadata/ImagemagickIdentify.php).

The issue occurs because there is additional output which is retrieved when using the identify command:

Decoded /tmp/magick-6050w68DqfhHR9x9. Dimensions: 328 x 450. Now saving...
Saved file /tmp/magick-60508Ys8A13Dlzsu
format:PAM|width:328|height:450|colorspace:sRGB|profiles:|exif_orientation:

As you can see there is a line "Decoded" and "Saved file" inside, which crashes the code later on.

Proposed resolution

A patch is created to allow to filter out all the values that do not contain a pipe sign, to be able to continue.

Remaining tasks

It might be required to review this, maybe the original cause can be fixed in a different way, or there is another solution possible than checking on the pipe sign.

CommentFileSizeAuthor
#7 outback-sunset-fade-2021-32.jpg1.44 MBmach3.zone
#2 imagemagick-image-metadata-cleanup-3432948-0.patch795 byteshoebekewim

Issue fork imagemagick-3432948

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

hoebekewim created an issue. See original summary.

hoebekewim’s picture

hoebekewim commented
StatusFileSize
new imagemagick-image-metadata-cleanup-3432948-0.patch795 bytes
hoebekewim’s picture

hoebekewim commented
Status: Active » Needs review
hoebekewim’s picture

hoebekewim commented
Issue summary: View changes
meanderix’s picture

meanderix commented

We noticed that for some jpeg images the following command was executed:

/usr/bin/gm identify -format "format:%m|width:%w|height:%h|exif_orientation:%[EXIF:Orientation]\n" "/path/to/image.jpeg"

(Here we're using the graphicsmagick binary, but I guess its the same problem also in imagemagick.)

The array that is returned would look like this:

[
  "format:JPEG|width:1076|height:1151|exif_orientation:1",
  "1",
  "",
]

It seems like token replacement correctly substitutes [EXIF:Orientation] with a "1", but then it also adds the same value to a new line.

ImagemagickIdentify then incorrectly assumes that the lines can be exploded (both with the "|" and the ":" characters).

meanderix’s picture

meanderix commented

This problem also seems to depend on what PHP version you're using. It looks like we were getting only a deprecation warnings in PHP 8.1 and 8.2, but in 8.3 and 8.4 trim() is now throwing an error.

mach3.zone’s picture

mach3.zone commented
StatusFileSize
new outback-sunset-fade-2021-32.jpg1.44 MB

It definitely throws an error that prevents the node from loading.
In my opinion, preventing a node is a major issue.
The patch works for us. Thanks, @meanderix.

I've attached a sample image for analysis purposes.

Also note, that there is a php warning right before the error:

Warning: Undefined array key 1 in Drupal\imagemagick\Plugin\FileMetadata\ImagemagickIdentify->identify() (Zeile 216 in /home/.../contrib/imagemagick/src/Plugin/FileMetadata/ImagemagickIdentify.php)
#0 /home/.../core/includes/bootstrap.inc(166): _drupal_error_handler_real(2, 'Undefined array...', '/home/...', 216)
#1 /home/.../contrib/imagemagick/src/Plugin/FileMetadata/ImagemagickIdentify.php(216): _drupal_error_handler(2, 'Undefined array...', '/home/...', 216)
#2 /home/.../contrib/imagemagick/src/Plugin/FileMetadata/ImagemagickIdentify.php(75): Drupal\imagemagick\Plugin\FileMetadata\ImagemagickIdentify->identify()

TypeError: trim(): Argument #1 ($string) must be of type string, null given in trim() (Zeile 222 in /home/.../contrib/imagemagick/src/Plugin/FileMetadata/ImagemagickIdentify.php).
mondrake’s picture

mondrake
Location 🇮🇹
commented
Version: 4.0.x-dev » 5.0.x-dev

mondrake opened merge request !68

mondrake’s picture

mondrake
Location 🇮🇹
commented

Thank you. All this unexpected text... should it be considered IM or GM buggy behavior? Has anybody checked issue logs?

Anyway, yes it seems like introducing more control in the parsing of identify's output is a good idea.

In the MR, using regex. Also added an unit test with the fail cases reported in the issue.

  • mondrake committed 0323cfda on 5.0.x 
    bug: #3432948 MetaData throwing errors

By: hoebekewim
By: meanderix
By...
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Fixed

Thanks all!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.