Make WordPress Core

Opened 9 years ago

Last modified 7 years ago

#33054 new enhancement

Better featured image/attachment sanity checks

Reported by: shawnlunny's profile ShawnLunny Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.2.2
Component: Media Keywords: needs-patch
Focuses: Cc:


Use case:

Add a post: If a user uploads a featured image and the file fails to write to disk, the post record contains the url to the image but of course the image is broken. The end state is one where the featured image is simply broken, but the post record references the image.

Possible solutions:

  1. Do an extra sanity check after write to disk that the image was actually created. If there was a failure alert the user. This is probably the easiest.
  1. Or reverse the order of operations, making it transactional in nature. That is, create the image(s) first, check if successful, and then update the post record. If there is a failure show an alert to the page, and don't write the image path to the post record as the file doesn't exist. It is alot cleaner to not have an image than having a broken one. It is also better if the image was successfully written to disk but the post url failed to update for some reason; we can always re choose from the media library.
  1. Going above and beyond, the sanity check would keep track of thumbnail sizes to be created and make sure they all were successful. If not, don't update the post record and remove the thumbnail sizes that did write to disk. Once again treat as a transaction; all or nothing.

Extended impact:
Many folks use image compression plugins or tools that compress the images before writing to disk. Currently if those items fail usually by timeout you are left with a broken image and the url is in the meta.

I propose that there be a pre-check as to the thumbnails to be created before the image write even occurs (add a new action hook that keeps track of the thumbnails we expect from add_image_size) and then check on the backside to make sure we have the image(s) we expect. After that all sanity check rules proposed above would apply.

Change History (2)

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

7 years ago

#2 @desrosj
7 years ago

  • Keywords needs-patch added

Hey, @ShawnLunny! Welcome to Trac! Thanks for the ticket. Are you interested and able to provide a patch for this?

Note: See TracTickets for help on using tickets.