wp_upload_bits() should fire wp_handle_upload

[dllh]

I've found a tricky issue with wp_upload_bits(). My use case is that for some file replication functionality, I'm hooking onto wp_handle_upload, which works fine in most cases. But in the case of sideloading thumbnails for an uploaded mp3, my file replication breaks because wp_upload_bits() doesn't fire wp_handle_upload (nor does the sideloading code).

The only two things I've found that use wp_upload_bits() in core are this attachment metadata code and the xml-rpc media endpoint. The xml-rpc code does fire wp_handle_upload, though it does so after inserting the attachment, when it seems like maybe it should happen before insertion (in case there's some error).

I think the way to introduce consistency here is maybe to add wp_handle_upload to wp_upload_bits() and remove it from the xml-rpc endpoint. I'm not sure whether there might be backwards compatibility concerns here, especially with the xml-rpc endpoint.

The repro in my case (which won't apply for most people but which might be helpfully illustrative) is simply to upload an mp3 file. When the file is parsed for ID3 tags and the thumbnail extracted, its bits are processed, but since wp_handle_upload doesn't fire in this context, the file isn't replicated properly and I wind up with a broken image in the media library (and thus a broken thumbnail for the mp3).

You can simulate a repro without having to implement file replication by simply writing a plugin that hooks onto wp_handle_upload and logs some debug info; for normal uploads, you'll see the debug, but in the case of this thumbnail sideload, you'll see debug output for the mp3 but not for its thumbnail. So while the specific file replication repro here isn't terribly common, it could certainly affect other use cases.

I'm attaching a patch that adds wp_handle_upload to wp_upload_bits() and removes it from the xml-rpc function (it won't be necessary since the xml-rpc function calls the here modified wp_upload_bits() function. The main things I'm not sure about in this patch are:

• Are there any backwards compatibility concerns?
• I'm not sure what the best second arg is in this case for wp_handle_upload; in the case of the media metadata, sideload seems appropriate, but is it sort of also appropriate for the xml-rpc or do we consider it an upload in that case? Is there a reasonable way to detect and send along a different context if needed?
• I ran unit tests and nothing broke, though with default config, some tests are skipped. I think this is normal but am not positive.

I've tested the patch on a trunk multisite with both image and mp3 uploads and see no regressions.

[SergeyBiryukov]

Related: #21292

[wonderboymusic]

In 34257:

Uploader: Fire 'wp_handle_upload' in wp_upload_bits(). Thusly, the filter in wp_xmlrpc_server::mw_newMediaObject() is redundant.

Props dllh.
Fixes #33539.