WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 3 weeks ago

#23127 new defect (bug)

Media Upload hangs on Crunching on too big image sizes.

Reported by: clubdesign Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.7
Component: Media Keywords:
Focuses: Cc:

Description (last modified by SergeyBiryukov)

If the uploaded file is too big for the Max Memory Limit Setting, then the upload is executed, but the image isn't resized to the set image sizes and the frontend gets stuck on "Crunching". I tracked the error down into the core and found out, that the main problem is in the GD library class ( wp-includes/class-wp-image-editor-gd.php ) in the load() function.

Everything works fine before line 91:

$this->image = @imagecreatefromstring( file_get_contents( $this->file ) );

So imagecreatefromstring fails because of a lack of memory. But as a chain reaction everything else fails afterwards. The library is not loaded and sent back, and so on. As a result no id is sent back to the frontend and the Loader hangs.

Maybe there should be any error catching and sending back before this GD function, that the frontend receives any error messages and that the upload is deleted in case of error.

Just my thoughts: As this error is a Fatal one, you can`t use any Exception stuff, cause there will not be any memory left to execute it. Maybe some checking if the id is present before sending the image back to crunching?

Attachments (2)

23127.patch (4.4 KB) - added by gmariani405 22 months ago.
Returns error message on fatal error, displays error message on fatal error if applicable
test.jpg (1.7 MB) - added by gmariani405 22 months ago.
Sample file to exhaust 256M of memory

Download all attachments as: .zip

Change History (23)

#1 follow-up: @clubdesign
6 years ago

Another thought on how to prevent this thing from hanging, is to do some basic calculations before to any GD Stuff.

As this article explains there is a rough formula for that:
Width x Height x 8 (bits) x 3(channels) / 8 x 1.65
should result in the memory needed for GD. Soooo: Max availiable Memory - Wp needed memory - calc. needed memory. If negative, kick error. Just as a thought. ;)

#2 @SergeyBiryukov
6 years ago

  • Description modified (diff)

#3 @SergeyBiryukov
6 years ago

  • Description modified (diff)

#4 @SergeyBiryukov
6 years ago

  • Component changed from Upload to Media

#6 @SergeyBiryukov
6 years ago

#24703 was marked as a duplicate.

#7 in reply to: ↑ 1 @kb_unhammer
6 years ago

Replying to clubdesign:

Another thought on how to prevent this thing from hanging, is to do some basic calculations before to any GD Stuff.

As this article explains there is a rough formula for that:
Width x Height x 8 (bits) x 3(channels) / 8 x 1.65
should result in the memory needed for GD. Soooo: Max availiable Memory - Wp needed memory - calc. needed memory. If negative, kick error. Just as a thought. ;)

As mentioned in #24703, it would also be nice if whatever error message is shown to the user mentions the php.ini memory_limit workaround.

#8 @SergeyBiryukov
6 years ago

#22870 was marked as a duplicate.

#9 @helen
6 years ago

#8754 was marked as a duplicate.

#10 @helen
6 years ago

#7491 was marked as a duplicate.

#11 @helen
6 years ago

#12532 was marked as a duplicate.

#12 @helen
6 years ago

  • Version changed from 3.5 to 2.7

Apparently this is an ages old problem - some older tickets had already been closed, so I left them alone but marked as duplicates.

#13 @markoheijnen
6 years ago

  • Milestone changed from Awaiting Review to 3.7

Moving to 3.7. I will give it a spin this weekend.

#14 @helen
6 years ago

  • Milestone changed from 3.7 to Awaiting Review

No spin, punt.

#15 @helen
6 years ago

#25490 was marked as a duplicate.

#16 @SergeyBiryukov
5 years ago

#27004 was marked as a duplicate.

#17 @SergeyBiryukov
4 years ago

#31879 was marked as a duplicate.

#18 @SergeyBiryukov
4 years ago

#31940 was marked as a duplicate.

#19 @SergeyBiryukov
23 months ago

#41483 was marked as a duplicate.

@gmariani405
22 months ago

Returns error message on fatal error, displays error message on fatal error if applicable

#20 @gmariani405
22 months ago

Uploaded my first real patch, so please let me know if i did not follow protocol or standards, etc. I have code to safely return an error message in the event of a fatal error in this scenario. Also at the suggestion clubdesign, I have it estimating if it will fail and returning an wp_error. The only issue in this case is that I can't (haven't really dug into) get it to return a memory exhausted type error. Instead it just shows "An error occurred in the upload. Please try again later." which is atleast something. If the memory estimating is too unreliable, the fatal handler shows the appropriate memory error. I've also attached a sample image that will 9 times out of 10 cause a memory exhaust error for a limit of 256M. Again, any feedback is appreciated.

@gmariani405
22 months ago

Sample file to exhaust 256M of memory

#21 @spacedmonkey
3 weeks ago

I think we should focus the rest api and this ticket #40410

Note: See TracTickets for help on using tickets.