#50630 closed defect (bug) (duplicate)
imagick image editor does not support streams
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 5.5 |
Component: | Media | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
The imagick image editor (in particular the load
and save
methods) do not work with streams.
The use case for having the image editor work with streams is when using plugins like https://wordpress.org/plugins/gcs/ that register a custom stream handler (in that case for "gs://" URLs) and add a filter to upload_dir
so that uploads result in RPCs to external services.
There are a few reasons the original code didn't work:
realpath
doesn't work with streamsImagick::readImage
andImagick::writeImage
expect filenames, not URLs
I have a working patch which I'll upload to this ticket.
Attachments (1)
Change History (14)
#1
@
4 years ago
Some notes on the patch:
The patch uses Imagick::getImageBlob
and Imagick::readImageBlob
. I did also investigate using readImageFile
and writeImageFile
, but these appear to misbehave with some stream handlers (probably a bug in the PHP extension?).
This ticket was mentioned in Slack in #core by p00ya. View the logs.
4 years ago
#3
@
4 years ago
- Milestone changed from Awaiting Review to 5.6
Hi there, welcome to WordPress Trac! Thanks for the ticket.
Just noting that the patch looks good to me at a glance. Moving to the milestone so it doesn't get lost.
This ticket was mentioned in Slack in #core by thewebprincess. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#6
@
4 years ago
- Keywords has-patch added
Tagging to denote the ticket has a patch that's ready for review.
This ticket was mentioned in PR #521 on WordPress/wordpress-develop by p00ya.
4 years ago
#7
- Keywords has-unit-tests added
Support streams from the imagick image editor. Fixes problems with using realpath
and imagick::readImage
with streams.
Trac ticket: https://core.trac.wordpress.org/ticket/50630
#8
@
4 years ago
Hi, just a note to any reviewers that they should look at the github PR rather than reviewing the patch I attached here on trac (which now has merge conflicts).
#9
@
4 years ago
Thanks so much for the patch and tests!
Agreed @SergeyBiryukov, let's see if we can fix this for 5.6.
Previously: #42663
Thinking maybe we should move this over there, so the discussion continues in the same place?
#10
@
4 years ago
I'm subbed on #42663 now if you want to move discussion there.
Let me know whether I should:
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#12
@
4 years ago
- Milestone 5.6 deleted
- Resolution set to duplicate
- Status changed from new to closed
Sounds good -- Yes, please point the PR over there, so we can discuss your notes/questions there.
Since the (other) ticket was started a while ago, I'm definitely open to a change in approach from what was commented there. Just want to be sure we take those concerns into account when choosing what goes in!
I'm going to mark this one as a duplicate, then assign #42663 to 5.6, like this one was, so it stays on track.
dream-encode commented on PR #521:
4 years ago
#13
Merged into WP Core in https://core.trac.wordpress.org/changeset/49230
Patch for #50630