[wp-trac] [WordPress Trac] #29257: media_sideload_image() calls rename() on temp file, which can break streams

[WordPress Trac]

#29257: media_sideload_image() calls rename() on temp file, which can break streams
--------------------------+-----------------------------
 Reporter:  joehoyle      |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  Media         |    Version:  3.9.2
 Severity:  normal        |   Keywords:
  Focuses:                |
--------------------------+-----------------------------
 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.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/29257>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform