Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#29525 closed defect (bug) (wontfix)

New media - media-new.php broken

Reported by: aliwebdesarrollo Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.8.3
Component: Media Keywords: reporter-feedback close
Focuses: Cc:


1 - Go to Media
2 - Upload a new media file (image file) with regular or multi-upload
3 - Click Upload
4 - media-new.php return blank page without a message.

I found that this is a very common bug in a lot of forums and suddenly i start debug the media-new.php file, to find where its brokes.

It tooks me a while but suddenly i found, it was in the GD class (i also try with imagick class and also broke). Apparently this class is using the function "file_get_contents()" witch is not recommended for this cases instead after a while fighting with this i change this function to "readfile()" and suddenly the file upload perfect.

In wordpress 3.8.3 look into /wp-includes/class-wp-image-editor-gd.php line 104

The change will be:
$this->image = @imagecreatefromstring( file_get_contents( $this->file ) );

change by:
$this->image = @imagecreatefromstring( readfile( $this->file ) );

The most important is that: Readfile will read the file directly into the output buffer, and file_get_contents will load the file into memory, when you echo the result the data is copied from memory to the output buffer effectively using 2 times the memory of readfile. ( http://stackoverflow.com/questions/20095175/php-readfile-vs-file-get-contents#answer-20095276)

Change History (7)

#1 @karpstrucking
6 years ago

  • Keywords dev-feedback added

#2 @nacin
6 years ago

What's the bug here, hitting a memory limit?

#3 @helen
5 years ago

  • Keywords reporter-feedback added; dev-feedback removed

#4 @helen
5 years ago

  • Focuses ui administration removed

#5 @SergeyBiryukov
5 years ago

The change will be:
$this->image = @imagecreatefromstring( file_get_contents( $this->file ) );

change by:
$this->image = @imagecreatefromstring( readfile( $this->file ) );

readfile() returns the number of bytes read from the file, while imagecreatefromstring() here specifically requires the file contents as a string. So it doesn't seem like a direct replacement would work as expected.

#6 @SergeyBiryukov
5 years ago

  • Keywords close added

We could probably use readfile() with output buffering, but that would be the same as using file_get_contents().

We already try to set memory_limit to a maximum possible value right before that line, I don't see what else we could do here.

#7 @wonderboymusic
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

No feedback - @SergeyBiryukov's comment:6 is a good wrap-up

Note: See TracTickets for help on using tickets.