Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#24459 closed defect (bug) (fixed)

Editing images with GD breaks with streams

Reported by: rmccue Owned by: ryan
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: has-patch reporter-feedback
Focuses: Cc:


See #18543 for the original ticket when support for this was added.

Unfortunately, when this was committed, it was inadvertently broken. Although the $filename parameter is set to null, this is never used, and instead $arguments is passed to the GD functions. GD then receives a stream URL, and breaks completely.

Instead, $arguments[1] should be set to null in WP_Image_Editor::make_image() if the URL is a stream.

Attachments (4)

24459.diff (414 bytes) - added by rmccue 8 years ago.
24459.2.diff (873 bytes) - added by markoheijnen 8 years ago.
24459.3.diff (1.0 KB) - added by nacin 8 years ago.
24459.4.diff (1.8 KB) - added by markoheijnen 8 years ago.

Download all attachments as: .zip

Change History (15)

8 years ago

#1 @rmccue
8 years ago

  • Keywords has-patch added

#2 @SergeyBiryukov
8 years ago

  • Milestone changed from Awaiting Review to 3.6

#3 @nacin
8 years ago

markoheijnen, DH-Shredder?

#4 @rmccue
8 years ago

For reference, you can reproduce via App Engine, disabling the workaround code in there (line 508).

8 years ago

#5 @markoheijnen
8 years ago

The patch looks fine to me. Did remove the $dst_file variable in my patch since that isn't needed anymore.

8 years ago

8 years ago

#6 @markoheijnen
8 years ago

Added a new patch that moves this logic to the GD editor. We can check if arguments[1] is set and then unset it but you never know what other implementation someone has. With previous patches imagick will fatal error since it would send 2 arguments instead of only 1.

I did some basic tests but not yet with streams. My laptop is crashing randomly.

#7 @markoheijnen
8 years ago

  • Keywords reporter-feedback added

Ryan, can you test out the latest patch?

#8 @nacin
8 years ago

24459.4.diff seems to make sense to me.

#9 @aaroncampbell
8 years ago

I tested 24459.4.diff and it seems to work for me.

Last edited 8 years ago by aaroncampbell (previous) (diff)

#10 @ryan
8 years ago

I tested editing images and creating headers (from uploads and from library) on wordpress.com with local and replicated files. Seems fine, looks good.

#11 @ryan
8 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In 24727:

Fix editing images with GD when using streams.

Props rmccue, markoheijnen, nacin
fixes #24459

Note: See TracTickets for help on using tickets.