Make WordPress Core

Opened 7 weeks ago

Last modified 6 weeks ago

#48853 new defect (bug)

Error generating thumbnails of PDF files

Reported by: joseaneto Owned by:
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.3
Component: Media Keywords: reporter-feedback 2nd-opinion needs-testing
Focuses: Cc:
PR Number:

Description (last modified by SergeyBiryukov)

After updating WordPress to 5.3, we saw how the thumbnails of the PDF files were not generated.

After many tests in different environments (we have tested with PHP versions 7.0, 7.1, 7.2 and 7.3, and in different servers), I found the cause, this change: [46238]

Calling setOption before readImage produces an internal error in Imagick (ImagickException: Failed to read the file) that causes it not to be able to read the file, while moving this line to run after readImage generates thumbnails without problems.

Attachments (2)

48853.diff (1.3 KB) - added by azaozz 6 weeks ago.
48853.1.diff (2.6 KB) - added by azaozz 6 weeks ago.

Download all attachments as: .zip

Change History (19)

#1 @SergeyBiryukov
7 weeks ago

  • Description modified (diff)

#2 @SergeyBiryukov
7 weeks ago

  • Component changed from Upload to Media

#3 @SergeyBiryukov
7 weeks ago

Related/duplicate: #48778

6 weeks ago

#4 @azaozz
6 weeks ago

  • Keywords has-patch reporter-feedback 2nd-opinion added
  • Milestone changed from Awaiting Review to 5.3.1

In 48853.diff: move setOption() after the image has been loaded as suggested.

@joseaneto could you confirm this is now working please.

Needs more testing as it seems to happen only in some ImageMagick versions? But generally don't think setting the option after the image has been loaded can "break" anything.

Also moving to the 5.3.1 milestone for consideration as the change was in 5.3.

Last edited 6 weeks ago by azaozz (previous) (diff)

This ticket was mentioned in Slack in #core-media by azaozz. View the logs.

6 weeks ago

#6 follow-up: @mikeschroder
6 weeks ago

@azaozz after I apply/build 48853.diff , https://core.trac.wordpress.org/raw-attachment/ticket/45598/demo_cropped.pdf no longer is cropped correctly in the generated thumbnail.

Just to double-check it's not my environment -- is that one uploading correctly for you?

#7 in reply to: ↑ 6 @azaozz
6 weeks ago

  • Keywords has-patch removed

Replying to mikeschroder:

Just to double-check it's not my environment -- is that one uploading correctly for you?

Ah, my bad, re-testing with demo_cropped.pdf, the preview image is not cropped with the patch. The setOption( 'pdf:use-cropbox', true ); seem to be ignored.

It works here properly without the patch, ImageMagick 7.0.9, PHP 7.3.

@joseaneto could you confirm if the generated image is cropped when moving that setOption() after readImage() or that just prevents the error you're seeing? If the latter, perhaps there is a way to test for that error before setting the option?

#8 follow-up: @joseaneto
6 weeks ago

Until now I had been testing with normal pdfs, documents, and the error occurred when generating the thumbnail.

Now I see that with the file demo_cropped.pdf, the thumbnail is generated correctly (without applying the patch).

So the bug occurs in pdf files to which ImageMagick cannot apply "use-cropbox".

And true, if I apply the patch even though all thumbnails are generated, the image is not cropped, so setOption is ignored.

  • PHP Version 7.3.12
  • Imagick module version 3.4.4
  • ImageMagick 6.9.4 (as default in Cloudlinux/Centos 7)

Maybe is related with the ImageMagick version vs "pdf:use-cropbox", as we have the same results with other PHP versions, I'm not sure.

#9 in reply to: ↑ 8 ; follow-up: @azaozz
6 weeks ago

Replying to joseaneto:

So the bug occurs in pdf files to which ImageMagick cannot apply "use-cropbox".

Yes, that seems to be the problem. Seems not in ImageMagick but in Ghostscript. This comment by @cranewest has some more info: https://core.trac.wordpress.org/ticket/48778#comment:12 and points to this (ancient) bug in GS 8.7: https://bugs.ghostscript.com/show_bug.cgi?id=690676. Perhaps see if your Ghostscript version is affected and try updating it if possible.

Looking at what can be done in WP as a workaround, doesn't seem there are many options. Perhaps can try to set the option and load the .pdf file, and if it fails, remove the option and try to load again?

#10 @azaozz
6 weeks ago

#48778 was marked as a duplicate.

#11 in reply to: ↑ 9 @joseaneto
6 weeks ago

It's strange, I checked and we have running on all servers version 9.25 of Ghostscript, not too old.

  • ghostscript-9.25-2.el7_7.3.x86_64a

I'll try to debug as @cranewest did and see if it's related with Ghostscript, something like a regresion bug.

6 weeks ago

#12 @azaozz
6 weeks ago

  • Keywords needs-testing added

48853.1.diff is a possible way to retry "loading" the PDF with 'pdf:use-cropbox' option set to false. It introduces another method pdf_load_source() in WP_Image_Editor_Imagick so it does not change load() and pdf_setup() significantly (and not introduce nested try {} catch () {} in load(). This, of course can also be written differently and adding a new method avoided.

It is also not tested very well as I'm having problems reproducing the particular error here (seem not to be able to downgrade Ghostscript to 8.7).

Would be great if this (or a similar patch) can be tested asap so it can make it in 5.3.1. If not, this should probably move to the 5.4 milestone.

#13 @joseaneto
6 weeks ago

Checked the patch in two different env and at least in my case solves the problem, all pdfs, including those that can apply "use-cropbox" generate the thumbnail correctly.

This ticket was mentioned in Slack in #core-media by azaozz. View the logs.

6 weeks ago

#15 @cranewest
6 weeks ago

The patch did not resolve the issue for me (still running Ghostscript 8.7). edit: I still get the exact same errors.

Last edited 6 weeks ago by cranewest (previous) (diff)

#16 @audrasjb
6 weeks ago

  • Milestone changed from 5.3.1 to 5.4

Moving this ticket to WP 5.4, as it still needs testing/patch and since we are getting close to release WP 5.3.1 RC 1.

#17 @azaozz
6 weeks ago

Yeah, don't think this is ready for 5.3.1. 48853.1.diff is a good start but need to make sure it works everywhere, in all cases. These "specific version bugs" are pretty hard to handle well.

Note: See TracTickets for help on using tickets.