Opened 10 months ago

Last modified 2 weeks ago

#21292 assigned defect (bug)

XML-RPC: wp_upload_bits should act like wp_handle_upload

Reported by: markoheijnen Owned by: westi
Priority: normal Milestone: Future Release
Component: XML-RPC Version: 3.0
Severity: normal Keywords: has-patch 3.7-early
Cc: ercoli@…, Viper007Bond, lol@…

Description (last modified by 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

Attachments (3)

21292.diff (3.5 KB) - added by markoheijnen 10 months ago.
21292-2.diff (3.9 KB) - added by markoheijnen 10 months ago.
Added unfiltered_upload capability check
21292-3.diff (5.4 KB) - added by markoheijnen 10 months ago.
Added mime type matching

Download all attachments as: .zip

Change History (22)

  • Description modified (diff)
  • Cc ercoli@… added

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.

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.

Uploaded my first try to fix the upload limit

Added unfiltered_upload capability check

  • Description modified (diff)
  • Summary changed from Using wp_handle_upload in the XML-RPC to XML-RPC: wp_upload_bits should act like wp_handle_upload

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

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.

Added mime type matching

  • Cc viper007bond added
  • Cc Viper007Bond added; viper007bond removed
  • Keywords has-patch added; needs-patch removed

In 22471:

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

  • Owner set to westi
  • Status changed from new to assigned

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.

  • Keywords 3.6-early added
  • Milestone changed from 3.5 to Future Release

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

  • Cc lol@… added

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.

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.

  • Keywords 3.6-early removed
  • Milestone changed from Future Release to 3.6
  • Version changed from 1.5 to 3.0

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.

  • Keywords 3.7-early added; dev-feedback removed
  • Milestone changed from 3.6 to Future Release
Note: See TracTickets for help on using tickets.