WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 9 months 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:

Description

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 11 months ago.
24459.2.diff (873 bytes) - added by markoheijnen 9 months ago.
24459.3.diff (1.0 KB) - added by nacin 9 months ago.
24459.4.diff (1.8 KB) - added by markoheijnen 9 months ago.

Download all attachments as: .zip

Change History (15)

rmccue11 months ago

comment:1 rmccue11 months ago

  • Keywords has-patch added

comment:2 SergeyBiryukov11 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:3 nacin9 months ago

markoheijnen, DH-Shredder?

comment:4 rmccue9 months ago

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

markoheijnen9 months ago

comment:5 markoheijnen9 months ago

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

nacin9 months ago

markoheijnen9 months ago

comment:6 markoheijnen9 months 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.

comment:7 markoheijnen9 months ago

  • Keywords reporter-feedback added

Ryan, can you test out the latest patch?

comment:8 nacin9 months ago

24459.4.diff seems to make sense to me.

comment:9 aaroncampbell9 months ago

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

Last edited 9 months ago by aaroncampbell (previous) (diff)

comment:10 ryan9 months 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.

comment:11 ryan9 months 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.