#65517 closed defect (bug) (worksforme)
Sideload external images on the server via a `url` REST parameter
| Reported by: | adamsilverstein | Owned by: | adamsilverstein |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | REST API | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests has-test-info commit |
| Cc: | Focuses: |
Description (last modified by )
The attachments REST endpoint (POST /wp/v2/media) currently only creates an attachment from an uploaded file (the request body or a $_FILES entry). When the block editor uploads an externally-hosted image to the media library, it reads the remote image's bytes in the browser with window.fetch() and posts the resulting blob.
A browser cross-origin fetch is subject to CORS, so it fails for any host that does not send permissive headers, and the failure is silently swallowed. The "Upload to Media Library" toolbar action on an image inserted by URL, and the pre-publish "External media" panel, therefore fail for most external hosts.
This breaks entirely once the editor is cross-origin isolated, which client-side media processing requires (Document-Isolation-Policy: isolate-and-credentialless). In that mode the browser cannot read a cross-origin image's bytes at all.
The fix is to let the server fetch the URL, which is the same primitive that core's media_sideload_image() already relies on. Server-side fetching is not subject to browser CORS, so external uploads work regardless of cross-origin isolation.
Proposed change
Extend WP_REST_Attachments_Controller to accept an optional url parameter on the creatable route:
get_endpoint_args_for_item_schema()registers aurlarg (string,formaturi, sanitized withsanitize_url), alongside the existinggenerate_sub_sizesandconvert_formatclient-side media arguments.create_item()routes any request that supplies aurlthrough a newcreate_item_from_url()method, after the existing sub-size and scaling filters have been applied, so those filters continue to govern derivative generation.create_item_from_url():- requires the
upload_filescapability; - derives and validates a filename from the URL path before downloading anything, returning a
rest_invalid_url(400) error when the URL has no usable filename (for example a query-string-only URL); - downloads the remote file with
download_url(), which validates the host and blocks requests to private or local addresses; - sideloads the file with
media_handle_sideload(), cleaning up the temporary file if the sideload fails; - returns a 201 response with a
Locationheader pointing at the new attachment.
- requires the
No existing behavior changes when no url is supplied: the normal uploaded-file path is untouched.
Testing instructions
Automated:
npm run test:php -- --filter 'create_item_from_url' --group restapi npm run test:php -- --filter 'test_url_registered_as_creatable_arg' --group restapi
Six new tests in tests/phpunit/tests/rest-api/rest-attachments-controller.php cover: sideload without sub-sizes, attachment parenting via the post parameter, download-error propagation, rejection of a URL without a filename, the upload_files capability guard, and registration of the url argument. All pass locally (20 assertions).
Manual:
- Enable client-side media processing (so the editor is cross-origin isolated).
- Insert an Image block and paste a URL to an externally-hosted image.
- Select the block and click "Upload to Media Library" โ the image is added to the library and the block updates to the local copy.
- Alternatively, add an external image and open the pre-publish panel; the "External media" upload now succeeds.
References
- Core PR: โhttps://github.com/WordPress/wordpress-develop/pull/12268
- Gutenberg PR: โhttps://github.com/WordPress/gutenberg/pull/79409
- Gutenberg issue: โhttps://github.com/WordPress/gutenberg/issues/79407
Change History (33)
This ticket was mentioned in โPR #12268 on โWordPress/wordpress-develop by โ@adamsilverstein.
7 weeks ago
#2
#4
@
7 weeks ago
- Keywords has-test-info added; needs-testing removed
Patch testing report
Patch / PR tested
- โhttps://github.com/WordPress/wordpress-develop/pull/12268 (branch backport/79409-external-image-sideload @ 93c911e3ca)
- trunk @ fb76cccf79 with PR applied
Environment
WordPress: 7.1-alpha-62161-src
PHP: 8.2.18 (Docker)
MySQL: 8.0.36
OS: macOS 26.5.1
Browser/Client: server-side REST (PHPUnit + live wp eval dispatch)
Local wordpress-develop @ โhttp://localhost:8889
Steps
- Checked out PR #12268 and ran the new PHPUnit tests.
- Ran the full WP_Test_REST_Attachments_Controller class to check for regressions.
- Dispatched a live POST /wp/v2/media with a real external image URL.
- Exercised the guard paths live: filename-less URL, private address, and a subscriber.
Results
- New tests: pass โ
create_item_from_urlfilter group is 7 tests / 20 assertions, all green. - Arg registration: pass โ
test_url_registered_as_creatable_arg(1 test, 5 assertions). - Full controller class: pass โ 137 tests, 2 skipped, 0 failures (after clearing a stale upload).
- Pre-existing failure:
test_sideload_scaled_unique_filenamefails identically on clean trunk (stalecanola-scaled.jpgin uploads forces a-1suffix). Not caused by this PR; goes green once the stale file is removed. - Live sideload: pass โ POST with
urlof a real external PNG returned 201, stored locally under /uploads, media_type=image, sub-sizes generated (thumbnail, medium, medium_large). - Filename-less URL (
?img=123): pass โ 400rest_invalid_url, no download attempted. - Private address (
http://127.0.0.1/...): pass โ 500http_request_failed;download_url()blocks the local request. - Capability guard (subscriber): pass โ 403
rest_cannot_create.
Note
The Gutenberg editor UI flow (paste image URL โ "Upload to Media Library") was not exercised; it requires the companion Gutenberg PR #79409 and cross-origin isolation, which are not present in stock wordpress-develop. The server-side endpoint the PR changes was tested directly and end-to-end instead.
Conclusion
PR #12268 lets the server sideload an external image via an optional url REST parameter, avoiding the browser CORS fetch that fails uThe change is minimal and scoped tothe ticket: a new optional url arg plus a create_item_from_url() method, with the existing uploaded-file path untouched when ned signatures, hook params, or return shapes โ no backward-compatibility concern. Filename derivation, the upload_files capability check, and download_url()'s privae as designed, and thesub-size/scaling filters continue to govern derivative generation on the URL path. Recommend commit.
โ@adamsilverstein commented on โPR #12268:
5 weeks ago
#6
Synced this backport with the final merged state of WordPress/gutenberg#79409, which gained two changes during review:
- Reject URLs whose extension does not map to an image MIME type before downloading, so files that can never be accepted (such as PHP scripts) are not fetched at all.
- Re-apply
rest_validate_request_arg()in theurlvalidate_callback, since a custom callback replaces the default and would otherwise silently drop the schema's string type and uri format checks.
The corresponding tests are ported as well (non-image extension data provider and non-string url rejection). All 15 url-sideload tests pass locally and PHPCS is clean.
#7
@
5 weeks ago
- Keywords commit added
This one was merged in Gutenberg, the PR is approved and is ready for commit for core.
This ticket was mentioned in โPR #12670 on โWordPress/wordpress-develop by โ@andrewserong.
3 weeks ago
#10
In WP 7.1, the /wp/v2/media REST API endpoint supports a url param to allow sideloading media via a url, without requiring the browser to first download an image and upload via a standard media upload POST request.
This PR adds a call to check_upload_size for this upload path, to create parity with the other upload paths (e.g. the multipart and raw-body upload paths).
To test manually that this doesn't regress the "upload to media library" feature, try adding an Image block to a post using an external url and click the "upload to media library button". It should work as on trunk. Here's what that button looks like:
And here's some test markup for you to try that out with:
<figure class="wp-block-image size-large">[[Image(https://user-images.githubusercontent.com/1204802/100067796-fc3e8700-2e36-11eb-993b-6b80b4310b87.png)]]</figure>
Trac ticket: https://core.trac.wordpress.org/ticket/65517
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 4.8
Used for: Identifying the gap in upload paths; final implementation and tests were reviewed and edited by me.
โ@andrewserong commented on โPR #12670:
3 weeks ago
#12
Should we backport to Gutenberg?
Indeed we should! Backport here: โhttps://github.com/WordPress/gutenberg/pull/80659
#14
@
2 weeks ago
- Resolution fixed
- Status closed โ reopened
I've been reviewing the changes from this ticket and I have a clarifying question.
Is it intentional that url, convert_format, and generate_sub_sizes (i.e. sideloading via URL) work on /wp/v2/media even when not registered in schema (via the wp_is_client_side_media_processing_enabled() check)?
In the current state, all of these parameters are processed by the endpoint, even when wp_is_client_side_media_processing_enabled() is false, but may have some slightly different behaviors based on the combination of checks/non-checks.
I think there are a couple tweaks that can be made in either direction to improve what happens vs what is expected.
Happy to work up a PR with that clarification. :)
#15
@
2 weeks ago
I think there are a couple tweaks that can be made in either direction to improve what happens vs what is expected.
@andrewserong @ramonopoly @adamsilverstein, Regarding this point, is there anything that can be addressed in the 7.1 release? If not, I would like to punt this ticket to 7.2.
#16
@
2 weeks ago
If not, I would like to punt this ticket to 7.2.
I might need to defer to @adamsilverstein on this one, but my take is that we can likely close this ticket / punt to 7.2 for enhancements.
That said, if @jeremyfelt has PRs to share that should make it in for 7.1, we can re-open! (I.e. I know some of the nuance of this endpoint will likely need a little more polishing).
#17
@
12 days ago
Some runtime data from testing 7.1-beta4, in case it helps with the 7.1 vs 7.2 decision above.
[62841] behaves as described on multisite. With fileupload_maxk set to 1:
multisite 512 bytes = accepted multisite 2048 bytes = rest_upload_file_too_big
Two observations on the current state of this path. Neither contradicts the commit's stated scope, but both seemed worth recording while the ticket is open:
- check_upload_size() returns early when ! is_multisite(), so on single-site there is no size ceiling on this path. upload_max_filesize and post_max_size govern request bodies rather than a server-side fetch, so nothing else bounds it:
single-site 512 bytes = accepted single-site 2048 bytes = accepted (same 1 KB setting)
That looks intentional, since [62841] is explicitly scoped to multisite limits. Flagging it only because the practical effect is that the most common configuration has no ceiling on this path.
- The check runs after download_url() has finished writing the temporary file, so the bytes are already on disk by the time the size decision happens. Bounding the transfer itself with a maximum response size, rather than inspecting the completed file, would cover both cases and avoid fetching something that is going to be rejected.
Tested on 7.1-beta4, PHP 8.2 single-site and PHP 7.4 multisite, calling WP_REST_Attachments_Controller::check_upload_size() directly with control and oversized files.
#18
@
12 days ago
Good notes and testing @courane01! Let's leave this ticket open for 7.1 and I can dig into this a little more tomorrow. I think that one sounds worth fixing for 7.1.
This ticket was mentioned in โSlack in #core by adrianduffell. โView the logs.
8 days ago
#20
@
8 days ago
Let's leave this ticket open for 7.1 and I can dig into this a little more tomorrow. I think that one sounds worth fixing for 7.1.
@andrewserong How did you go with this? Do you think it's still a chance to make it in 7.1?
This ticket was mentioned in โPR #12825 on โWordPress/wordpress-develop by โ@adamsilverstein.
7 days ago
#21
Follow up to [62841], addressing the two observations @courane01 made while testing 7.1-beta4 in https://core.trac.wordpress.org/ticket/65517#comment:17.
check_upload_size() returns early when ! is_multisite(), so on single site - the common case - the URL sideload path has no size ceiling at all. upload_max_filesize and post_max_size bound a request body, not a fetch the server makes itself, so nothing else was stopping a url parameter from pulling in a file of any size. This applies wp_max_upload_size() on that path, so a URL can't bring in a file larger than the same site would accept as a direct upload.
The second half is that the check ran after download_url() had already streamed the whole file to disk. The limit is now also passed to the request as limit_response_size, which stops the transfer once it's passed, so an oversized remote file never lands on disk in full. One byte over the ceiling is enough to fail the size check, so the file is still rejected. Compression is already disabled for streamed requests, so this doesn't change how the response is decoded.
The multisite checks are untouched and still run first, so rest_upload_file_too_big and rest_upload_limited_space are returned as before for the network file size limit and the site space quota. Sites that want a different ceiling can use the existing upload_size_limit filter.
## How has this been tested
Automated:
npm run test:php -- --filter 'create_item_from_url' --group restapi npm run test:php -- -c tests/phpunit/multisite.xml --filter 'create_item_from_url' --group restapi
- Two new tests: one that a file over the limit is rejected on single site with
rest_upload_file_too_big, and one that the download request is capped atwp_max_upload_size() + 1. Both fail on trunk without the change - I checked by stashing the source change and re-running. - Existing sideload tests: 15 pass on single site, 17 on multisite, including the two multisite tests from [62841].
- Full
WP_Test_REST_Attachments_Controllerclass: 190 tests, 2 skipped, 0 failures. phpcs --standard=phpcs.xml.distclean on both changed files.
## Types of changes
- Apply
wp_max_upload_size()as a ceiling on the URL sideload path, so it applies on single site as well as multisite. - Pass the ceiling to the download as
limit_response_sizeso the transfer is bounded rather than inspected after the fact. - Add tests for both.
## Open questions
- Is
wp_max_upload_size()the right ceiling here? It's derived from the PHP directives that govern request bodies, which don't really apply to a server-side fetch. The argument for it is parity: sideloading shouldn't accept a file the same user couldn't upload directly, and it's the number the media library already shows people. If we'd rather hosts be able to allow larger server-side fetches specifically, a dedicated filter would do it, though I'd lean toward not adding new API this late in 7.1. limit_response_sizetruncates rather than erroring, which is why the ceiling is set one byte high and the size check still does the rejecting. That works, but it does mean the transport's truncation behavior is load-bearing. Happy to drop it and keep only the post-download check if that feels too clever for a late-cycle fix.
## 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 ticket. I reviewed and verified the behavior and test results myself.
โ@andrewserong commented on โPR #12825:
7 days ago
#22
Thanks for putting this up, I'd started to look at it and this is pretty much exactly where I'd landed, too ๐
Is wp_max_upload_size() the right ceiling here?
In my view, yes. The behaviour for url in this endpoint is effectively getting the server to perform an upload on behalf of the user, so I think making it behave as much as possible (i.e. with the same restrictions) as a user generated upload is the right call for 7.1.
limit_response_size truncates rather than erroring, which is why the ceiling is set one byte high and the size check still does the rejecting.
This too seems good to me, as it prevents the temp file from growing beyond the threshold, and it's a fairly simple change code-wise.
This ticket was mentioned in โPR #12833 on โWordPress/wordpress-develop by โ@adamsilverstein.
7 days ago
#23
Addresses @jeremyfelt's question in https://core.trac.wordpress.org/ticket/65517#comment:14 about url, generate_sub_sizes, and convert_format being processed on POST /wp/v2/media even when they aren't registered in the schema.
They needed fixing in opposite directions, which is what this does.
url should work either way. Sideloading an external image works around a cross-origin fetch the browser can't make, and that fails whether or not client side media processing is enabled - cross-origin isolation makes it necessary, not conditional. As far as I can tell the feature doesn't depend on client side media processing at all, so the argument is now registered unconditionally.
Worth being clear that leaving it unregistered was never disabling it. create_item() reads the parameter either way, so the sideload already ran on sites without the feature, just without the sanitize_url and wp_http_validate_url() callbacks the registered argument carries. An unsafe URL came back as a bare http_request_failed instead of a 400. Registering it always is what actually closes that.
generate_sub_sizes and convert_format go the other way. They hand image processing to the client, and the /media/<id>/sideload route the client uploads the results to is only registered when the feature is on. Honoring them otherwise leaves an attachment with no sub-sizes and no way to add them, and generate_sub_sizes of false also relaxes the unsupported image type check in create_item_permissions_check() on a site that never opted in. Both are now ignored unless client side media processing is enabled.
## How has this been tested
npm run test:php -- --filter 'WP_Test_REST_Attachments_Controller' --group restapi
- Four new tests with client side media processing disabled:
urlis registered while the other two aren't, sideloading works, an unsafe URL is rejected with a 400, andgenerate_sub_sizesoffalseis ignored so sub-sizes are still generated. Three of the four fail on trunk without the change - I checked by stashing the source change and re-running. The fourth is a regression guard, since the sideload already worked on trunk, just unvalidated. - Full
WP_Test_REST_Attachments_Controllerclass: 192 tests, 2 skipped, 0 failures. phpcs --standard=phpcs.xml.distclean on both changed files.wp-api-generated.jsneeds no update. The fixture is generated with client side media processing enabled, and in that configuration the registered arguments are unchanged.
## Types of changes
- Register the
urlargument unconditionally inget_endpoint_args_for_item_schema(). - Ignore
generate_sub_sizesandconvert_formatincreate_item()andcreate_item_permissions_check()unless client side media processing is enabled. - Add tests for both, and a
disable_client_side_media_processing()test helper.
## Note on an existing test
test_upload_unsupported_image_type_skipped_when_not_generating_sub_sizes (from #64836) called the permissions check with generate_sub_sizes of false without enabling client side media processing, so it was asserting exactly the behavior this changes. It now enables the feature first, which keeps its original intent - the comment on it already says "when the client handles image processing" - but flagging it since it's an existing test from another ticket. @jeremyfelt @andrewserong does that read right to you?
## 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 ticket. I reviewed and verified the behavior and test results myself.
โ@andrewserong commented on โPR #12833:
7 days ago
#24
Ignore generate_sub_sizes and convert_format in create_item() and create_item_permissions_check() unless client side media processing is enabled.
the comment on it already says "when the client handles image processing" - but flagging it since it's an existing test from another ticket. @jeremyfelt @andrewserong does that read right to you?
I'm a little on the fence about this one. In terms of the REST API itself, for the behaviours of switching off conversion or generating sub-sizes, do we care if client-side media processing is enabled? Is whether client-side media processing is enabled orthogonal to the behaviour here?
I don't mind too much either way, I'm mostly thinking about how to keep things simple.
โ@adamsilverstein commented on โPR #12825:
7 days ago
#25
U updated the errant doc block, this is "good to go".
โ@jeremyfelt commented on โPR #12833:
6 days ago
#26
@adamsilverstein @andrewserong ๐๐ป Sorry for the late flag, I'm still wrapping my head around bits here. Here's my understanding of the paths that we're adding in 7.1.
First, there is a new "Upload to Media Library" button in the editor that appears when you have image block markup like this:
<figure class="wp-block-image size-large">[[Image(https://example.test/external/1234.jpg)]]</figure>
This submits a payload to the wp/v2/media endpoint with post and url attributes. It does not attempt to do anything with client side media processing.
This can also be fired as part of a pre-publish check suggesting that external media be uploaded.
Second, when a supported image is uploaded by the user in a supported browser and true === wp_is_client_side_media_processing_enabled(), the resizing is handled in the browser and associated with multiple requests:
- One request to the
wp/v2/mediaendpoint withfile(image binary),post(post ID), andgenerate_sub_sizes(false) attributes. - Multiple requests to the
wp/v2/media/<id>/sideloadendpoint withfile(image binary),image_size, andconvert_format(false) attributes. - One request to the
wp/v2/media/<id>/finalizeendpoint withsub_sizesas an array of data about the generated image sizes.
And then third, as a side effect of adding those features, the ability for extenders to make calls to these endpoints:
wp/v2/mediawith theurlattribute (andgenerate_sub_sizesif client side processing enabled)wp/v2/media/<id>/sideload(if client side processing enabled)wp/v2/media/<id>/finalize(if client side processing enabled)
Conclusions
It seems like the WordPress core editor side of things is working as expected, but I think there are a handful of other things that can be cleaned up or clarified before folks discover the endpoints and start using them. It will be harder to correct in the future and maintain back-compat.
- Return an error if both
fileandurlattributes are sent towp/v2/mediaat the same time. - Ensure fields like
title,alt_text,caption, etc.... work when aurlattribute is provided. - Fire
rest_pre_insert_attachmentandrest_insert_attachmentfor requests whereurlattribute is provided. (And add a check foris_wp_error()on the pre-hook.) - Call
wp_after_insert_post()to firewp_after_insert_post. (A comment increate_item_from_url()says it's handled, but attachments are returned too early inwp_insert_post())
Items 2, 3, and 4 come from a lack of parity between create_item() and create_item_from_url(). It may be helpful to merge those methodsโor rather than return early, continue with create_item() after create_item_from_url() handles the initial sideload.
โ@adamsilverstein commented on โPR #12833:
6 days ago
#27
Items 2, 3, and 4 come from a lack of parity between create_item() and create_item_from_url(). It may be helpful to merge those methodsโor rather than return early, continue with create_item() after create_item_from_url() handles the initial sideload.
That makes sense, we would want the usual create_item pathway to fire as usual, as if the user had upload the file directly.
โ@andrewserong commented on โPR #12825:
6 days ago
#30
Looks like this has been committed as of โhttps://github.com/WordPress/wordpress-develop/commit/5f5d96bd4b0ad25b803dc507c08b5336fff926ed. Good to close this one out now?
โ@adamsilverstein commented on โPR #12825:
6 days ago
#31
Yep, closing - fixed in https://core.trac.wordpress.org/changeset/63015
#32
@
6 days ago
- Milestone 7.1
- Resolution โ worksforme
- Status reopened โ closed
Closing this out as fixed by https://core.trac.wordpress.org/changeset/63015
Follow up for the remaining review item in https://core.trac.wordpress.org/ticket/65808
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
## Description
Extends the attachments REST endpoint (
POST /wp/v2/media) to accept an optionalurlparameter. When present, the server downloads the remote image withdownload_url()and sideloads it withmedia_handle_sideload(), instead of the browser fetching the bytes and posting a blob.When a user inserts an image by URL and uploads it to the media library, the editor previously read the remote image's bytes in the browser with
window.fetch()and posted the resulting blob. A browser cross-origin fetch is subject to CORS, so it fails for any host that does not send permissive headers, and the failure is silently swallowed.This breaks entirely once the editor is cross-origin isolated, which client-side media processing requires (
Document-Isolation-Policy: isolate-and-credentialless). Letting the server fetch the URL โ the same primitive behind core'smedia_sideload_image()โ avoids browser CORS entirely, so external uploads work regardless of isolation.## Approach
WP_REST_Attachments_Controller::get_endpoint_args_for_item_schema(): register aurlarg on the creatable route (alongside the existinggenerate_sub_sizes/convert_formatclient-side media args).WP_REST_Attachments_Controller::create_item(): when aurlis present, route the request through a newcreate_item_from_url()that downloads and sideloads on the server. The existing sub-size / scaling filters continue to govern derivative generation.create_item_from_url(): requires theupload_filescapability, derives and validates a filename from the URL path before downloading, downloads withdownload_url()(which validates the host and blocks private/local addresses), sideloads withmedia_handle_sideload(), and returns a 201 with aLocationheader.## Testing Instructions
Automated:
Six new tests in
tests/phpunit/tests/rest-api/rest-attachments-controller.phpcover theurlbranch: sideload without sub-sizes, attachment parenting, download-error propagation, filename-less URL rejection, theupload_filescapability guard, andurlarg registration. All pass locally (20 assertions).Manual:
## Trac ticket
This is a backport of the PHP changes from Gutenberg PR โhttps://github.com/WordPress/gutenberg/pull/79409 (fixes โhttps://github.com/WordPress/gutenberg/issues/79407).