Media & WXR Uploads: Inaccurate error reporting because move_uploaded_file() fails to return false.

[emhr]

Steps to reproduce:
Current month directory exists in uploads directory hierarchy but is not writable.

Case-1 Upload a new media item. Data stored in database but file is not uploaded.

Case-2 Upload a WXR import file using the WP importer plugin. Plugin reports it's fallback error message: "This does not appear to be a WXR file, missing/invalid WXR version number" even if the WXR file has a valid wxr version number. The import fails.

Expected behavior:
The uploader should return the following error, "The uploaded file could not be moved to"…path

Solution:
Pass precise path to wp_tempnam() instead of default

Bug Since: #18206

Related: #19720

[emhr]

minor optimization

[duck_]

Good catch, thanks!

The changeset that caused this was [18482].

I am uploading two different patch options:

1. Check the return value of copy() and return an error then.
2. move_uploaded_file() to the uploads directory as before. If a resize is requested the new, resized image is made and renamed.

The second has the benefit of only moving/copying the upload once if a resize is not requested. Currently we always move and then copy anyway. I haven't done proper testing for when a resize is requested for either (though it's only the second that could have a bug), and I don't think the second does enough to ensure that the resized file name is unique.

A couple of other things I noticed about [18482]:

1. If the upload cannot be moved to wp_tempnam($filename) (this is rather unlikely) then the error message will display an incorrect path
2. There's no feedback returned to the user if an image resize fails

A third option is to remove this resizing code. This was actually (mostly) done in [19223] for #19174, but it came back in [19225] and [19226] to allow plugins to make use of resizing. However, since this code has been in wp_handle_upload() for a release it probably cannot be taken out now.

Finally, I'm not really sure why, but I seem to feel that it's a strange location for resizing code.

[emhr]

I tested 19800.check-copy.diff and 19800.less-copying.diff They both provide proper error reporting for the two cases I mentioned.
They both assign unique filenames as expected for all files including the resized ones.

19800.less-copying.diff looks great in comparison. Reducing disk writes is good.

[nacin]

It would be good to get azaozz's input, but I would pull the resizing code.

[azaozz]

Replying to nacin:

...but I would pull the resizing code.

Agreed. The best option is to let Plupload resize images (before uploading), the PHP functionality was a fallback for the browsers that still don't support this in JS.

[duck_]

In [20019]:

Drop image resizing code from wp_handle_upload(). Fixes #19800.

This code stops wp_handle_upload() from reporting errors when the upload couldn't be moved to its final local and it was a non-JS fallback that is unused.