imagick image editor does not support streams

[p00ya]

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 streams
• Imagick::readImage and Imagick::writeImage expect filenames, not URLs

I have a working patch which I'll upload to this ticket.

[p00ya]

Patch for #50630

[p00ya]

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?).

[SergeyBiryukov]

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.

[hellofromTonya]

Tagging to denote the ticket has a patch that's ready for review.

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

[p00ya]

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).

[kirasong]

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?

[p00ya]

I'm subbed on #42663 now if you want to move discussion there.

Let me know whether I should:

1. point the PR to #42663
2. add support for very old versions of the imagick extension as #42663's patch did
3. try and call make_image from _save like #42663's patch did (I'm against this)
4. add a dummy stream wrapper for testing like #42663's patch did (instead of testing with file:// URLs)

[kirasong]

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.

Merged into WP Core in https://core.trac.wordpress.org/changeset/49230