Problem/Motivation

PurgeFileManager::buildImageDerivativeUrls() tests whether a file has
derivatives by handing its URI to the image factory:

if (!$this->imageFactory->get($file_uri)->isValid()) {
  return $urls;
}

hook_file_insert() / _update() / _delete() fire for **every** file entity,
so this runs for PDFs, Word documents, archives and so on whenever
purge_file.settings:image_styles is non-empty.

Under GD that is a wasted getimagesize(). Under drupal/imagemagick it is not:
ImagemagickToolkit::isValid() → getMimeType() → parseFile() shells out to identify unconditionally, and ImageMagick then runs the *delegate* for that format — LibreOffice for .doc, Ghostscript for .pdf. Where the delegate is missing, every save of a non-image file logs:

ImageMagick error 1: /usr/bin/mv: cannot stat '/tmp/magick-XXXXXXXX.pdf': No such file or directory
identify-im6.q16: delegate failed `'libreoffice' --headless --convert-to pdf -outdir `dirname '%i'` '%i' 2> '%u'; /usr/bin/mv '%i.pdf' '%o'' @ error/delegate.c/InvokeDelegate/1966.
identify-im6.q16: unable to open file `/tmp/magick-XXXXXXXX': No such file or directory @ error/constitute.c/ReadImage/619.
[command: identify [identify] [-format] [...] [/path/to/files/example.doc]]

Where it is present there is no error, but a converter process is still forked per file, so a default content import, migration or bulk upload serially spawns one external process for every non-image file.

The ImageStyle::supportsUri() check below already excludes these files; the image factory call is only an early bail-out, so this cost buys nothing.

Steps to reproduce

  1. Install drupal/imagemagick and select ImageMagick as the image toolkit.
  2. Enable purge_file and select at least one image style in its settings form.
  3. Save a file entity for a .doc or .pdf, importing default content or uploading a Document media item is enough.
  4. An ImageMagick error is logged for the file and a delegate process is forked.

Proposed resolution

Restrict the derivative lookup to files that can actually have derivatives, and decide that from data already held on the file entity, so that saving a non-image file never reaches the image toolkit at all. Detecting a non-image file must not itself cost an external process.

Images must keep working exactly as they do today, including the existing rejection of files that claim to be images but cannot be read.

Issue fork purge_file-3625005

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:

Comments

frouco created an issue. See original summary.

frouco’s picture

Assigned: frouco » Unassigned

Ready to be reviewed

  • omarlopesino committed fe1e4816 on 2.0.x authored by frouco
    Issue #3625005: Saving a non-image file runs the image toolkit on it,...
omarlopesino’s picture

Looks nice to me. Seems that isValid must be used when we know we have an image.

I will release a rc1 version with this fix, as this has been battle-tested enough I think.

omarlopesino’s picture

Status: Active » Fixed

Released at 2.0.0-rc1

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.