media_sideload_image() calls rename() on temp file, which can break streams

[joehoyle]

I'm not sure where I would put the "blame" on the following issue, but this is the problem as I see it:

1. media_sideload_image() downloads an URL to a temp file on filesystem
2. media_handle_sideload() is called with the file array which calls _wp_handle_upload()
3. _wp_handle_upload() will attempt to rename() the file to the correct uploads dir form the temp file

That should all be fine, unless one is using a custom stream wrapper for their uploads dir (S3, App Engine etc). It's not possible to rename() across streams, from ​http://php.net/manual/en/function.rename.php:

Note:
The old name. The wrapper used in oldname must match the wrapper used in newname.

A couple of solutions I see:

1. Use copy() / unlink() combo in the case of different stream wrappers
2. Make get_temp_dir() filterable so plugins offering custom stream wrappers can replace the temp dir with a directory on the custom stream. This might have some unintended affects though, such as plugin updates etc being downloaded to the custom stream wrapper that may (or may not) be intended for uploads only.

[mattheu]

Use copy & unlink

[mattheu]

Added a patch that uses copy & unlink instead of rename. This solution seems less likely to cause issues.

[mattheu]

A bit more context - the issue arose when using this: ​https://github.com/humanmade/S3-Uploads

[joehoyle]

Patch still applies cleanly.

Discussed briefly with johnbillion, potential issue: server doesn't have room to copy the file so fails. However, in the case of move_uploaded_file being called - then double the image uploaded file size is required as move_uploaded_file does a copy operation, not a rename; any checks that make use of this function will apply to both and I don't think should be applied to this small-fix path.

[chriscct7]

How likely though is it that the server will not have enough room to duplicate an image?
I think that is an almost non-existent occurrence.

[dd32]

This is pretty interesting, in that PHP internally determines when it can't use a standard rename (Say, when it's a different disk volume) and fakes it using copy() instead. Why it doesn't do this with streams I'm not sure.. (I can guess..)

[dd32]

In 35579:

Media: Allow media_sideload_image() to work when the upload directory is a PHP Stream. Using copy() instead of rename() allows the function to work across different stream sources.
Props mattheu.
Fixes #29257