Make WordPress Core

Opened 7 weeks ago

Closed 6 days ago

Last modified 6 days ago

#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 adamsilverstein)

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 a url arg (string, format uri, sanitized with sanitize_url), alongside the existing generate_sub_sizes and convert_format client-side media arguments.
  • create_item() routes any request that supplies a url through a new create_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_files capability;
    • 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 Location header pointing at the new attachment.

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:

  1. Enable client-side media processing (so the editor is cross-origin isolated).
  2. Insert an Image block and paste a URL to an externally-hosted image.
  3. Select the block and click "Upload to Media Library" โ€” the image is added to the library and the block updates to the local copy.
  4. Alternatively, add an external image and open the pre-publish panel; the "External media" upload now succeeds.

References

Change History (33)

#1 @adamsilverstein
7 weeks ago

  • Owner set to adamsilverstein
  • Status new โ†’ assigned

This ticket was mentioned in โ€‹PR #12268 on โ€‹WordPress/wordpress-develop by โ€‹@adamsilverstein.


7 weeks ago
#2

## Description

Extends the attachments REST endpoint (POST /wp/v2/media) to accept an optional url parameter. When present, the server downloads the remote image with download_url() and sideloads it with media_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's media_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 a url arg on the creatable route (alongside the existing generate_sub_sizes / convert_format client-side media args).
  • WP_REST_Attachments_Controller::create_item(): when a url is present, route the request through a new create_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 the upload_files capability, derives and validates a filename from the URL path before downloading, downloads with download_url() (which validates the host and blocks private/local addresses), sideloads with media_handle_sideload(), and returns a 201 with a Location header.

## 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 the url branch: sideload without sub-sizes, attachment parenting, download-error propagation, filename-less URL rejection, the upload_files capability guard, and url arg registration. All pass locally (20 assertions).

Manual:

  1. Enable client-side media processing (so the editor is cross-origin isolated).
  2. Insert an Image block and paste a URL to an externally hosted image.
  3. Select the block and click "Upload to Media Library" โ€” the image is added to the library and the block updates to the local copy.

## 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).

#3 @adamsilverstein
7 weeks ago

  • Keywords needs-testing added

#4 @khokansardar
7 weeks ago

  • Keywords has-test-info added; needs-testing removed

Patch testing report

Patch / PR tested

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

  1. Checked out PR #12268 and ran the new PHPUnit tests.
  2. Ran the full WP_Test_REST_Attachments_Controller class to check for regressions.
  3. Dispatched a live POST /wp/v2/media with a real external image URL.
  4. Exercised the guard paths live: filename-less URL, private address, and a subscriber.

Results

  • New tests: pass โ€” create_item_from_url filter 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_filename fails identically on clean trunk (stale canola-scaled.jpg in uploads forces a -1 suffix). Not caused by this PR; goes green once the stale file is removed.
  • Live sideload: pass โ€” POST with url of 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 โ€” 400 rest_invalid_url, no download attempted.
  • Private address (http://127.0.0.1/...): pass โ€” 500 http_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.

#5 @adamsilverstein
7 weeks ago

#65528 was marked as a duplicate.

โ€‹@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 the url validate_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 @adamsilverstein
5 weeks ago

  • Keywords commit added

This one was merged in Gutenberg, the PR is approved and is ready for commit for core.

#8 @adamsilverstein
5 weeks ago

  • Resolution โ†’ fixed
  • Status assigned โ†’ closed

In 62659:

REST API: Sideload external images via a url parameter.

Extend the attachments endpoint (POST /wp/v2/media) to accept an optional url parameter. When a request supplies a url, WP_REST_Attachments_Controller::create_item() routes it through a new create_item_from_url() method that downloads the remote file with download_url() and stores it with media_handle_sideload(), rather than the browser fetching the bytes and posting a blob. The existing uploaded-file path is unchanged when no url is supplied, and the sub-size and scaling filters continue to govern derivative generation.

See related Gutenberg pull request: โ€‹https://github.com/WordPress/gutenberg/pull/79409 and issue: โ€‹https://github.com/WordPress/gutenberg/issues/79407.

Props swissspidy, khokansardar, westonruter.
Fixes #65517.

#9 @sabernhardt
4 weeks ago

  • Milestone Awaiting Review โ†’ 7.1

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:

https://github.com/user-attachments/assets/efd44446-8b14-4561-9827-b4b17aa1dc4b

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.

#11 @andrewserong
3 weeks ago

  • Resolution fixed
  • Status closed โ†’ reopened

โ€‹@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

#13 @andrewserong
3 weeks ago

  • Resolution โ†’ fixed
  • Status reopened โ†’ closed

In 62841:

REST API: Enforce multisite upload limits when sideloading media from a URL.

The attachments controller's URL-based creation path, create_item_from_url(), passed the downloaded file to media_handle_sideload() without running check_upload_size(). Unlike the multipart and raw-body upload paths, it did not enforce the multisite maximum file size or the site's upload space quota.

Run check_upload_size() on the downloaded file before sideloading it, for parity with the other upload paths, and remove the temporary file when the check fails.

Developed in: โ€‹https://github.com/WordPress/wordpress-develop/pull/12670

Follow-up to [62659].

Props andrewserong, ramonopoly.

Fixes #65517.

#14 @jeremyfelt
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 @wildworks
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 @andrewserong
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 @courane01
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:

  1. 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.

  1. 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 @andrewserong
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 @adrianduffell
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 at wp_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_Controller class: 190 tests, 2 skipped, 0 failures.
  • phpcs --standard=phpcs.xml.dist clean 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_size so 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_size truncates 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: url is registered while the other two aren't, sideloading works, an unsafe URL is rejected with a 400, and generate_sub_sizes of false is 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_Controller class: 192 tests, 2 skipped, 0 failures.
  • phpcs --standard=phpcs.xml.dist clean on both changed files.
  • wp-api-generated.js needs 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 url argument unconditionally in get_endpoint_args_for_item_schema().
  • Ignore generate_sub_sizes and convert_format in create_item() and create_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:

  1. One request to the wp/v2/media endpoint with file (image binary), post (post ID), and generate_sub_sizes (false) attributes.
  2. Multiple requests to the wp/v2/media/<id>/sideload endpoint with file (image binary), image_size, and convert_format (false) attributes.
  3. One request to the wp/v2/media/<id>/finalize endpoint with sub_sizes as 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:

  1. wp/v2/media with the url attribute (and generate_sub_sizes if client side processing enabled)
  2. wp/v2/media/<id>/sideload (if client side processing enabled)
  3. 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.

  1. Return an error if both file and url attributes are sent to wp/v2/media at the same time.
  2. Ensure fields like title, alt_text, caption, etc.... work when a url attribute is provided.
  3. Fire rest_pre_insert_attachment and rest_insert_attachment for requests where url attribute is provided. (And add a check for is_wp_error() on the pre-hook.)
  4. Call wp_after_insert_post() to fire wp_after_insert_post. (A comment in create_item_from_url() says it's handled, but attachments are returned too early in wp_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.

#28 @adamsilverstein
6 days ago

  • Description modified (diff)

#29 @adamsilverstein
6 days ago

In 63015:

REST API: Bound the size of media sideloaded from a URL.

Ensure upload limits are honored when fetching sideloaded image from URL.

WP_REST_Attachments_Controller::create_item_from_url() only ran check_upload_size(), which returns early when ! is_multisite(), so a single site had no ceiling at all on this path: upload_max_filesize and post_max_size bound a request body, not a fetch the server makes itself. Apply wp_max_upload_size() to the download, so a URL cannot bring in a file larger than the same site would accept as a direct upload, and pass that limit to the request as limit_response_size so an oversized file is not written to disk in full before being rejected. The multisite checks are unchanged and still run first, and no ceiling is applied when wp_max_upload_size() returns 0.

Follow-up to [62659], [62841].

Props andrewserong, courane01.
See #65517.

โ€‹@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?

#32 @adamsilverstein
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

#33 @adamsilverstein
6 days ago

In 63018:

REST API: Always register the media creation arguments.

The url, generate_sub_sizes, and convert_format arguments for POST /wp/v2/media were only registered when client side media processing is enabled, but create_item() and create_item_permissions_check() honored all three either way. Since an unregistered argument skips the validation and sanitization its registration carries, an unsafe sideload url failed with a bare http_request_failed instead of a 400. Gating registration also made the schema depend on request context rather than site configuration, since wp_is_client_side_media_processing_enabled() is derived from is_ssl() and the host, so the same site could advertise different arguments depending on how it was reached.

All three arguments are now registered unconditionally. None of them require the feature: sideloading from a URL works around a cross-origin fetch the browser cannot make, and skipping sub-size generation or format conversion is something the server can do on its own. One condition is kept: generate_sub_sizes of false no longer relaxes the unsupported image type check in create_item_permissions_check() unless client side media processing is enabled, since that check exists because the server cannot process the image and should only be relaxed when the client can. Behavior with client side media processing enabled is unchanged.

Follow-up to [62659], [62841].

Props andrewserong, jeremyfelt.
Fixes #65808. See #65517.

Note: See TracTickets for help on using tickets.