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
- Install
drupal/imagemagickand select ImageMagick as the image toolkit. - Enable
purge_fileand select at least one image style in its settings form. - Save a file entity for a
.docor.pdf, importing default content or uploading a Document media item is enough. - An
ImageMagick erroris 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
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
Comment #3
frouco commentedReady to be reviewed
Comment #5
omarlopesinoLooks 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.
Comment #6
omarlopesinoReleased at 2.0.0-rc1