REST API: attachments created from a `url` skip the shared insert path in `create_item()`

[adamsilverstein]

@jeremyfelt walked the 7.1 media upload paths while reviewing #65808 and found four gaps, in ​https://github.com/WordPress/wordpress-develop/pull/12833#issuecomment-5184878176. Opening this separately since the fix is a restructure rather than the argument registration that ticket covers.

create_item_from_url(), added in [62659], builds and returns its own response. A request that supplies a url therefore returns before create_item() does any of its work around the insert, and three of the four gaps fall out of that:

• Fields the schema accepts are silently dropped. title, caption, description, alt_text, featured_media, meta and terms are all applied by create_item() after insert_attachment() returns, which the URL path never reaches. They are accepted on the request and then thrown away.
• rest_pre_insert_attachment and rest_insert_attachment never fire. They fire from prepare_item_for_database() and insert_attachment() respectively, neither of which the URL path calls. Only rest_after_insert_attachment fires.
• wp_after_insert_post never fires either. The comment in create_item_from_url() says media_handle_sideload() handles it, but that isn't right: wp_insert_post() returns for attachments at ​post.php:5212, before the $fire_after_hooks block, so the hook cannot fire from that call no matter what is passed. create_item() compensates by calling wp_after_insert_post() explicitly; the URL path has nothing equivalent.

The fourth is separate: a request can supply both an uploaded file and a url. create_item() branches on url first, so the uploaded file is silently discarded rather than rejected.

Worth noting none of this is broken for the editor. The "Upload to Media Library" button and the pre-publish external media panel both post only post and url, so they don't hit any of it. This matters because the endpoint is public API, and as @jeremyfelt put it, it will be harder to correct once people discover it and start relying on the current shape.

Proposed change

Treat a URL as the third source of the file, alongside a multipart upload and a raw request body, rather than as a separate route through the controller:

• Replace create_item_from_url() with upload_from_url(), a sibling of upload_from_file() and upload_from_data(). It downloads the remote file, hands it to wp_handle_sideload() and returns the same data the other two handlers return.
• Call it from insert_attachment(), so insert_attachment() and create_item() perform the insert for every path. All three items above are fixed by construction rather than by duplicating the logic.
• Return a 400 from create_item() when a request supplies both a file and a url.
• Check prepare_item_for_database() for a WP_Error in insert_attachment(), so an error returned from rest_pre_insert_attachment is honored rather than used as if it were an attachment.

upload_from_url() is a rename of a method added this cycle in [62659] and not present in any released version, so there is no back-compat concern. The upload_files check inside it is dropped as redundant, since create_item_permissions_check() already denies a user without that capability before the callback runs, which is why the check was unreachable through the endpoint.

Sideloading also picks up two things it did not have: the parent post's date is used to place the file in the uploads folder, matching media_handle_upload(), and exif title and caption defaults are read from the image.

Testing instructions

Automated:

npm run test:php -- --filter 'WP_Test_REST_Attachments_Controller' --group restapi
npm run test:php -- -c tests/phpunit/multisite.xml --filter 'WP_Test_REST_Attachments_Controller' --group restapi

Five new tests, one per gap: a file and a url together are rejected, a raw body upload and a url together are rejected, the request fields are applied, the three insert hooks fire, and an error from rest_pre_insert_attachment is honored. All five fail without the change - I checked by stashing the source change and re-running.

Full WP_Test_REST_Attachments_Controller class: 201 tests on single site and 207 on multisite, 2 skipped in each, 0 failures. Full restapi group: 3554 tests, one unrelated pre-existing error in Test_oEmbed_Controller::test_proxy_with_classic_embed_provider, which fails identically with the change stashed. phpcs --standard=phpcs.xml.dist clean on both changed files.

Manual:

1. POST /wp/v2/media with a url and a title, caption, description and alt_text. Before the change only the URL is honored; after it, the fields are stored on the attachment.
2. Hook rest_insert_attachment and wp_after_insert_post and post the same request. Neither fires before the change; both do after.
3. POST /wp/v2/media with both a file and a url. Before the change the file is silently discarded; after it, the request returns a 400.
4. Insert an external image in the editor and use "Upload to Media Library". Unchanged, since that request only sends post and url.

References

• Core PR: ​https://github.com/WordPress/wordpress-develop/pull/PENDING
• Original review comment: ​https://github.com/WordPress/wordpress-develop/pull/12833#issuecomment-5184878176
• Follow up to [62659], [62841], and to #65517 and #65808.

Addresses the four items @jeremyfelt found while walking the 7.1 media upload paths in ​https://github.com/WordPress/wordpress-develop/pull/12833#issuecomment-5184878176.

Three of the four fall out of one thing: create_item_from_url() builds and returns its own response, so a request that supplies a url returns before create_item() does any of its work around the insert. Fields the schema accepts are dropped, rest_pre_insert_attachment and rest_insert_attachment never fire, and neither does wp_after_insert_post. The comment saying media_handle_sideload() handles that last one isn't right - wp_insert_post() returns for attachments at post.php:5212, before the $fire_after_hooks block, so it can't fire from there no matter what is passed. create_item() calls it explicitly, which is why the uploaded-file path is fine.

Rather than copy that tail into the URL path, this makes a URL the third source of the file, alongside a multipart upload and a raw request body. create_item_from_url() becomes upload_from_url(), a sibling of upload_from_file() and upload_from_data(): it downloads the remote file, hands it to wp_handle_sideload(), and returns the same data they return. insert_attachment() picks the handler and create_item() does the rest for every path, so the three items are fixed by construction rather than by re-implementing them. That's your second suggestion @jeremyfelt - it turned out to fit the existing shape of the class better than merging the two create_item methods.

The fourth item is separate and is just an explicit 400: a request that supplies both a file and a url was silently discarding the file. The check looks at the multipart file params and at the Content-Disposition filename, since that's how upload_from_data() identifies a raw body upload.

Also added the is_wp_error() check on the pre-hook you mentioned, so an error returned from rest_pre_insert_attachment is honored instead of being used as if it were an attachment.

## Two things worth a look

• upload_from_url() is a rename of create_item_from_url(), which was added in [62659] this cycle and isn't in any released version, so there's no back-compat cost. But it is a protected method that's been sitting in beta, so flagging it rather than burying it.
• I dropped the upload_files check that was inside the old method. create_item_permissions_check() already denies that capability before the callback runs, so the check was unreachable through the endpoint - its 403 could never actually be returned, the permission check's 400 always won first. The test for it now dispatches through the endpoint instead of invoking the method by reflection. Happy to put it back if you'd rather keep the belt and braces.

Sideloading also picks up two things it didn't have, both just from joining the shared path: the parent post's date is used to place the file in the uploads folder, matching media_handle_upload(), and exif title and caption defaults are read from the image.

Note this doesn't change anything for the editor. As you worked out, "Upload to Media Library" and the pre-publish external media panel both send only post and url, so they never hit any of it.

## Stacked on two other PRs

This branch includes #12825 (size ceiling on the sideload path) and #12833 (always register the url argument), since all three touch these same two methods. The diff against trunk therefore shows all three. The parity commit itself is the only new one here: 0b7ea15981.

## How has this been tested

npm run test:php -- --filter 'WP_Test_REST_Attachments_Controller' --group restapi
npm run test:php -- -c tests/phpunit/multisite.xml --filter 'WP_Test_REST_Attachments_Controller' --group restapi

• Five new tests, one per item: a file and a url together are rejected, a raw body upload and a url together are rejected, the request fields are applied, the three insert hooks fire, and an error from rest_pre_insert_attachment is honored. All five fail without the change - I checked by stashing the source change and re-running.
• Full WP_Test_REST_Attachments_Controller class: 201 tests on single site and 207 on multisite, 2 skipped in each, 0 failures.
• Full restapi group: 3554 tests. One pre-existing unrelated error, Test_oEmbed_Controller::test_proxy_with_classic_embed_provider, which fails identically with the change stashed.
• phpcs --standard=phpcs.xml.dist clean on both changed files.
• No schema change, so wp-api-generated.js needs no update.

Manual, worth trying if you're testing:

1. POST /wp/v2/media with a url plus title, caption, description and alt_text. Before this the fields are dropped; after it they're stored.
2. Hook rest_insert_attachment and wp_after_insert_post and post the same request. Neither fires before, both do after.
3. POST /wp/v2/media with both a file and a url. Before this the file is silently discarded; after it the request returns a 400.

## Types of changes

• Replace create_item_from_url() with upload_from_url(), returning wp_handle_sideload() data like the other upload handlers.
• Call it from insert_attachment() so the URL path shares the insert, hooks, fields and terms handling.
• Return a 400 when a request supplies both an uploaded file and a url.
• Check prepare_item_for_database() for a WP_Error in insert_attachment().
• Add tests for each.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Drafting the implementation and tests from the review notes on the PR. I reviewed and verified the behavior and test results myself.

Thanks for the ping — I've run out of time to test this today, but conceptually I like it! Makes sense to go through the shared path to me. I'll give it a closer look tomorrow if it's still open 👍

Thanks for the ping — I've run out of time to test this today, but conceptually I like it! Makes sense to go through the shared path to me. I'll give it a closer look tomorrow if it's still open 👍

If desired, this should be ok to land in RC, we will of course need a double review and to commit to both 7.2 and 7.1.

[khokansardar]

Patch testing report

Patch / PR tested

• ​https://github.com/WordPress/wordpress-develop/pull/12846
• Branch 748b30296e, merge-base with trunk 6904e896b8

Environment

WordPress: 7.1-beta4-62899-src
PHP: 8.2.18 (Docker)
MySQL: 8.0.36
OS: macOS 26.5.2
Browser: Chromium, desktop viewport
Local wordpress-develop @ ​http://localhost:8889

Steps

1. PHP-only patch, so states toggle with git checkout origin/trunk -- <controller> and back. No build.
2. Real HTTP POST /wp/v2/media from wp-admin with X-WP-Nonce, as administrator.
3. Sideload URL, verbatim in every case: https://s.w.org/images/home/screen-themes.png (image/png, 217831 bytes).
4. An mu-plugin records which insert hooks fire per request.
5. Same matrix run in both states and diffed, so every case is classified by fact.

Results

• url + title/caption/description/alt_text: pass (fixes) - trunk dropped all four (title screen-themes, caption/description/alt_text empty) -> Sideload title 65810 / Sideload caption 65810 / Sideload description 65810 / Sideload alt 65810
• insert hooks on the url path: pass (fixes) - rest_after_insert_attachment only -> rest_pre_insert_attachment, rest_insert_attachment, rest_after_insert_attachment, wp_after_insert_post, same four and same order as the multipart path
• multipart file + url: pass (fixes) - 201 with the file silently discarded -> 400 rest_invalid_param, no download attempted
• raw body + url: pass (fixes) - 201 with the body discarded -> 400 rest_invalid_param
• rest_pre_insert_attachment returning WP_Error: pass (fixes) - 201, filter never ran -> 400 carrying the filter's error code
• plain multipart upload with title + alt_text: pass (guard) - 201, fields applied, same four hooks both states
• url with a non-image extension: pass (guard) - 400 rest_invalid_url both states
• url + post (the editor's "Upload to Media Library" shape): pass (guard) - 201, attached, folder /2019/03/ from the parent's date both states
• PHPUnit WP_Test_REST_Attachments_Controller: 202 tests / 0 failures single site, 208 / 0 multisite, 2 skipped each
• Full restapi group: 3555 tests, 1 error in Test_oEmbed_Controller::test_proxy_with_classic_embed_provider, identical with both files reverted - pre-existing
• Source reverted with the new tests kept: all five new tests fail, so none is redundant
• phpcs and phpstan clean; wp-api-generated.js regenerates with no drift

Conclusion

PR #12846 makes a url the third file source alongside a multipart upload and a raw
body, so insert_attachment()e insert for every path. Allfour gaps are fixed and each is discriminating against unpatched trunk. The change is
minimal and idiomatic, with noated churn. The droppedupload_files check is genuinely unreachable, since create_item_permissions_check()
denies first. No released backtem_from_url()` was added in[62659] this cycle.

Two things worth correcting in the commit message.

1. Sideloading did not "pick up" the parent date or the exif defaults - media_handle_sideload() alre trunk a url attached to apost dated 2019-03 already landed in /2019/03/. What actually changes is th*page* parent no longer backdaent() excludes pages (trunk/2017/05/ -> patched /2026/08/), and an exif caption now lands in post_erather than post_content. Bohe uploaded-file path, but they are the opposite of what the description says.
2. The is_wp_error() check on prepare_item_for_database() also fixes the uploaded-file path, which has n returning a WP_Error fromrest_pre_insert_attachment on an ordinary upload previously produced a mangled attachment. In scope per the tc.

Recommend commit.

[adamsilverstein]

This wasn't quite ready so I am punting it to 7.2 or potentially a 7.1.1 release if we get reports of related issues.