Make WordPress Core

Opened 2 years ago

Closed 15 months ago

#48853 closed defect (bug) (fixed)

Error generating thumbnails of PDF files

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

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 (3)

48853.diff (1.3 KB) - added by azaozz 2 years ago.
48853.1.diff (2.6 KB) - added by azaozz 2 years ago.
48853.2.diff (3.1 KB) - added by azaozz 2 years ago.

Download all attachments as: .zip

Change History (39)

#1 @SergeyBiryukov
2 years ago

  • Description modified (diff)

#2 @SergeyBiryukov
2 years ago

  • Component changed from Upload to Media

#3 @SergeyBiryukov
2 years ago

Related/duplicate: #48778

2 years ago

#4 @azaozz
2 years 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 2 years ago by azaozz (previous) (diff)

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

2 years ago

#6 follow-up: @mikeschroder
2 years 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
2 years 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
2 years 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
2 years 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
2 years ago

#48778 was marked as a duplicate.

#11 in reply to: ↑ 9 @joseaneto
2 years 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.

2 years ago

#12 @azaozz
2 years 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
2 years 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.

2 years ago

#15 @cranewest
2 years 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 2 years ago by cranewest (previous) (diff)

#16 @audrasjb
2 years 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
2 years 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.

#18 @dantahoua
2 years ago

Just to say, I tested with different version of ImageMagick.
Never worked until I change the order of the setOption after readImage.
So the patch is working here with all version of imagick and imagemagck I tested (Image Magick 6.7 to 7).
Have to make some test with different version of Ghostscript.
WP 5.3.2
PHP 7.2.26
Imagick 3.4.4
Image Magick 7.0.9-17
CentOs 6
GhostScript 8.70

#19 @dantahoua
2 years ago

Just made a test with ghostscript-9.50. Everything works fine with OR without the patch. I'm lucky to be able to change the Ghostscript version but not everybody could do that, so I suggest to go with the Patch as it seems to resolve the problem in many case...

#20 @mikeschroder
2 years ago

I found this problem on an MWP test server at GoDaddy running GhostScript 8.70, causing the WP tests to fail.

While the errors are still very visible (since it still "crashes"), 48853.1.diff lets them pass as expected.

2 years ago

#21 @azaozz
2 years ago

  • Milestone changed from 5.4 to 5.5

Looking at 48853.1.diff again, it may be better to deprecate pdf_setup() function in favor of pdf_load_source(). The latter does the "setup" and then tries to load the source.

In 48853.2.diff: deprecate pdf_setup() and introduce pdf_load_source(). The latter does the "setup" and then tries to load the source. If it fails, it tries to run gs without the use-cropbox option. It also uses a nested try/catch as it tests for several possible failures.

I still couldn't reproduce this, having troubles installing (compatible) ImageMagic and GNU Ghostscript 8.7. Also seems it's too late to add this in 5.4, moving to the next milestone.

#22 follow-up: @mikeschroder
2 years ago

I'll test the new patch, thanks @azaozz! I'll note, I'm fine with the previous patch/method.
Would you mind explaining a bit more why you think it's a good idea to deprecate and create a new method?

I'm also wondering why this is too late for 5.4. I know we had been considering it for a minor release, it's a bug, and I think it'd be great if we could fix it for folks earlier, given that there are almost two weeks left in beta before RC.

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

2 years ago

This ticket was mentioned in Slack in #hosting-community by mike. View the logs.

2 years ago

This ticket was mentioned in Slack in #hosting-community by mike. View the logs.

23 months ago

#26 @audrasjb
23 months ago

#48778 was marked as a duplicate.

#27 @skylabb
23 months ago

I have the same issue uploading PDF with exact error message. Some more details per my observation:

I don't think the problem is caused by any plugin conflict. I deactivated all plugins for the site and the problem persisted. Of this same site running the past couple years using the same server, WP theme and plugins, I never had this issue until recently along with a bunch of other users after WP upgrades.

The issue is that after I hit the upload button, a new square box in the media lib view pan shows the upload process with a progress RED bar. Then the error message comes up. And if I refresh my screen, the uploaded file shows up in libary list as normally. It seems auto server response process is not working properly in the uploading process requiring a manual screen refresh.

Also interestingly, I have a different WP stall in latest version on a different server and using different theme, the second site doesn't have the upload issue. The square box in library pan shows a BLUE upload progress bar, then the screen refreshes itself to show success upload updates.

From my observation above, I'd say the latest versions of WP are probably having some glitch or are conflicting with certain server environments or design themes. But, you can't blame the server or theme because these existing elements used to be working just fine with previous WP versions. It is WP that introduces something new in recent versions and comes into conflict with existing setups. And this seems to be affecting quite a few sites, not just a couple isolated cases. In other words, WP is the problem per my conclusion.


UPDATE 2/27/20

After an exhausting elimination process, I believe I've found the culprit causing PDF thumbnail generation error messages - "Unexpected response from the server.." or "HTTP error." (screenshot: https://ibb.co/28WCtf5)

The elements have been eliminated are: server environment, PHP version, themes, plugins, directory permission - all the usual suspects. In this case, they have little to do with the problem. Lowest WP version tested for is 5.0.8.

As I have eliminated pretty much all of the above, what's left to give? As the subject title of this problem suggests - "Error generating THUMBNAILS of PDF files." So I went to check under Media Settings area (screenshot: https://ibb.co/fqCPCvS), I noticed that I had custom thumbnail sizes. So I changed these numbers back to default settings. And voila, problem was gone and things were working the way they were supposed to.

So theoretically, if users can enter any custom thumbnail size numbers that suite their needs, this should not break Wordpress. Shall we call this the PDF thumbnail bug?

Last edited 23 months ago by skylabb (previous) (diff)

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

22 months ago

#29 @n5hzr
20 months ago

I'm seeing this same issue when uploading PDF files. My max upload is set to 600 MB. PDF files < 26.9 MB will upload without any trouble. PDF files > 32.2 MB produce this error. This is running on IIS, and PHP 7.1 with WP 5.4.1

I've tried the 48853.2 patch listed above and it didn't make any difference in my operation.

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

19 months ago

#32 @SergeyBiryukov
18 months ago

  • Milestone changed from 5.5 to Future Release

Looks like this still needs more testing, moving to a future release for now.

#33 in reply to: ↑ 22 ; follow-up: @mikeschroder
17 months ago

Replying to mikeschroder:

Would you mind explaining a bit more why you think it's a good idea to deprecate and create a new method?

As it seems 8.7.0 is a version that is standard (and seems to be current) on CentOS 6, thought I'd ping to start conversation again here.

Currently I prefer the approach in 48853.1.diff -- could you please explain a little on why you decided to switch to deprecation, @azaozz?

#34 in reply to: ↑ 33 @azaozz
17 months ago

  • Milestone changed from Future Release to 5.6

Replying to mikeschroder:

Currently I prefer the approach in 48853.1.diff...

Right. The only difference is 48853.1.diff changes how protected function pdf_setup() works, but agree it may be the better option (the function is protected, changing it shouldn't bring any back-compat concerns). Please feel free to commit it :)

Bringing to 5.6 ideally with a bit more testing.

#35 @mikeschroder
17 months ago

  • Owner set to mikeschroder
  • Status changed from new to assigned

Thanks @azaozz!

Self-assigning for 5.6.

#36 @mikeschroder
15 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 49174:

Media: Work around use-cropbox bug in Ghostscript 8.70

Wraps Imagick::readImage() for PDFs with exception handling, trying again without use-cropbox if this fails.

Introduces WP_Image_Editor_Imagick::pdf_load_source().

Works around a known issue in Ghostscript 8.70 (fixed in 8.71) that results in a stack underflow.
While it only affects this version, it remains a common version found on hosts, and prevented some PDF thumbnails from being generated.

See this Ghostscript bug for more details: https://bugs.ghostscript.com/show_bug.cgi?id=690676

Props azaozz, joseaneto, cranewest, dantahoua, n5hzr, mikeschroder.
Fixes #48853.

Note: See TracTickets for help on using tickets.