From afca1e99711e802ebef205b11fefb7b9d0302125 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 4 Aug 2026 00:18:08 -0700 Subject: [PATCH 1/3] 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 ee6d4c862eb5e6a521d4717527c19935d0ce8857 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 4 Aug 2026 23:08:31 -0700 Subject: [PATCH 2/3] 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', From 63f6f1acab0fd9d46262d28751561a620c18aaf6 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Tue, 4 Aug 2026 23:29:47 -0700 Subject: [PATCH 3/3] Register the media creation arguments unconditionally. Gating `generate_sub_sizes` and `convert_format` on client side media processing made the schema depend on request context rather than site configuration, since the feature is derived from the scheme and host. The same site could advertise different arguments over http and https, or when reached behind a proxy that does not set HTTPS. Both arguments are instructions the server can carry out on its own - skipping sub sizes or format conversion needs no client - so they are now registered and honored either way, and the runtime checks that duplicated the registration condition are gone. The one condition kept is the unsupported image type check in the permissions callback: that check exists because the server cannot process the image, so it is only relaxed when client side media processing means something else can. Sub sizes skipped this way remain recoverable through wp_update_image_subsizes(), and behavior with client side media processing enabled is unchanged. --- .../class-wp-rest-attachments-controller.php | 45 +++++------ .../rest-api/rest-attachments-controller.php | 78 ++++++++++++++----- 2 files changed, 77 insertions(+), 46 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 72ca58b34819a..61d33ed1957c8 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 @@ -241,18 +241,17 @@ public function get_endpoint_args_for_item_schema( $method = WP_REST_Server::CRE return $args; } - if ( wp_is_client_side_media_processing_enabled() ) { - $args['generate_sub_sizes'] = array( - 'type' => 'boolean', - 'default' => true, - 'description' => __( 'Whether to generate image sub sizes.' ), - ); - $args['convert_format'] = array( - 'type' => 'boolean', - 'default' => true, - 'description' => __( 'Whether to convert image formats.' ), - ); - } + $args['generate_sub_sizes'] = array( + 'type' => 'boolean', + 'default' => true, + 'description' => __( 'Whether to generate image sub sizes.' ), + ); + + $args['convert_format'] = array( + 'type' => 'boolean', + 'default' => true, + 'description' => __( 'Whether to convert image formats.' ), + ); $args['url'] = array( 'type' => 'string', @@ -388,10 +387,11 @@ public function create_item_permissions_check( $request ) { /* * 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. + * skip the server-side image editor support check. This check exists + * because the server cannot process the image, so it is only relaxed when + * client side media processing is enabled and something else can. Asking + * to skip sub sizes on a site without it does not make an unsupported + * image type any more usable. */ if ( wp_is_client_side_media_processing_enabled() && false === $request['generate_sub_sizes'] ) { $prevent_unsupported_uploads = false; @@ -462,17 +462,8 @@ 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 ( $client_side_media_processing && false === $request['generate_sub_sizes'] ) { + if ( 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. @@ -485,7 +476,7 @@ public function create_item( $request ) { } // Handle convert_format parameter. - if ( $client_side_media_processing && false === $request['convert_format'] ) { + if ( 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 361bcff7606e4..d9d9b51030b2c 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -3424,9 +3424,9 @@ 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. + * The check is only relaxed when client-side media processing is enabled, + * since that is what makes the client able to handle the image, so the + * feature is enabled here. * * @ticket 64836 * @ticket 65517 @@ -5758,18 +5758,18 @@ public function test_url_registered_as_creatable_arg() { } /** - * 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. + * Verifies that the media creation arguments are registered even when + * client-side media processing is disabled. * - * 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. + * The feature is determined per request, from the scheme and host, so gating + * the schema on it would advertise different arguments for the same site + * depending on how it was reached. * * @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() { + public function test_creatable_args_registered_without_client_side_media_processing() { $this->disable_client_side_media_processing(); $routes = rest_get_server()->get_routes(); @@ -5783,8 +5783,8 @@ public function test_url_registered_as_creatable_arg_without_client_side_media_p $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'] ); + $this->assertArrayHasKey( 'generate_sub_sizes', $creatable['args'] ); + $this->assertArrayHasKey( 'convert_format', $creatable['args'] ); } /** @@ -5840,18 +5840,18 @@ public function test_url_arg_rejects_unsafe_urls_without_client_side_media_proce } /** - * 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. + * Verifies that `generate_sub_sizes` is honored when client-side media + * processing is disabled. * - * 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. + * Skipping sub-size generation is a request the server can carry out on its + * own, so it does not depend on the feature. Sub-sizes can still be added + * later with wp_update_image_subsizes(). * * @ticket 65517 * * @covers WP_REST_Attachments_Controller::create_item */ - public function test_generate_sub_sizes_ignored_without_client_side_media_processing() { + public function test_generate_sub_sizes_honored_without_client_side_media_processing() { $this->disable_client_side_media_processing(); wp_set_current_user( self::$superadmin_id ); @@ -5871,10 +5871,50 @@ public function test_generate_sub_sizes_ignored_without_client_side_media_proces $this->assertSame( 201, $response->get_status() ); $metadata = wp_get_attachment_metadata( $data['id'], true ); - $this->assertNotEmpty( + $this->assertEmpty( $metadata['sizes'] ?? array(), - 'Sub-sizes should still be generated when client-side media processing is disabled.' + 'Sub-sizes should not be generated when generate_sub_sizes is false.' + ); + } + + /** + * Verifies that `generate_sub_sizes` does not relax the unsupported image + * type check when client-side media processing is disabled. + * + * That check exists because the server cannot process the image, so it should + * only be relaxed when the client can process it instead. Otherwise the + * upload is stored unprocessable. + * + * @ticket 65517 + * + * @covers WP_REST_Attachments_Controller::create_item_permissions_check + */ + public function test_unsupported_image_type_still_checked_without_client_side_media_processing() { + $this->disable_client_side_media_processing(); + + wp_set_current_user( self::$author_id ); + + add_filter( 'wp_image_editors', '__return_empty_array' ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_file_params( + array( + 'file' => array( + 'name' => 'avif-lossy.avif', + 'type' => 'image/avif', + 'tmp_name' => self::$test_avif_file, + 'error' => 0, + 'size' => filesize( self::$test_avif_file ), + ), + ) ); + $request->set_param( 'generate_sub_sizes', false ); + + $controller = new WP_REST_Attachments_Controller( 'attachment' ); + $result = $controller->create_item_permissions_check( $request ); + + $this->assertWPError( $result ); + $this->assertSame( 'rest_upload_image_type_not_supported', $result->get_error_code() ); } /**