XML-RPC: wp_upload_bits should act like wp_handle_upload

[markoheijnen]

At this moment there isn't a check for file size when uploading an image through the XML-RPC. The reason is that the method wp_upload_bits is used. This only checks it does is if the file size is to big for a network installation.

The function check_upload_size() is something what you want except that it will use wp_die() if there is an error like this. The function is used as a filter: wp_handle_upload_prefilter. Which only get applied in the function wp_handle_upload.

We should probably change check_upload_size() a bit so it doesn't use wp_die() but does it somewhere else.

related: #6559 and #21085

[SergeyBiryukov]

Related: #20057

[markoheijnen]

Finding a way to add ​https://github.com/daniloercoli/WordPress-streaming-xml-rpc in the new media method. And when it works well in 3.5 also add it in 3.6 for the current one.

[markoheijnen]

Just a weird thought. Would it make sense to fake a $_FILES array to pass through wp_handle_upload(). By using the code of Danilo to store the image localy that would make sense.

Other solution is not using it and fix wp_upload_bits(). Looking again to it and they should both do the same thing. The only difference is the input and how to act on that.

[markoheijnen]

Uploaded my first try to fix the upload limit

[markoheijnen]

Added unfiltered_upload capability check

[markoheijnen]

Added a new patch that checks if a user can upload any file.

[markoheijnen]

Removed wp_check_filetype in the beginning of wp_upload_bits() and added wp_check_filetype_and_ext in the end.
In this way you can manage the check trough the 4th parameter. (copied and edited the code from #11946)

The only bad part is the need of unlink on error since you need to upload the image.

[markoheijnen]

Added mime type matching

[nacin]

In 22471:

Account for unfiltered_upload cap in wp_upload_bits(). see #21292.

[nacin]

Two remaining issues:

• wp_upload_bits() does not use wp_check_filetype_and_ext().

• wp_upload_bits() does not consult check_upload_size().

westi, your call for 3.5 or punt.

[westi]

I think we should punt this from 3.5 and come back in 3.6 to resolve those two issues.

[markoheijnen]

Well for me this is -1. Totally understand why people don't want to contribute on regular basis because of this.
I tried multiple times to get feedback on my latest patch but never gotten one.

[nacin]

1) It's a significant patch with quite a bit of changes, and a lot of potential to break things.

2) There were more pressing changes in XML-RPC, image handling, and the upload APIs for 3.5. Most of that, you participated in.

3) It has no unit tests.

4) It is not a regression, and can easily be traced back to 3.0 — when MU was merged and wp_check_filetype_and_ext() was introduced.

Overall, we're looking at a low priority fix for a low priority ticket. It'll get addressed in a future release. It would be good if more people reviewed it — but the fact that no one did is more indicative of it being a low priority than something that got lost.

[markoheijnen]

Yes, all those issues you can track back to 3.0 since this function didn't changed then.
It is however a high priority ticket since it can effect people badly (no disk space) and it shouldn't matter if no one did reviewed it or had see this issues.

It does has some unit tests but only indirectly through the XML-RPC method unit tests. I will write some more unit tests for it and refresh the patch this week.

[wonderboymusic]

these are all marked 3.7-early

[markoheijnen]

Punting to 3.8 but I marked this on my todo list with high priority ;)

[markoheijnen]

Didn't had time to really look into this. Will tackle this in the next weeks but to late for 3.8

[MikeHansenMe]

see #30284

[wonderboymusic]

In 34577:

XML-RPC: add a wp_uploadFile unit test.

Props MikeHansenMe.
See #21292.

[netweb]

r34577 fails all multisite unit tests, see ​https://travis-ci.org/aaronjorbin/develop.wordpress/builds/82269676

There was 1 failure:
1) Tests_XMLRPC_wp_uploadFile::test_network_limit
Failed asserting that Array (...) is an instance of class "IXR_Error".
/home/travis/build/aaronjorbin/develop.wordpress/tests/phpunit/tests/xmlrpc/wp/uploadFile.php:53

[wonderboymusic]

In 34603:

After [34577], alter wp_xmlrpc_server::mw_newMediaObject() to check upload space in multisite.

See #21292.

[boonebgorges]

The test has to come out - the reason it had been removed in [30530] was that the test has never passed. It'll only pass once the issue raised in this ticket is resolved: namely, that wp_upload_bits() enforces multisite upload space restrictions.

[boonebgorges]

In 34604:

Remove failing xmlrpc upload test.

This test was added [34577], but shouldn't have been, because the bug it
describes has not been fixed.

See #21292.