From 4a954db4e32d2bed797ae1b7a51d3e51b2a1eda9 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 3 Aug 2026 17:04:40 -0700 Subject: [PATCH 1/5] REST API: Bound the size of media sideloaded from a URL. The attachments controller's URL creation path, `create_item_from_url()`, only ran `check_upload_size()`, which returns early when `! is_multisite()`. A single site therefore 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, so any URL could pull a file of any size into the media library. 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. The limit is passed to the HTTP request as `limit_response_size`, which stops the transfer once it is passed, so an oversized remote file is no longer written to disk in full before being rejected. Sites that need a different ceiling can adjust it with the existing `upload_size_limit` filter. The multisite checks are unchanged and still run first, so `rest_upload_file_too_big` and `rest_upload_limited_space` continue to be returned for the network file size limit and the site space quota. No limit is applied when `wp_max_upload_size()` cannot determine a size. Follow-up to [62659], [62841]. See #65517. --- .../class-wp-rest-attachments-controller.php | 43 +++++++++ .../rest-api/rest-attachments-controller.php | 92 +++++++++++++++++++ 2 files changed, 135 insertions(+) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php index 6e06f1563c50c..8f4d48ab4703e 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php @@ -626,12 +626,42 @@ protected function create_item_from_url( $request ) { ); } + /* + * Cap the download at the same size the site would accept as a direct + * upload. check_upload_size() only applies on multisite, so without a + * ceiling here a single site has no limit at all on this path: the + * `upload_max_filesize` and `post_max_size` directives bound a request + * body, not a fetch the server makes itself. + * + * A limit of 0 means the size could not be determined, in which case + * no ceiling is applied. + */ + $max_size = (int) wp_max_upload_size(); + /* * Download the remote file with WordPress's HTTP API, which validates * the host and blocks requests to private or local addresses. This is * the same primitive core's media_sideload_image() relies on. + * + * `limit_response_size` stops the transfer once the limit is passed, + * so an oversized remote file is never written to disk in full. One + * byte over the ceiling is enough to fail the size check below. */ + $limit_response_size = static function ( $args ) use ( $max_size ) { + $args['limit_response_size'] = $max_size + 1; + return $args; + }; + + if ( $max_size > 0 ) { + add_filter( 'http_request_args', $limit_response_size ); + } + $tmp_file = download_url( $url ); + + if ( $max_size > 0 ) { + remove_filter( 'http_request_args', $limit_response_size ); + } + if ( is_wp_error( $tmp_file ) ) { return $tmp_file; } @@ -649,6 +679,19 @@ protected function create_item_from_url( $request ) { return $size_check; } + if ( $max_size > 0 && wp_filesize( $tmp_file ) > $max_size ) { + if ( file_exists( $tmp_file ) ) { + wp_delete_file( $tmp_file ); + } + + return new WP_Error( + 'rest_upload_file_too_big', + /* translators: %s: Maximum allowed file size in kilobytes. */ + sprintf( __( 'This file is too big. Files must be less than %s KB in size.' ), number_format( $max_size / KB_IN_BYTES ) ), + array( 'status' => 400 ) + ); + } + $attachment_id = media_handle_sideload( $file_array, $post_id ); if ( is_wp_error( $attachment_id ) ) { diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 90899df850d47..ab7aa34c785f6 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -5394,6 +5394,98 @@ public function test_create_item_from_url_exceeds_multisite_site_upload_space() $this->assertErrorResponse( 'rest_upload_limited_space', $response, 400 ); } + /** + * Verifies that the URL sideload path enforces the site's maximum upload + * size on single site as well as multisite. + * + * check_upload_size() returns early when ! is_multisite(), so before this + * check a single site had no ceiling at all on this path. + * + * @ticket 65517 + * + * @covers WP_REST_Attachments_Controller::create_item_from_url + */ + public function test_create_item_from_url_exceeds_max_upload_size() { + $this->enable_client_side_media_processing(); + + wp_set_current_user( self::$superadmin_id ); + + // The fixture the download is mocked with is comfortably larger than this. + add_filter( 'upload_size_limit', array( $this, 'filter_small_upload_size_limit' ), 20 ); + add_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10, 3 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/too-big.jpg' ); + $request->set_param( 'generate_sub_sizes', false ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10 ); + + $this->assertErrorResponse( 'rest_upload_file_too_big', $response, 400 ); + } + + /** + * Verifies that the download itself is bounded, so an oversized remote file + * is not written to disk in full before the size check rejects it. + * + * @ticket 65517 + * + * @covers WP_REST_Attachments_Controller::create_item_from_url + */ + public function test_create_item_from_url_limits_the_download_size() { + $this->enable_client_side_media_processing(); + + wp_set_current_user( self::$superadmin_id ); + + $request_args = null; + + $capture_args = static function ( $response, $args, $url ) use ( &$request_args ) { + $request_args = $args; + + if ( ! empty( $args['filename'] ) ) { + copy( DIR_TESTDATA . '/images/canola.jpg', $args['filename'] ); + } + + return array( + 'response' => array( + 'code' => 200, + 'message' => 'OK', + ), + 'headers' => array(), + 'cookies' => array(), + 'body' => '', + ); + }; + + add_filter( 'pre_http_request', $capture_args, 10, 3 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/photo.jpg' ); + $request->set_param( 'generate_sub_sizes', false ); + + rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', $capture_args, 10 ); + + $this->assertIsArray( $request_args, 'The download request should have been made.' ); + $this->assertSame( + (int) wp_max_upload_size() + 1, + $request_args['limit_response_size'], + 'The download should be capped one byte past the maximum upload size.' + ); + } + + /** + * Filters the maximum upload size down to a value smaller than the image + * fixture used to mock the download. + * + * @return int A deliberately small upload size limit, in bytes. + */ + public function filter_small_upload_size_limit() { + return 1024; + } + /** * Verifies that a URL with no usable path bails with a 400 before any * download is attempted, rather than handing an empty filename to the From afca1e99711e802ebef205b11fefb7b9d0302125 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 4 Aug 2026 00:18:08 -0700 Subject: [PATCH 2/5] REST API: Always register the media `url` argument. Sideloading an external image with the `url` parameter on `POST /wp/v2/media` works around a cross-origin fetch the browser cannot make. That fetch fails regardless of whether client-side media processing is enabled, so the argument was registered too narrowly. Leaving the argument unregistered did not disable it. `create_item()` reads the parameter either way, so on a site without client-side media processing the sideload still ran, but without the `sanitize_url` and `wp_http_validate_url()` callbacks the registered argument carries, and an unsafe URL returned a bare `http_request_failed` rather than a 400. Register `url` unconditionally so its validation always applies. The `generate_sub_sizes` and `convert_format` parameters are different: they hand image processing to the client, and the route the client uploads the results to is only registered when the feature is enabled. Honoring them otherwise would leave an attachment with no sub-sizes and no way to add them, and would relax 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. Follow-up to [62659]. See #65517. --- .../class-wp-rest-attachments-controller.php | 100 ++++++++----- .../rest-api/rest-attachments-controller.php | 139 ++++++++++++++++++ 2 files changed, 202 insertions(+), 37 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php index 6e06f1563c50c..b32184f763df7 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php @@ -237,7 +237,11 @@ public function register_routes() { public function get_endpoint_args_for_item_schema( $method = WP_REST_Server::CREATABLE ) { $args = parent::get_endpoint_args_for_item_schema( $method ); - if ( WP_REST_Server::CREATABLE === $method && wp_is_client_side_media_processing_enabled() ) { + if ( WP_REST_Server::CREATABLE !== $method ) { + return $args; + } + + if ( wp_is_client_side_media_processing_enabled() ) { $args['generate_sub_sizes'] = array( 'type' => 'boolean', 'default' => true, @@ -248,39 +252,47 @@ public function get_endpoint_args_for_item_schema( $method = WP_REST_Server::CRE 'default' => true, 'description' => __( 'Whether to convert image formats.' ), ); - $args['url'] = array( - 'type' => 'string', - 'format' => 'uri', - 'description' => __( 'URL of an external image to sideload into the media library, instead of uploading a file.' ), - 'sanitize_callback' => 'sanitize_url', - 'validate_callback' => static function ( $url, $request, $param ) { - /* - * A custom validate_callback replaces the default - * rest_validate_request_arg(), so re-apply it first to keep - * the schema checks (string type, uri format) enforced. - */ - $valid = rest_validate_request_arg( $url, $request, $param ); - if ( is_wp_error( $valid ) ) { - return $valid; - } + } - /* - * Reject URLs that are not safe to request server-side. wp_http_validate_url() - * enforces an HTTP(S) scheme and blocks private, local, and otherwise - * disallowed hosts, guarding the sideload against SSRF. - */ - if ( false === wp_http_validate_url( $url ) ) { - return new WP_Error( - 'rest_invalid_url', - __( 'Invalid URL. Provide a valid, publicly reachable HTTP or HTTPS image URL.' ), - array( 'status' => 400 ) - ); - } + /* + * Sideloading from a URL works around a cross-origin fetch the browser + * cannot make, which is a problem whether or not client side media + * processing is enabled, so the argument is always registered. Leaving + * it unregistered would not disable it: create_item() would still read + * the parameter, but without the sanitize and validate callbacks below. + */ + $args['url'] = array( + 'type' => 'string', + 'format' => 'uri', + 'description' => __( 'URL of an external image to sideload into the media library, instead of uploading a file.' ), + 'sanitize_callback' => 'sanitize_url', + 'validate_callback' => static function ( $url, $request, $param ) { + /* + * A custom validate_callback replaces the default + * rest_validate_request_arg(), so re-apply it first to keep + * the schema checks (string type, uri format) enforced. + */ + $valid = rest_validate_request_arg( $url, $request, $param ); + if ( is_wp_error( $valid ) ) { + return $valid; + } - return true; - }, - ); - } + /* + * Reject URLs that are not safe to request server-side. wp_http_validate_url() + * enforces an HTTP(S) scheme and blocks private, local, and otherwise + * disallowed hosts, guarding the sideload against SSRF. + */ + if ( false === wp_http_validate_url( $url ) ) { + return new WP_Error( + 'rest_invalid_url', + __( 'Invalid URL. Provide a valid, publicly reachable HTTP or HTTPS image URL.' ), + array( 'status' => 400 ) + ); + } + + return true; + }, + ); return $args; } @@ -381,9 +393,14 @@ public function create_item_permissions_check( $request ) { */ $prevent_unsupported_uploads = apply_filters( 'wp_prevent_unsupported_mime_type_uploads', true, $files['file']['type'] ?? null ); - // When the client handles image processing (generate_sub_sizes is false), - // skip the server-side image editor support check. - if ( false === $request['generate_sub_sizes'] ) { + /* + * When the client handles image processing (generate_sub_sizes is false), + * skip the server-side image editor support check. The parameter is only + * honored when client side media processing is enabled: it is not + * registered otherwise, so a site that has not opted in should not have + * this check relaxed by an unrecognized parameter. + */ + if ( wp_is_client_side_media_processing_enabled() && false === $request['generate_sub_sizes'] ) { $prevent_unsupported_uploads = false; } @@ -452,8 +469,17 @@ public function create_item( $request ) { ); } + /* + * The generate_sub_sizes and convert_format parameters hand image + * processing to the client, so they are only honored when client side + * media processing is enabled. They are not registered otherwise, and + * an unregistered parameter is still readable here, so the check has to + * be repeated rather than left to the schema. + */ + $client_side_media_processing = wp_is_client_side_media_processing_enabled(); + // Handle generate_sub_sizes parameter. - if ( false === $request['generate_sub_sizes'] ) { + if ( $client_side_media_processing && false === $request['generate_sub_sizes'] ) { add_filter( 'intermediate_image_sizes_advanced', '__return_empty_array', 100 ); add_filter( 'fallback_intermediate_image_sizes', '__return_empty_array', 100 ); // Disable server-side EXIF rotation so the client can handle it. @@ -462,7 +488,7 @@ public function create_item( $request ) { } // Handle convert_format parameter. - if ( false === $request['convert_format'] ) { + if ( $client_side_media_processing && false === $request['convert_format'] ) { add_filter( 'image_editor_output_format', '__return_empty_array', 100 ); } diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 90899df850d47..7a62df4bb4983 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -207,6 +207,18 @@ private function enable_client_side_media_processing(): void { do_action( 'rest_api_init', $wp_rest_server ); } + /** + * Turns client-side media processing off and rebuilds the REST server so the + * routes are registered with the feature disabled. + */ + private function disable_client_side_media_processing(): void { + add_filter( 'wp_client_side_media_processing_enabled', '__return_false' ); + + global $wp_rest_server; + $wp_rest_server = new Spy_REST_Server(); + do_action( 'rest_api_init', $wp_rest_server ); + } + public function test_register_routes() { $routes = rest_get_server()->get_routes(); $this->assertArrayHasKey( '/wp/v2/media', $routes ); @@ -3412,9 +3424,16 @@ public function test_upload_unsupported_image_type_with_filter() { * Tests the permissions check directly with file params set, since the core * check uses get_file_params() which is only populated for multipart uploads. * + * The client only handles processing when client-side media processing is + * enabled, which is also the only case where `generate_sub_sizes` is + * registered, so the feature is enabled here. + * * @ticket 64836 + * @ticket 65517 */ public function test_upload_unsupported_image_type_skipped_when_not_generating_sub_sizes() { + $this->enable_client_side_media_processing(); + wp_set_current_user( self::$author_id ); add_filter( 'wp_image_editors', '__return_empty_array' ); @@ -5569,6 +5588,126 @@ public function test_url_registered_as_creatable_arg() { $this->assertSame( 'uri', $creatable['args']['url']['format'] ); } + /** + * Verifies that the `url` argument is registered even when client-side media + * processing is disabled, while the two arguments that hand image processing + * to the client are not. + * + * Sideloading from a URL works around a cross-origin fetch the browser cannot + * make, which is a problem either way, so it does not depend on the feature. + * + * @ticket 65517 + * + * @covers WP_REST_Attachments_Controller::get_endpoint_args_for_item_schema + */ + public function test_url_registered_as_creatable_arg_without_client_side_media_processing() { + $this->disable_client_side_media_processing(); + + $routes = rest_get_server()->get_routes(); + $creatable = null; + foreach ( $routes['/wp/v2/media'] as $route ) { + if ( ! empty( $route['methods'][ WP_REST_Server::CREATABLE ] ) ) { + $creatable = $route; + break; + } + } + + $this->assertNotNull( $creatable, 'The media route should register a CREATABLE handler.' ); + $this->assertArrayHasKey( 'url', $creatable['args'] ); + $this->assertArrayNotHasKey( 'generate_sub_sizes', $creatable['args'] ); + $this->assertArrayNotHasKey( 'convert_format', $creatable['args'] ); + } + + /** + * Verifies that sideloading an external image works when client-side media + * processing is disabled. + * + * @ticket 65517 + * + * @covers WP_REST_Attachments_Controller::create_item + * @covers WP_REST_Attachments_Controller::create_item_from_url + */ + public function test_create_item_from_url_without_client_side_media_processing() { + $this->disable_client_side_media_processing(); + + wp_set_current_user( self::$superadmin_id ); + + add_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10, 3 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/photo.jpg' ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10 ); + + $data = $response->get_data(); + + $this->assertSame( 201, $response->get_status() ); + $this->assertSame( 'image', $data['media_type'] ); + $this->assertSame( 'https://example.com/photo.jpg', $this->last_download_url ); + } + + /** + * Verifies that the `url` argument's validation runs when client-side media + * processing is disabled, so an unsafe URL is rejected with a 400 rather than + * reaching the download. + * + * @ticket 65517 + * + * @covers WP_REST_Attachments_Controller::get_endpoint_args_for_item_schema + */ + public function test_url_arg_rejects_unsafe_urls_without_client_side_media_processing() { + $this->disable_client_side_media_processing(); + + wp_set_current_user( self::$superadmin_id ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'http://127.0.0.1/private.jpg' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + } + + /** + * Verifies that `generate_sub_sizes` is ignored when client-side media + * processing is disabled, so a site that has not opted in still gets its + * sub-sizes generated. + * + * The argument is not registered in that case, but an unregistered parameter + * is still readable from the request, so it has to be ignored explicitly. + * + * @ticket 65517 + * + * @covers WP_REST_Attachments_Controller::create_item + */ + public function test_generate_sub_sizes_ignored_without_client_side_media_processing() { + $this->disable_client_side_media_processing(); + + wp_set_current_user( self::$superadmin_id ); + + add_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10, 3 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/photo.jpg' ); + $request->set_param( 'generate_sub_sizes', false ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10 ); + + $data = $response->get_data(); + + $this->assertSame( 201, $response->get_status() ); + + $metadata = wp_get_attachment_metadata( $data['id'], true ); + $this->assertNotEmpty( + $metadata['sizes'] ?? array(), + 'Sub-sizes should still be generated when client-side media processing is disabled.' + ); + } + /** * Verifies that the `url` argument rejects values that are not safe to * request server-side, guarding the sideload against SSRF. From e47b7bb737d6f3e79274843ebc30ed17179cab5c Mon Sep 17 00:00:00 2001 From: Adam Silverstein Date: Tue, 4 Aug 2026 09:49:47 -0700 Subject: [PATCH 3/5] Update src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php --- .../endpoints/class-wp-rest-attachments-controller.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php index 8f4d48ab4703e..5d00b3782368d 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php @@ -633,8 +633,7 @@ protected function create_item_from_url( $request ) { * `upload_max_filesize` and `post_max_size` directives bound a request * body, not a fetch the server makes itself. * - * A limit of 0 means the size could not be determined, in which case - * no ceiling is applied. + * When `wp_max_upload_size` returns 0, no ceiling is applied. */ $max_size = (int) wp_max_upload_size(); From 0b7ea15981002f4ede7d222875f9880cc2335160 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 4 Aug 2026 16:34:19 -0700 Subject: [PATCH 4/5] REST API: Insert URL sideloads through the shared attachment path. `create_item_from_url()` built and returned its own response, so a request supplying a `url` skipped everything `create_item()` does around the insert. Fields the schema accepts were dropped, `rest_pre_insert_attachment` and `rest_insert_attachment` never fired, and neither did `wp_after_insert_post`: `wp_insert_post()` returns for attachments before its after-insert hooks, so `media_handle_sideload()` could not fire it despite the comment claiming it did. Make a URL the third source of the file, alongside a multipart upload and a raw request body. `upload_from_url()` downloads the remote file and hands it to `wp_handle_sideload()`, returning the same data as the other two handlers, so `insert_attachment()` and `create_item()` perform the insert for every path. A request that supplies both a file and a `url` is now rejected instead of having one of the two silently discarded. `insert_attachment()` also checks `prepare_item_for_database()` for an error, so an error returned from `rest_pre_insert_attachment` is honored rather than used as an attachment. The capability check in the removed method is dropped as redundant, since `create_item_permissions_check()` already denies a user without `upload_files` before the callback runs. See #65810. --- .../class-wp-rest-attachments-controller.php | 114 +++++---- .../rest-api/rest-attachments-controller.php | 235 ++++++++++++++++-- 2 files changed, 272 insertions(+), 77 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php index e0b2e6fbeaaec..4d69d09216052 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php @@ -469,6 +469,29 @@ public function create_item( $request ) { ); } + /* + * A request names the file to attach either as an upload or as a URL to + * sideload, never as both: the two are alternative sources for the same + * attachment, so honoring one would silently discard the other. An + * upload arrives as a multipart file parameter or as the raw request + * body, which upload_from_data() identifies by its Content-Disposition + * filename. + */ + if ( ! empty( $request['url'] ) ) { + $headers = $request->get_headers(); + $disposition = empty( $headers['content_disposition'] ) + ? null + : self::get_filename_from_disposition( $headers['content_disposition'] ); + + if ( ! empty( $request->get_file_params() ) || ! empty( $disposition ) ) { + return new WP_Error( + 'rest_invalid_param', + __( 'The url parameter cannot be combined with an uploaded file.' ), + array( 'status' => 400 ) + ); + } + } + /* * The generate_sub_sizes and convert_format parameters hand image * processing to the client, so they are only honored when client side @@ -496,18 +519,6 @@ public function create_item( $request ) { add_filter( 'image_editor_output_format', '__return_empty_array', 100 ); } - /* - * 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 ); - $this->remove_client_side_media_processing_filters(); - return $response; - } - $insert = $this->insert_attachment( $request ); if ( is_wp_error( $insert ) ) { @@ -602,33 +613,23 @@ public function create_item( $request ) { } /** - * Sideloads an external image from a URL into the media library. + * Handles an image sideloaded from a URL. * * Downloads the remote file on the server, avoiding a cross-origin browser - * fetch that fails under cross-origin isolation. Whether sub-sizes are - * generated is governed by the filters applied in create_item(). + * fetch that fails under cross-origin isolation. The downloaded file is + * handed to wp_handle_sideload(), so this returns the same data as the + * uploaded-file handlers and the attachment is inserted by the shared code + * in insert_attachment(). * * @since 7.1.0 * - * @param WP_REST_Request $request Full details about the request. - * @return WP_REST_Response|WP_Error Response object on success, WP_Error object on failure. + * @param string $url URL of the image to sideload. + * @param string|null $time Optional. Time formatted in 'yyyy/mm'. Default null. + * @return array|WP_Error Data from wp_handle_sideload(). */ - protected function create_item_from_url( $request ) { - // Sideloading downloads and stores a file, so require the upload capability. - if ( ! current_user_can( 'upload_files' ) ) { - return new WP_Error( - 'rest_cannot_create', - __( 'Sorry, you are not allowed to upload media on this site.' ), - array( 'status' => rest_authorization_required_code() ) - ); - } - + protected function upload_from_url( $url, $time = null ) { + // Include filesystem functions to get access to download_url() and wp_handle_sideload(). require_once ABSPATH . 'wp-admin/includes/file.php'; - require_once ABSPATH . 'wp-admin/includes/media.php'; - require_once ABSPATH . 'wp-admin/includes/image.php'; - - $url = $request['url']; - $post_id = ! empty( $request['post'] ) ? (int) $request['post'] : 0; // Derive the filename from the URL path before downloading anything. $url_path = wp_parse_url( $url, PHP_URL_PATH ); @@ -721,36 +722,25 @@ protected function create_item_from_url( $request ) { ); } - $attachment_id = media_handle_sideload( $file_array, $post_id ); + $sideloaded = wp_handle_sideload( $file_array, array( 'test_form' => false ), $time ); - if ( is_wp_error( $attachment_id ) ) { + if ( isset( $sideloaded['error'] ) ) { /* - * media_handle_sideload() deletes the temp file on success; remove - * it explicitly when the sideload fails. + * wp_handle_sideload() moves the temp file on success; remove it + * explicitly when it fails. */ if ( file_exists( $tmp_file ) ) { wp_delete_file( $tmp_file ); } - return $attachment_id; - } - - $attachment = get_post( $attachment_id ); - - $request->set_param( 'context', 'edit' ); - /* - * media_handle_sideload() fires the standard insert hooks (including - * wp_after_insert_post), but not the REST-specific action, so fire it - * here for parity with the uploaded-file path in create_item(). - */ - /** This action is documented in wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php */ - do_action( 'rest_after_insert_attachment', $attachment, $request, true ); - - $response = $this->prepare_item_for_response( $attachment, $request ); - $response->set_status( 201 ); - $response->header( 'Location', rest_url( rest_get_route_for_post( $attachment_id ) ) ); + return new WP_Error( + 'rest_upload_sideload_error', + $sideloaded['error'], + array( 'status' => 500 ) + ); + } - return $response; + return $sideloaded; } /** @@ -790,7 +780,16 @@ protected function insert_attachment( $request ) { } } - if ( ! empty( $files ) ) { + /* + * A URL is the third way to name the file, alongside a multipart upload + * and a raw request body. All three produce the same data, so the rest + * of this method is shared: the URL path gets the same title and caption + * defaults, the same hooks, and the same terms and meta handling as an + * uploaded file. + */ + if ( ! empty( $request['url'] ) ) { + $file = $this->upload_from_url( $request['url'], $time ); + } elseif ( ! empty( $files ) ) { $file = $this->upload_from_file( $files, $headers, $time ); } else { $file = $this->upload_from_data( $request->get_body(), $headers, $time ); @@ -831,6 +830,11 @@ protected function insert_attachment( $request ) { $attachment = $this->prepare_item_for_database( $request ); + // The rest_pre_insert_attachment filter can return an error. + if ( is_wp_error( $attachment ) ) { + return $attachment; + } + $attachment->post_mime_type = $type; $attachment->guid = $url; diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index c27b4035b9c55..da9af1c80086d 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -5353,7 +5353,8 @@ public function mock_image_download( $response, $args, $url ) { * @ticket 65517 * * @covers WP_REST_Attachments_Controller::create_item - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_sideloads_without_subsizes() { $this->enable_client_side_media_processing(); @@ -5389,7 +5390,8 @@ public function test_create_item_from_url_sideloads_without_subsizes() { * @ticket 65517 * * @covers WP_REST_Attachments_Controller::create_item - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_generates_subsizes_by_default() { $this->enable_client_side_media_processing(); @@ -5420,7 +5422,8 @@ public function test_create_item_from_url_generates_subsizes_by_default() { * * @ticket 65517 * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_fires_rest_after_insert_attachment() { $this->enable_client_side_media_processing(); @@ -5461,7 +5464,8 @@ public function test_create_item_from_url_fires_rest_after_insert_attachment() { * @ticket 65517 * * @covers WP_REST_Attachments_Controller::create_item - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_attaches_to_post() { $this->enable_client_side_media_processing(); @@ -5494,7 +5498,8 @@ public function test_create_item_from_url_attaches_to_post() { * @ticket 65517 * * @covers WP_REST_Attachments_Controller::create_item - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_returns_error_on_download_failure() { $this->enable_client_side_media_processing(); @@ -5526,7 +5531,8 @@ public function test_create_item_from_url_returns_error_on_download_failure() { * @group multisite * @group ms-required * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url * @covers WP_REST_Attachments_Controller::check_upload_size */ public function test_create_item_from_url_exceeds_multisite_max_filesize() { @@ -5559,7 +5565,8 @@ public function test_create_item_from_url_exceeds_multisite_max_filesize() { * @group multisite * @group ms-required * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url * @covers WP_REST_Attachments_Controller::check_upload_size */ public function test_create_item_from_url_exceeds_multisite_site_upload_space() { @@ -5591,7 +5598,8 @@ public function test_create_item_from_url_exceeds_multisite_site_upload_space() * * @ticket 65517 * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_exceeds_max_upload_size() { $this->enable_client_side_media_processing(); @@ -5619,7 +5627,8 @@ public function test_create_item_from_url_exceeds_max_upload_size() { * * @ticket 65517 * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_limits_the_download_size() { $this->enable_client_side_media_processing(); @@ -5681,7 +5690,8 @@ public function filter_small_upload_size_limit() { * * @ticket 65517 * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_rejects_url_without_filename() { $this->enable_client_side_media_processing(); @@ -5716,7 +5726,8 @@ public function test_create_item_from_url_rejects_url_without_filename() { * * @dataProvider data_create_item_from_url_rejects_non_image_extension * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url * * @param string $url URL with a disallowed file extension. */ @@ -5766,7 +5777,8 @@ public function data_create_item_from_url_rejects_non_image_extension() { * * @ticket 65517 * - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_requires_upload_capability() { $subscriber_id = self::factory()->user->create( array( 'role' => 'subscriber' ) ); @@ -5783,18 +5795,11 @@ public function test_create_item_from_url_requires_upload_capability() { $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); $request->set_param( 'url', 'https://example.com/denied.jpg' ); - $controller = new WP_REST_Attachments_Controller( 'attachment' ); - $method = new ReflectionMethod( $controller, 'create_item_from_url' ); - if ( PHP_VERSION_ID < 80100 ) { - $method->setAccessible( true ); - } - $result = $method->invoke( $controller, $request ); + $response = rest_get_server()->dispatch( $request ); remove_filter( 'pre_http_request', $track ); - $this->assertWPError( $result ); - $this->assertSame( 'rest_cannot_create', $result->get_error_code() ); - $this->assertSame( 403, $result->get_error_data()['status'] ); + $this->assertErrorResponse( 'rest_cannot_create', $response ); $this->assertFalse( $downloaded, 'No download should be attempted without upload_files.' ); } @@ -5886,7 +5891,8 @@ public function test_url_registered_as_creatable_arg_without_client_side_media_p * @ticket 65517 * * @covers WP_REST_Attachments_Controller::create_item - * @covers WP_REST_Attachments_Controller::create_item_from_url + * @covers WP_REST_Attachments_Controller::insert_attachment + * @covers WP_REST_Attachments_Controller::upload_from_url */ public function test_create_item_from_url_without_client_side_media_processing() { $this->disable_client_side_media_processing(); @@ -6012,4 +6018,189 @@ public function test_url_arg_rejects_unsafe_urls() { $this->assertSame( 400, $result->get_error_data()['status'] ); } } + + /** + * Verifies that a request cannot supply both an uploaded file and a `url`. + * + * The two are alternative sources for the same attachment, so honoring one + * would silently discard the other. + * + * @ticket 65810 + * + * @covers WP_REST_Attachments_Controller::create_item + */ + public function test_create_item_rejects_a_file_and_url_together() { + wp_set_current_user( self::$superadmin_id ); + + // Fail loudly if the request is not rejected and a download is attempted. + $downloaded = false; + $track = static function () use ( &$downloaded ) { + $downloaded = true; + return new WP_Error( 'http_request_failed', 'Should not be reached.' ); + }; + add_filter( 'pre_http_request', $track ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/photo.jpg' ); + $request->set_file_params( + array( + 'file' => array( + 'file' => file_get_contents( self::$test_file ), + 'name' => 'canola.jpg', + 'size' => filesize( self::$test_file ), + 'tmp_name' => self::$test_file, + ), + ) + ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', $track ); + + $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + $this->assertFalse( $downloaded, 'No download should be attempted when a file is also supplied.' ); + } + + /** + * Verifies that a request cannot supply both a raw body upload and a `url`. + * + * The raw body path is identified by its Content-Disposition filename, so + * the conflict has to be detected from the headers rather than from the + * file parameters. + * + * @ticket 65810 + * + * @covers WP_REST_Attachments_Controller::create_item + */ + public function test_create_item_rejects_a_body_upload_and_url_together() { + wp_set_current_user( self::$superadmin_id ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_header( 'Content-Type', 'image/jpeg' ); + $request->set_header( 'Content-Disposition', 'attachment; filename=canola.jpg' ); + $request->set_body( file_get_contents( self::$test_file ) ); + $request->set_param( 'url', 'https://example.com/photo.jpg' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + } + + /** + * Verifies that the fields carried on the request are applied to an + * attachment created from a `url`, as they are for an uploaded file. + * + * The URL path previously returned before create_item() applied them, so + * they were accepted by the schema and then silently dropped. + * + * @ticket 65810 + * + * @covers WP_REST_Attachments_Controller::create_item + * @covers WP_REST_Attachments_Controller::insert_attachment + */ + public function test_create_item_from_url_applies_request_fields() { + wp_set_current_user( self::$superadmin_id ); + + add_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10, 3 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/fields.jpg' ); + $request->set_param( 'title', 'My sideloaded title' ); + $request->set_param( 'caption', 'My sideloaded caption' ); + $request->set_param( 'description', 'My sideloaded description' ); + $request->set_param( 'alt_text', 'My sideloaded alt text' ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10 ); + + $this->assertSame( 201, $response->get_status() ); + + $data = $response->get_data(); + $attachment = get_post( $data['id'] ); + + $this->assertSame( 'My sideloaded title', $attachment->post_title ); + $this->assertSame( 'My sideloaded caption', $attachment->post_excerpt ); + $this->assertSame( 'My sideloaded description', $attachment->post_content ); + $this->assertSame( 'My sideloaded alt text', get_post_meta( $attachment->ID, '_wp_attachment_image_alt', true ) ); + } + + /** + * Verifies that the insert hooks fire when an attachment is created from a + * `url`, as they do for an uploaded file. + * + * rest_pre_insert_attachment and rest_insert_attachment fire from + * prepare_item_for_database() and insert_attachment(), which the URL path + * did not reach, and wp_after_insert_post never fires for attachments from + * wp_insert_post() because it returns before the after-insert hooks. + * + * @ticket 65810 + * + * @covers WP_REST_Attachments_Controller::create_item + * @covers WP_REST_Attachments_Controller::insert_attachment + */ + public function test_create_item_from_url_fires_the_insert_hooks() { + wp_set_current_user( self::$superadmin_id ); + + $fired = array(); + $spy = static function ( $hook ) use ( &$fired ) { + return static function ( $value ) use ( $hook, &$fired ) { + $fired[] = $hook; + return $value; + }; + }; + + $pre = $spy( 'rest_pre_insert_attachment' ); + $insert = $spy( 'rest_insert_attachment' ); + $after = $spy( 'wp_after_insert_post' ); + + add_filter( 'rest_pre_insert_attachment', $pre ); + add_action( 'rest_insert_attachment', $insert ); + add_action( 'wp_after_insert_post', $after ); + add_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10, 3 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/hooks.jpg' ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10 ); + remove_action( 'wp_after_insert_post', $after ); + remove_action( 'rest_insert_attachment', $insert ); + remove_filter( 'rest_pre_insert_attachment', $pre ); + + $this->assertSame( 201, $response->get_status() ); + $this->assertContains( 'rest_pre_insert_attachment', $fired ); + $this->assertContains( 'rest_insert_attachment', $fired ); + $this->assertContains( 'wp_after_insert_post', $fired ); + } + + /** + * Verifies that an error returned from the rest_pre_insert_attachment + * filter is honored rather than used as if it were an attachment. + * + * @ticket 65810 + * + * @covers WP_REST_Attachments_Controller::insert_attachment + */ + public function test_create_item_from_url_honors_a_pre_insert_error() { + wp_set_current_user( self::$superadmin_id ); + + $deny = static function () { + return new WP_Error( 'rest_pre_insert_denied', 'Denied.', array( 'status' => 400 ) ); + }; + + add_filter( 'rest_pre_insert_attachment', $deny ); + add_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10, 3 ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_param( 'url', 'https://example.com/denied-by-filter.jpg' ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'pre_http_request', array( $this, 'mock_image_download' ), 10 ); + remove_filter( 'rest_pre_insert_attachment', $deny ); + + $this->assertErrorResponse( 'rest_pre_insert_denied', $response, 400 ); + } } From ee6d4c862eb5e6a521d4717527c19935d0ce8857 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 4 Aug 2026 23:08:31 -0700 Subject: [PATCH 5/5] Remove the explanatory comment above the url argument registration. The comment described why an earlier trunk iteration changed rather than documenting the feature, which read as out of place for new 7.1 code. The registration stands on its own. See https://github.com/WordPress/wordpress-develop/pull/12846#discussion_r3717070819 --- .../endpoints/class-wp-rest-attachments-controller.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php index 89ee536567eea..72ca58b34819a 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php @@ -254,13 +254,6 @@ public function get_endpoint_args_for_item_schema( $method = WP_REST_Server::CRE ); } - /* - * Sideloading from a URL works around a cross-origin fetch the browser - * cannot make, which is a problem whether or not client side media - * processing is enabled, so the argument is always registered. Leaving - * it unregistered would not disable it: create_item() would still read - * the parameter, but without the sanitize and validate callbacks below. - */ $args['url'] = array( 'type' => 'string', 'format' => 'uri',