WordPress.org
Get WordPress

Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#28907 closed defect (bug) (fixed)

media_handle_upload calls wp_read_image_metadata on video files, causing errors and timeouts.

Reported by: slangley Owned by: wonderboymusic
Priority: normal Milestone: 4.1
Component: Media Version: 3.8
Severity: normal Keywords: has-patch
Cc: Focuses: performance

Description

When uploading media files, if the mime type is anything other than audio/* then wp_read_image_metadata will be called on the file, even if the file is not an image.

This in turn calls getimagesize which will read the entire file trying to determine if it's (a) an image, and (b) it's dimensions. If we know the uploaded media has a mime type video/* (and probably others) then doing this is pointless. If it turns out your uploaded media is stored remotely, (e.g. on your CDN) then this results in failed uploads and timeouts as it takes a while to completely read large video files.

The most straightforward fix is to just check if the mime type is for a video and skip the call to wp_read_image_metadata, something like

Code highlighting: 

  } else if ( preg_match( '#^video#', $type ) === false ) {
    if ( $image_meta = @wp_read_image_metadata( $file ) ) {
      if ( trim( $image_meta['title'] ) && ! is_numeric( sanitize_title( $image_meta['title'] ) ) )
        $title = $image_meta['title'];
      if ( trim( $image_meta['caption'] ) )
        $content = $image_meta['caption'];
    }
}

Attachments (1)

media.php.patch​ (967 bytes ) - added by jrf 12 years ago.
Patch as suggested by submitter

Download all attachments as: .zip

Change History (8)

#1 @SergeyBiryukov
12 years ago

  • Keywords needs-patch added

@jrf
12 years ago

Patch as suggested by submitter

#2 @jrf
12 years ago

  • Keywords has-patch added; needs-patch removed

I've added a patch implementing the suggestion by slangley.
Better would be IMHO to create a separate case for video which would look for the relevant information, though as there are lots of different places that information could be, that's beyond the scope of this bug.

#3 @aidanlane
12 years ago

Thanks slangley or for filing this bug report and to SergeyBiryukov and jrf handling this, as I was the one that had the original issue (​http://wordpress.org/support/topic/large-uploads-die-with-503-service-unavailable).

However, I wonder if just checking if 'not video' is enough? We use WordPress to host ZIP archives also (for interactives), which seems to be 'natively' supported as an 'archive' (e.g. with wp-includes/images/media/archive.png). Should ZIP archives also be be downloaded and have it's image dimensions evaluated?
Is it too dangerous just to check if the $type is some form of image?

Thanks again for all your help!

#4 @tellyworth
12 years ago

  • Version trunk → 3.8

#5 @wonderboymusic
12 years ago

  • Owner set to wonderboymusic
  • Resolution → fixed
  • Status new → closed

In 29737:

In media_handle_upload(), don't call wp_read_image_metadata() on things that aren't images (like videos). We never caught this error, because we are suppressing it by calling @wp_read_image_metadata().

Props jrf for the initial patch.
Fixes #28907.

#6 @wonderboymusic
12 years ago

  • Milestone Awaiting Review → 4.1

This ticket was mentioned in IRC in #wordpress-dev by celloexpressions. ​View the logs.
12 years ago

Note: See TracTickets for help on using tickets.