Image: sideload external images on the server when uploading to the library#79409
Image: sideload external images on the server when uploading to the library#79409adamsilverstein wants to merge 13 commits into
Conversation
…ibrary Uploading an image inserted by URL to the media library read the remote image's bytes in the browser with window.fetch and posted the resulting blob. Under cross-origin isolation, which client-side media processing requires, that cross-origin fetch is blocked, so the upload could not complete. Accept a `url` parameter on the attachments create endpoint and sideload the remote image on the server instead, storing only the original file (no sub-sizes). The image block and the pre-publish external-media panel use this path through a new mediaSideloadFromUrl editor setting, so external uploads work regardless of cross-origin isolation. The now-unused client-side fetch helper is removed.
Cover the server-side external image sideload path that backs the cross-origin-isolation fallback: - PHP: exercise the `url` param branch on POST /wp/v2/media, asserting it sideloads the remote image, generates no sub-sizes when generate_sub_sizes is false, attaches to the parent post, propagates download errors, and registers the `url` arg on the creatable route. - JS: unit test mediaSideloadFromUrl for the request shape, attachment transform, post/wp_id resolution, and error handling.
adamsilverstein
added
the
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
github-actions
Bot
added
[Package] Editor
|
Size Change: -84 B (0%) Total Size: 7.49 MB 📦 View Changed
|
adamsilverstein
added
the
[Feature] Client Side Media
github-project-automation
Bot
moved this to 🔎 Needs Review
in WordPress 7.1 Editor Tasks
adamsilverstein
added
the
[Status] In Progress
The server-side URL sideload path is reached via create_item(), which the parent controller already gates on upload_files through create_item_permissions_check(). Add an explicit capability check at the top of create_item_from_url() so the path bails early and never downloads a remote file for a user who cannot upload media, independent of how it is invoked.
Address review feedback on the URL sideload path: - Bail with a 400 rest_invalid_url when the URL has no usable path (e.g. https://example.com/?img=123), so an empty filename is never handed to media_handle_sideload(). The filename is derived before downloading, so an unusable URL no longer triggers a wasted remote fetch. - Set the Location header on the 201 response, matching the REST convention the parent create_item() follows.
|
Flaky tests detected in 3c2213e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/28190662299
|
…erver-sideload # Conflicts: # packages/editor/CHANGELOG.md
adamsilverstein
requested review from
Mamaduka
and removed request for
Mamaduka and
spacedmonkey
adamsilverstein
requested review from
andrewserong,
ramonjd and
swissspidy
and removed request for
ajitbohra and
fabiankaegy
setAccessible() is deprecated since PHP 8.5 and has had no effect since PHP 8.1, where reflection can invoke protected methods without it. The PHP 8.5 unit test job treats the deprecation notice as an error, failing test_create_item_from_url_requires_upload_capability. Only call it on PHP < 8.1, matching the existing pattern in other phpunit tests.
|
This makes a lot of sense. Otherwise a bit wasteful to first download the image just to upload it again. Cut the middle man :) |
| /* | ||
| * When a URL is supplied instead of an uploaded file, sideload the | ||
| * remote image on the server. This avoids a cross-origin browser fetch, | ||
| * which fails under cross-origin isolation. The sub-size and scaling | ||
| * filters applied above still govern whether derivatives are generated. | ||
| */ | ||
| if ( ! empty( $request['url'] ) ) { | ||
| $response = $this->create_item_from_url( $request ); | ||
| } else { | ||
| $response = parent::create_item( $request ); | ||
| } |
There was a problem hiding this comment.
Also makes sense to me. Great idea.
My first thought was, "url appears a lot in media-related args, could we qualify it somehow", but I couldn't really come up with a better alternative other that target_url and the schema is self documenting anyway.
| mediaSideloadFromUrl: hasUploadPermissions | ||
| ? mediaSideloadFromUrl | ||
| : undefined, |
There was a problem hiding this comment.
We need to find a better way to share similar utilities.
Additionally, do we really need three methods for uploading the media? Can these (mediaSideload, mediaSideloadFromUrl) be part of mediaUpload?
Since implementation details aren't clear, maybe we should make access to these new media methods private.
There was a problem hiding this comment.
making them private for now.
For the record we have:
- mediaUpload — Browser-side upload. Takes a filesList, uploads each file and creates a new attachment per file. Fires onFileChange repeatedly (once per generated sub-size). The general-purpose path for user file uploads.
- mediaUploadOnSuccess — Callback hook invoked when an upload's attachments are ready; used by the client-side processing pipeline to react to finalized media.
- mediaSideload — Uploads a File (already in the browser) onto an existing attachment (attachmentId required). Creates no new attachment; returns sub-size data. This is a step inside client-side media processing: the browser generates derivatives, sideloads each onto the parent, then finalizes.
- mediaSideloadFromUrl (new in Image: sideload external images on the server when uploading to the library #79409) — Sends a url to the server, which downloads it and creates a new attachment via media_handle_sideload(). The browser never reads the bytes, so it works under cross-origin isolation. Fires onSuccess once with the attachment.
- mediaFinalize — Calls the finalize endpoint to assemble an attachment from the sub-sizes accumulated by mediaSideload, completing the client-side processing flow.
- mediaDelete — Deletes an attachment (cleanup, e.g. orphaned media when sub-size generation fails entirely).
Maybe we could rename mediaSideloadFromUrl to mediaLoadFromUrl so its less confusing?
The server-side sideload-from-URL path is still settling, so expose it through a private setting key (mirroring mediaUploadOnSuccessKey) rather than committing to a public block editor setting. Consumers read it via the unlocked key. Addresses review feedback on #79409.
…erver-sideload # Conflicts: # packages/block-library/src/image/image.js
This branch carries the server-side sideload PHP changes from #79409, so the backport-changelog check requires this PR's URL on the entry as well.
|
Stacked a CI-validation PR on top of this one to prove the external-image upload e2e coverage works once the server-side sideload is in place: #79605. It re-enables the CI is green there (all 8 Playwright shards + PHP), so this PR unblocks the last remaining skipped test from #79407. Once this merges (and the Chromium 148 upgrade #79495 lands), the one-line un-skip folds into trunk and the staging PR can be closed. |
4 participants
What
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.The Image block "Upload to Media Library" toolbar action and the pre-publish "External media" panel use this server-side path through a new
mediaSideloadFromUrlblock editor setting. The now-unused client-side fetch helper (media-util.js) is removed.Why
Fixes #79407.
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, see #79342). 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.How
lib/media/class-gutenberg-rest-attachments-controller.php: register aurlarg on the creatable route and route create requests with aurlthrough a newcreate_item_from_url()that downloads and sideloads on the server. Existing sub-size / scaling filters continue to govern derivative generation; the editor setting requestsgenerate_sub_sizes: false, storing only the original.packages/editor/src/utils/media-sideload-from-url/: new utility that POSTs the URL to the media endpoint and resolves the current post (withwp_idfallback for templates).packages/block-library/src/image/image.jsandpackages/editor/src/components/post-publish-panel/maybe-upload-media.js: use the new setting instead ofwindow.fetch()+mediaUpload().Testing Instructions
Automated tests
phpunit/media/class-gutenberg-rest-attachments-controller-test.phpcovers theurlbranch — sideload without sub-sizes, attachment parenting, download-error propagation, andurlarg registration.packages/editor/src/utils/media-sideload-from-url/test/index.jscovers the request shape, attachment transform, post/wp_idresolution, and error handling (6 tests, all passing).