From c8d080816031687d1edd9710ddd6dac939fbdbda Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 22 May 2026 13:30:02 -0700 Subject: [PATCH 1/8] REST API: Move sideload metadata writing to the finalize endpoint Backport of Gutenberg PR #75888. Eliminate the read-modify-write race between concurrent sideloads for the same attachment by no longer writing attachment metadata in the sideload endpoint. Instead, sideload returns lightweight sub-size data (dimensions, filename, filesize) which the client accumulates and passes to the finalize endpoint, which writes all collected sub-sizes in a single metadata update. This matches how core generates sub-sizes (one metadata write after all sizes exist) and replaces the earlier per-attachment locking approach that the merged Gutenberg PR ultimately abandoned. --- .../class-wp-rest-attachments-controller.php | 136 +++++++++---- .../rest-api/rest-attachments-controller.php | 188 +++++++++++++++++- 2 files changed, 277 insertions(+), 47 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 21805778ba659..da53ce9dcd188 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 @@ -113,10 +113,46 @@ public function register_routes() { 'callback' => array( $this, 'finalize_item' ), 'permission_callback' => array( $this, 'edit_media_item_permissions_check' ), 'args' => array( - 'id' => array( + 'id' => array( 'description' => __( 'Unique identifier for the attachment.' ), 'type' => 'integer', ), + 'sub_sizes' => array( + 'description' => __( 'Array of sub-size metadata collected from sideload responses.' ), + 'type' => 'array', + 'default' => array(), + 'items' => array( + 'type' => 'object', + 'properties' => array( + 'image_size' => array( + 'type' => 'string', + 'required' => true, + ), + 'width' => array( + 'type' => 'integer', + 'minimum' => 1, + ), + 'height' => array( + 'type' => 'integer', + 'minimum' => 1, + ), + 'file' => array( + 'type' => 'string', + ), + 'mime_type' => array( + 'type' => 'string', + 'pattern' => '^image/.*', + ), + 'filesize' => array( + 'type' => 'integer', + 'minimum' => 1, + ), + 'original_image' => array( + 'type' => 'string', + ), + ), + ), + ), ), ), 'allow_batch' => $this->allow_batch, @@ -2082,16 +2118,19 @@ public function sideload_item( WP_REST_Request $request ) { $image_size = $request['image_size']; - $metadata = wp_get_attachment_metadata( $attachment_id, true ); - - if ( ! $metadata ) { - $metadata = array(); - } + // Build sub-size data to return to the client. + // The client accumulates these and sends them all to the finalize + // endpoint, which writes the metadata in a single operation. This + // avoids the read-modify-write race that concurrent sideloads for the + // same attachment would otherwise hit. + $sub_size_data = array( + 'image_size' => $image_size, + ); if ( 'original' === $image_size ) { - $metadata['original_image'] = wp_basename( $path ); + $sub_size_data['file'] = wp_basename( $path ); } elseif ( 'scaled' === $image_size ) { - // The current attached file is the original; record it as original_image. + // Record the current attached file as the original. $current_file = get_attached_file( $attachment_id, true ); if ( ! $current_file ) { @@ -2102,7 +2141,7 @@ public function sideload_item( WP_REST_Request $request ) { ); } - $metadata['original_image'] = wp_basename( $current_file ); + $sub_size_data['original_image'] = wp_basename( $current_file ); // Validate the scaled image before updating the attached file. $size = wp_getimagesize( $path ); @@ -2117,6 +2156,7 @@ public function sideload_item( WP_REST_Request $request ) { } // Update the attached file to point to the scaled version. + // This writes to _wp_attached_file meta, not _wp_attachment_metadata. if ( get_attached_file( $attachment_id, true ) !== $path && ! update_attached_file( $attachment_id, $path ) @@ -2128,42 +2168,21 @@ public function sideload_item( WP_REST_Request $request ) { ); } - $metadata['width'] = $size[0]; - $metadata['height'] = $size[1]; - $metadata['filesize'] = $filesize; - $metadata['file'] = _wp_relative_upload_path( $path ); + $sub_size_data['width'] = $size[0]; + $sub_size_data['height'] = $size[1]; + $sub_size_data['filesize'] = $filesize; + $sub_size_data['file'] = _wp_relative_upload_path( $path ); } else { - $metadata['sizes'] = $metadata['sizes'] ?? array(); - $size = wp_getimagesize( $path ); - $metadata['sizes'][ $image_size ] = array( - 'width' => $size ? $size[0] : 0, - 'height' => $size ? $size[1] : 0, - 'file' => wp_basename( $path ), - 'mime-type' => $type, - 'filesize' => wp_filesize( $path ), - ); - } - - wp_update_attachment_metadata( $attachment_id, $metadata ); - - $response_request = new WP_REST_Request( - WP_REST_Server::READABLE, - rest_get_route_for_post( $attachment_id ) - ); - - $response_request['context'] = 'edit'; - - if ( isset( $request['_fields'] ) ) { - $response_request['_fields'] = $request['_fields']; + $sub_size_data['width'] = $size ? $size[0] : 0; + $sub_size_data['height'] = $size ? $size[1] : 0; + $sub_size_data['file'] = wp_basename( $path ); + $sub_size_data['mime_type'] = $type; + $sub_size_data['filesize'] = wp_filesize( $path ); } - $response = $this->prepare_item_for_response( get_post( $attachment_id ), $response_request ); - - $response->header( 'Location', rest_url( rest_get_route_for_post( $attachment_id ) ) ); - - return $response; + return rest_ensure_response( $sub_size_data ); } /** @@ -2215,9 +2234,11 @@ private static function filter_wp_unique_filename( $filename, $dir, $number, $at /** * Finalizes an attachment after client-side media processing. * - * Triggers the 'wp_generate_attachment_metadata' filter so that - * server-side plugins can process the attachment after all client-side - * operations (upload, thumbnail generation, sideloads) are complete. + * Applies the sub-size metadata collected from sideload responses in a + * single metadata update, then triggers the 'wp_generate_attachment_metadata' + * filter so that server-side plugins can process the attachment after all + * client-side operations (upload, thumbnail generation, sideloads) are + * complete. * * @since 7.1.0 * @@ -2237,6 +2258,35 @@ public function finalize_item( WP_REST_Request $request ) { $metadata = array(); } + // Apply all sub-size metadata collected from sideload responses. + $sub_sizes = $request['sub_sizes'] ?? array(); + + foreach ( $sub_sizes as $sub_size ) { + $image_size = $sub_size['image_size']; + + if ( 'original' === $image_size ) { + $metadata['original_image'] = $sub_size['file']; + } elseif ( 'scaled' === $image_size ) { + if ( ! empty( $sub_size['original_image'] ) ) { + $metadata['original_image'] = $sub_size['original_image']; + } + $metadata['width'] = $sub_size['width'] ?? 0; + $metadata['height'] = $sub_size['height'] ?? 0; + $metadata['filesize'] = $sub_size['filesize'] ?? 0; + $metadata['file'] = $sub_size['file'] ?? ''; + } else { + $metadata['sizes'] = $metadata['sizes'] ?? array(); + + $metadata['sizes'][ $image_size ] = array( + 'width' => $sub_size['width'] ?? 0, + 'height' => $sub_size['height'] ?? 0, + 'file' => $sub_size['file'] ?? '', + 'mime-type' => $sub_size['mime_type'] ?? '', + 'filesize' => $sub_size['filesize'] ?? 0, + ); + } + } + /** This filter is documented in wp-admin/includes/image.php */ $metadata = apply_filters( 'wp_generate_attachment_metadata', $metadata, $attachment_id, 'update' ); diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 79e9d23cf9dd3..76c944c72f55e 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -3275,16 +3275,32 @@ public function test_sideload_scaled_image() { $this->assertSame( 200, $response->get_status(), 'Sideloading scaled image should succeed.' ); + // The sideload endpoint returns lightweight sub-size data; the metadata + // is written later by the finalize endpoint. + $sub_size = $response->get_data(); + $this->assertSame( 'scaled', $sub_size['image_size'], 'Response should echo the image_size.' ); + $this->assertSame( wp_basename( $original_file ), $sub_size['original_image'], 'Response original_image should be the basename of the original attached file.' ); + $this->assertGreaterThan( 0, $sub_size['width'], 'Response width should be positive.' ); + $this->assertGreaterThan( 0, $sub_size['height'], 'Response height should be positive.' ); + $this->assertGreaterThan( 0, $sub_size['filesize'], 'Response filesize should be positive.' ); + $this->assertStringContainsString( 'scaled', $sub_size['file'], 'Response file should reference the scaled version.' ); + + // The attached file is still repointed to the scaled version during sideload. + $new_file = get_attached_file( $attachment_id, true ); + $this->assertStringContainsString( 'scaled', wp_basename( $new_file ), 'Attached file should now be the scaled version.' ); + + // Finalize with the collected sub-size, which writes the metadata. + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" ); + $request->set_param( 'sub_sizes', array( $sub_size ) ); + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' ); + $metadata = wp_get_attachment_metadata( $attachment_id ); // The original file should now be recorded as original_image. $this->assertArrayHasKey( 'original_image', $metadata, 'Metadata should contain original_image.' ); $this->assertSame( wp_basename( $original_file ), $metadata['original_image'], 'original_image should be the basename of the original attached file.' ); - // The attached file should now point to the scaled version. - $new_file = get_attached_file( $attachment_id, true ); - $this->assertStringContainsString( 'scaled', wp_basename( $new_file ), 'Attached file should now be the scaled version.' ); - // Metadata should have width, height, filesize, and file updated. $this->assertArrayHasKey( 'width', $metadata, 'Metadata should contain width.' ); $this->assertArrayHasKey( 'height', $metadata, 'Metadata should contain height.' ); @@ -3541,4 +3557,168 @@ public function test_finalize_item_invalid_id(): void { $this->assertErrorResponse( 'rest_post_invalid_id', $response, 404 ); } + + /** + * Tests that the finalize endpoint writes regular sub-size metadata + * collected from sideload responses. + * + * @ticket 62243 + * @covers WP_REST_Attachments_Controller::finalize_item + * @requires function imagejpeg + */ + public function test_finalize_writes_regular_sub_sizes(): void { + $this->enable_client_side_media_processing(); + + wp_set_current_user( self::$author_id ); + + // Create an attachment without generating sub-sizes server-side. + $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_param( 'generate_sub_sizes', false ); + $request->set_body( (string) file_get_contents( self::$test_file ) ); + $response = rest_get_server()->dispatch( $request ); + $attachment_id = $response->get_data()['id']; + + $this->assertSame( 201, $response->get_status() ); + + // Sideload a thumbnail sub-size; the response carries its metadata. + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" ); + $request->set_header( 'Content-Type', 'image/jpeg' ); + $request->set_header( 'Content-Disposition', 'attachment; filename=canola-thumb.jpg' ); + $request->set_param( 'image_size', 'thumbnail' ); + $request->set_body( (string) file_get_contents( self::$test_file ) ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertSame( 200, $response->get_status(), 'Sideloading a thumbnail should succeed.' ); + + $sub_size = $response->get_data(); + $this->assertSame( 'thumbnail', $sub_size['image_size'], 'Response should echo the image_size.' ); + + // Finalize with the collected sub-size, which writes it into metadata. + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" ); + $request->set_param( 'sub_sizes', array( $sub_size ) ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + $this->assertArrayHasKey( 'sizes', $metadata, 'Metadata should contain sizes.' ); + $this->assertArrayHasKey( 'thumbnail', $metadata['sizes'], 'Metadata sizes should contain the sideloaded thumbnail.' ); + $this->assertSame( 'image/jpeg', $metadata['sizes']['thumbnail']['mime-type'], 'Thumbnail mime-type should be recorded.' ); + $this->assertGreaterThan( 0, $metadata['sizes']['thumbnail']['filesize'], 'Thumbnail filesize should be positive.' ); + } + + /** + * Tests that the finalize endpoint records original_image from an + * 'original' sub-size collected from a sideload response. + * + * @ticket 62243 + * @covers WP_REST_Attachments_Controller::finalize_item + * @requires function imagejpeg + */ + public function test_finalize_writes_original_metadata(): void { + $this->enable_client_side_media_processing(); + + wp_set_current_user( self::$author_id ); + + // Create an attachment without generating sub-sizes server-side. + $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_param( 'generate_sub_sizes', false ); + $request->set_body( (string) file_get_contents( self::$test_file ) ); + $response = rest_get_server()->dispatch( $request ); + $attachment_id = $response->get_data()['id']; + + $this->assertSame( 201, $response->get_status() ); + + // Sideload the 'original' version (simulating a rotated image), which + // returns the basename without writing metadata. + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" ); + $request->set_header( 'Content-Type', 'image/jpeg' ); + $request->set_header( 'Content-Disposition', 'attachment; filename=canola-original.jpg' ); + $request->set_param( 'image_size', 'original' ); + $request->set_body( (string) file_get_contents( self::$test_file ) ); + $response = rest_get_server()->dispatch( $request ); + $original_data = $response->get_data(); + + $this->assertSame( 200, $response->get_status(), 'Sideloading the original should succeed.' ); + $this->assertSame( 'original', $original_data['image_size'], 'Response should echo the image_size.' ); + $this->assertSame( 'canola-original.jpg', $original_data['file'], 'Response should return the file basename.' ); + + // Sideload must not write metadata; that happens in finalize. + $metadata = wp_get_attachment_metadata( $attachment_id, true ); + $this->assertArrayNotHasKey( 'original_image', $metadata, 'Sideload should not write original_image metadata.' ); + + // Finalize with the collected original sub-size. + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" ); + $request->set_param( 'sub_sizes', array( $original_data ) ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + $this->assertSame( 'canola-original.jpg', $metadata['original_image'], 'Finalize should record original_image from the sub-size.' ); + } + + /** + * Tests that the finalize endpoint preserves existing image_meta (EXIF) + * when adding sub-sizes collected from sideload responses. + * + * @ticket 62243 + * @covers WP_REST_Attachments_Controller::finalize_item + * @requires function imagejpeg + * @requires extension exif + */ + public function test_finalize_preserves_image_meta(): void { + $this->enable_client_side_media_processing(); + + wp_set_current_user( self::$author_id ); + + $exif_file = DIR_TESTDATA . '/images/2004-07-22-DSC_0008.jpg'; + + // Create an attachment without generating sub-sizes server-side. + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_header( 'Content-Type', 'image/jpeg' ); + $request->set_header( 'Content-Disposition', 'attachment; filename=2004-07-22-DSC_0008.jpg' ); + $request->set_param( 'generate_sub_sizes', false ); + $request->set_body( (string) file_get_contents( $exif_file ) ); + $response = rest_get_server()->dispatch( $request ); + $attachment_id = $response->get_data()['id']; + + $this->assertSame( 201, $response->get_status() ); + + $original_image_meta = wp_get_attachment_metadata( $attachment_id, true )['image_meta']; + + // Finalize with a thumbnail sub-size. + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" ); + $request->set_param( + 'sub_sizes', + array( + array( + 'image_size' => 'thumbnail', + 'width' => 150, + 'height' => 150, + 'file' => '2004-07-22-DSC_0008-150x150.jpg', + 'mime_type' => 'image/jpeg', + 'filesize' => 5000, + ), + ) + ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' ); + + $metadata = wp_get_attachment_metadata( $attachment_id ); + + // The sub-size should have been added. + $this->assertArrayHasKey( 'thumbnail', $metadata['sizes'], 'Finalize should add the thumbnail sub-size.' ); + + // The EXIF image_meta should be unchanged. + $this->assertSame( $original_image_meta['aperture'], $metadata['image_meta']['aperture'], 'Aperture should be preserved.' ); + $this->assertSame( $original_image_meta['camera'], $metadata['image_meta']['camera'], 'Camera should be preserved.' ); + $this->assertSame( $original_image_meta['focal_length'], $metadata['image_meta']['focal_length'], 'Focal length should be preserved.' ); + $this->assertSame( $original_image_meta['iso'], $metadata['image_meta']['iso'], 'ISO should be preserved.' ); + } } From 29d4cbb1453855743ce2f5e216a37c61ae67410c Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 22 May 2026 15:28:55 -0700 Subject: [PATCH 2/8] Tests: Point new finalize tests at Trac ticket #65329 The three sub-size finalize tests added in this backport used the placeholder ticket 62243 (the original client-side media feature ticket) before a dedicated ticket existed. Trac #65329 now tracks this change, so update those @ticket annotations. Pre-existing finalize tests keep 62243. --- .../phpunit/tests/rest-api/rest-attachments-controller.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 76c944c72f55e..8ca608d184e39 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -3562,7 +3562,7 @@ public function test_finalize_item_invalid_id(): void { * Tests that the finalize endpoint writes regular sub-size metadata * collected from sideload responses. * - * @ticket 62243 + * @ticket 65329 * @covers WP_REST_Attachments_Controller::finalize_item * @requires function imagejpeg */ @@ -3613,7 +3613,7 @@ public function test_finalize_writes_regular_sub_sizes(): void { * Tests that the finalize endpoint records original_image from an * 'original' sub-size collected from a sideload response. * - * @ticket 62243 + * @ticket 65329 * @covers WP_REST_Attachments_Controller::finalize_item * @requires function imagejpeg */ @@ -3666,7 +3666,7 @@ public function test_finalize_writes_original_metadata(): void { * Tests that the finalize endpoint preserves existing image_meta (EXIF) * when adding sub-sizes collected from sideload responses. * - * @ticket 62243 + * @ticket 65329 * @covers WP_REST_Attachments_Controller::finalize_item * @requires function imagejpeg * @requires extension exif From d1b3a76dad6d7a804d2683cdbe4a3cb9a1c1f0a9 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Fri, 22 May 2026 15:30:59 -0700 Subject: [PATCH 3/8] Tests: Cross-reference Trac #65329 in test_sideload_scaled_image This backport changes the scaled-image sideload behavior (sideload now returns sub-size data and metadata is written at finalize), so add a 65329 ticket reference alongside the existing 64737. --- tests/phpunit/tests/rest-api/rest-attachments-controller.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 8ca608d184e39..53df60e4b3b45 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -3245,6 +3245,7 @@ static function ( $data ) use ( &$captured_data ) { * Tests sideloading a scaled image for an existing attachment. * * @ticket 64737 + * @ticket 65329 * @requires function imagejpeg */ public function test_sideload_scaled_image() { From 1e166645c3c7cb60c52cfb92fb6caf22a5776427 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 15 Jun 2026 08:42:18 +0200 Subject: [PATCH 4/8] REST API: Regenerate client fixtures for finalize sub_sizes arg Moving sideload metadata writing to the finalize endpoint adds a sub_sizes argument to that endpoint's schema. Regenerate tests/qunit/fixtures/wp-api-generated.js so the "fixtures are up to date" CI guard (git diff --exit-code) passes. --- tests/qunit/fixtures/wp-api-generated.js | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/qunit/fixtures/wp-api-generated.js b/tests/qunit/fixtures/wp-api-generated.js index fa03d9751fe99..6d9a8054435e0 100644 --- a/tests/qunit/fixtures/wp-api-generated.js +++ b/tests/qunit/fixtures/wp-api-generated.js @@ -3733,6 +3733,43 @@ mockedApiResponse.Schema = { "description": "Unique identifier for the attachment.", "type": "integer", "required": false + }, + "sub_sizes": { + "description": "Array of sub-size metadata collected from sideload responses.", + "type": "array", + "default": [], + "items": { + "type": "object", + "properties": { + "image_size": { + "type": "string", + "required": true + }, + "width": { + "type": "integer", + "minimum": 1 + }, + "height": { + "type": "integer", + "minimum": 1 + }, + "file": { + "type": "string" + }, + "mime_type": { + "type": "string", + "pattern": "^image/.*" + }, + "filesize": { + "type": "integer", + "minimum": 1 + }, + "original_image": { + "type": "string" + } + } + }, + "required": false } } } From 1b4114123166876bf8e61e577810c05dddfb01aa Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Sat, 27 Jun 2026 17:25:13 -0700 Subject: [PATCH 5/8] REST API: Add tests for the sideload convert_format boolean arg Backports the convert_format test coverage from Gutenberg PR #77565. The convert_format argument and its handling already exist in core; this adds the missing test coverage, folding it into this sideload-finalize PR because the second test asserts the sideload response shape introduced here (a top-level file key on the lightweight sub-size response). Supersedes the separate backport PR for GB #77565, whose unique contribution was only these two tests. --- .../rest-api/rest-attachments-controller.php | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 53df60e4b3b45..8a84d2fbadfbf 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -3722,4 +3722,96 @@ public function test_finalize_preserves_image_meta(): void { $this->assertSame( $original_image_meta['focal_length'], $metadata['image_meta']['focal_length'], 'Focal length should be preserved.' ); $this->assertSame( $original_image_meta['iso'], $metadata['image_meta']['iso'], 'ISO should be preserved.' ); } + + /** + * Tests that the sideload route declares `convert_format` as a boolean arg. + * + * Without this declaration, multipart/form-data requests deliver the value as + * a string ("false") which evaluates truthy in PHP, so the sideload handler's + * `if ( ! $request['convert_format'] )` check never fires and the + * `image_editor_output_format` filter is never suppressed - meaning the + * server still performs the format conversion the client opted out of. + * + * @ticket 64737 + * @covers WP_REST_Attachments_Controller::register_routes + */ + public function test_sideload_route_declares_convert_format_boolean() { + $this->enable_client_side_media_processing(); + + $routes = rest_get_server()->get_routes(); + $endpoint = '/wp/v2/media/(?P[\d]+)/sideload'; + $this->assertArrayHasKey( $endpoint, $routes, 'Sideload route should exist.' ); + + $args = $routes[ $endpoint ][0]['args']; + + $this->assertArrayHasKey( 'convert_format', $args, 'Route should declare convert_format.' ); + $this->assertSame( 'boolean', $args['convert_format']['type'], 'convert_format should be a boolean.' ); + $this->assertTrue( $args['convert_format']['default'], 'convert_format should default to true.' ); + } + + /** + * Tests that sideloading with `convert_format=false` (sent as the string + * "false", matching multipart/form-data semantics) suppresses the + * alt-extension collision check in `wp_unique_filename()`, so a companion + * file sharing the attachment basename does not get a numeric suffix. + * + * Mirrors the HEIC companion upload flow: a JPEG derivative is created via + * the media endpoint, then the original is sideloaded under the same stem. + * Without the arg declared as boolean, "false" coerces truthy, the filter + * is never added, and the companion is bumped to `-1` while the JPEG stays + * unsuffixed. PNG stands in for HEIC because core's default + * `image_editor_output_format` only maps HEIC/HEIF to JPEG; a local filter + * adds a PNG to JPEG mapping to trigger the same alt-ext check. + * + * @ticket 64737 + * @covers WP_REST_Attachments_Controller::sideload_item + * @covers WP_REST_Attachments_Controller::register_routes + * @requires function imagejpeg + */ + public function test_sideload_convert_format_false_suppresses_alt_ext_suffix() { + $this->enable_client_side_media_processing(); + + wp_set_current_user( self::$author_id ); + + // Upload a JPEG "parent" attachment the way client-side uploads do. + $request = new WP_REST_Request( 'POST', '/wp/v2/media' ); + $request->set_header( 'Content-Type', 'image/jpeg' ); + $request->set_header( 'Content-Disposition', 'attachment; filename=heic-companion.jpg' ); + $request->set_param( 'generate_sub_sizes', false ); + $request->set_body( (string) file_get_contents( self::$test_file ) ); + + $response = rest_get_server()->dispatch( $request ); + $attachment_id = $response->get_data()['id']; + $this->assertSame( 201, $response->get_status() ); + + // Simulate an alt-ext conversion mapping so an alt-extension companion + // (PNG here, HEIC in production) would otherwise get a `-1` suffix. + $add_png_mapping = static function ( $formats ) { + $formats['image/png'] = 'image/jpeg'; + return $formats; + }; + add_filter( 'image_editor_output_format', $add_png_mapping, 5 ); + + // Sideload a companion sharing the same basename. Pass convert_format as + // the string "false" to match multipart/form-data request semantics. + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" ); + $request->set_header( 'Content-Type', 'image/png' ); + $request->set_header( 'Content-Disposition', 'attachment; filename=heic-companion.png' ); + $request->set_param( 'image_size', 'original' ); + $request->set_param( 'convert_format', 'false' ); + $request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/one-blue-pixel-100x100.png' ) ); + + $response = rest_get_server()->dispatch( $request ); + + remove_filter( 'image_editor_output_format', $add_png_mapping, 5 ); + + $this->assertSame( 200, $response->get_status() ); + + $data = $response->get_data(); + $this->assertSame( + 'heic-companion.png', + $data['file'], + 'Companion file should share the attachment basename without a numeric suffix.' + ); + } } From 295ea220ed9d6406141562815aed534504a292d4 Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Mon, 29 Jun 2026 10:21:18 -0700 Subject: [PATCH 6/8] Coding Standards: Use block comment style for multi-line comments Convert stacked // line comments into /* ... */ block comments for the multi-line explanatory comments added in this change. --- .../class-wp-rest-attachments-controller.php | 12 ++++++---- .../rest-api/rest-attachments-controller.php | 24 ++++++++++++------- 2 files changed, 23 insertions(+), 13 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 da53ce9dcd188..d0eb6a148bd10 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 @@ -2118,11 +2118,13 @@ public function sideload_item( WP_REST_Request $request ) { $image_size = $request['image_size']; - // Build sub-size data to return to the client. - // The client accumulates these and sends them all to the finalize - // endpoint, which writes the metadata in a single operation. This - // avoids the read-modify-write race that concurrent sideloads for the - // same attachment would otherwise hit. + /* + * Build sub-size data to return to the client. + * The client accumulates these and sends them all to the finalize + * endpoint, which writes the metadata in a single operation. This + * avoids the read-modify-write race that concurrent sideloads for the + * same attachment would otherwise hit. + */ $sub_size_data = array( 'image_size' => $image_size, ); diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 8a84d2fbadfbf..8cbfe3ecd89d0 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -3276,8 +3276,10 @@ public function test_sideload_scaled_image() { $this->assertSame( 200, $response->get_status(), 'Sideloading scaled image should succeed.' ); - // The sideload endpoint returns lightweight sub-size data; the metadata - // is written later by the finalize endpoint. + /* + * The sideload endpoint returns lightweight sub-size data; the metadata + * is written later by the finalize endpoint. + */ $sub_size = $response->get_data(); $this->assertSame( 'scaled', $sub_size['image_size'], 'Response should echo the image_size.' ); $this->assertSame( wp_basename( $original_file ), $sub_size['original_image'], 'Response original_image should be the basename of the original attached file.' ); @@ -3634,8 +3636,10 @@ public function test_finalize_writes_original_metadata(): void { $this->assertSame( 201, $response->get_status() ); - // Sideload the 'original' version (simulating a rotated image), which - // returns the basename without writing metadata. + /* + * Sideload the 'original' version (simulating a rotated image), which + * returns the basename without writing metadata. + */ $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" ); $request->set_header( 'Content-Type', 'image/jpeg' ); $request->set_header( 'Content-Disposition', 'attachment; filename=canola-original.jpg' ); @@ -3784,16 +3788,20 @@ public function test_sideload_convert_format_false_suppresses_alt_ext_suffix() { $attachment_id = $response->get_data()['id']; $this->assertSame( 201, $response->get_status() ); - // Simulate an alt-ext conversion mapping so an alt-extension companion - // (PNG here, HEIC in production) would otherwise get a `-1` suffix. + /* + * Simulate an alt-ext conversion mapping so an alt-extension companion + * (PNG here, HEIC in production) would otherwise get a `-1` suffix. + */ $add_png_mapping = static function ( $formats ) { $formats['image/png'] = 'image/jpeg'; return $formats; }; add_filter( 'image_editor_output_format', $add_png_mapping, 5 ); - // Sideload a companion sharing the same basename. Pass convert_format as - // the string "false" to match multipart/form-data request semantics. + /* + * Sideload a companion sharing the same basename. Pass convert_format as + * the string "false" to match multipart/form-data request semantics. + */ $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" ); $request->set_header( 'Content-Type', 'image/png' ); $request->set_header( 'Content-Disposition', 'attachment; filename=heic-companion.png' ); From 60a4c16ea90e1e78445476a55b7ef24518d299ee Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 1 Jul 2026 16:24:24 -0700 Subject: [PATCH 7/8] Tests: Use the source-format original size for the convert_format companion test The convert_format alt-ext-suffix test sideloaded a 100x100 PNG companion under image_size 'original' against a 640x480 JPEG parent. The sideload dimension validation added for the client-side-media route now rejects an 'original' whose dimensions do not match the attachment's recorded dimensions, so the request returned 400 instead of 200 and the test failed across the whole matrix. Sideload the companion as the source-format original size instead. A source-format companion kept beside its JPEG derivative is exactly what that size represents, and it is exempt from dimension validation, so the 100x100 PNG stand-in is accepted. The convert_format suppression the test exercises runs before the size branching, so the test's intent is unchanged. --- .../tests/rest-api/rest-attachments-controller.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 1aa489c2ccd0f..2d0751772d80f 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -4336,13 +4336,18 @@ public function test_sideload_convert_format_false_suppresses_alt_ext_suffix() { add_filter( 'image_editor_output_format', $add_png_mapping, 5 ); /* - * Sideload a companion sharing the same basename. Pass convert_format as - * the string "false" to match multipart/form-data request semantics. + * Sideload a companion sharing the same basename. Use the source-format + * original size: a source-format companion (HEIC in production, PNG + * here) kept beside its JPEG derivative is exactly what this size + * represents, and it is exempt from the sideload dimension validation + * that would otherwise reject a companion whose dimensions differ from + * the derivative. Pass convert_format as the string "false" to match + * multipart/form-data request semantics. */ $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" ); $request->set_header( 'Content-Type', 'image/png' ); $request->set_header( 'Content-Disposition', 'attachment; filename=heic-companion.png' ); - $request->set_param( 'image_size', 'original' ); + $request->set_param( 'image_size', WP_REST_Attachments_Controller::IMAGE_SIZE_SOURCE_ORIGINAL ); $request->set_param( 'convert_format', 'false' ); $request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/one-blue-pixel-100x100.png' ) ); From 98f5d48119f015108eb103ebec739ddf9471593b Mon Sep 17 00:00:00 2001 From: adamsilverstein Date: Wed, 1 Jul 2026 19:35:07 -0700 Subject: [PATCH 8/8] Tests: Point the convert_format sideload tests at Trac #65329 These tests ride on the sideload-metadata-to-finalize backport PR, which is tracked under #65329 (the ticket that owns this PR). Align their @ticket annotations with that ticket, matching the finalize tests already filed under #65329. --- tests/phpunit/tests/rest-api/rest-attachments-controller.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 2d0751772d80f..6884345376a9f 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -4273,7 +4273,7 @@ public function test_finalize_preserves_image_meta(): void { * `image_editor_output_format` filter is never suppressed - meaning the * server still performs the format conversion the client opted out of. * - * @ticket 64737 + * @ticket 65329 * @covers WP_REST_Attachments_Controller::register_routes */ public function test_sideload_route_declares_convert_format_boolean() { @@ -4304,7 +4304,7 @@ public function test_sideload_route_declares_convert_format_boolean() { * `image_editor_output_format` only maps HEIC/HEIF to JPEG; a local filter * adds a PNG to JPEG mapping to trigger the same alt-ext check. * - * @ticket 64737 + * @ticket 65329 * @covers WP_REST_Attachments_Controller::sideload_item * @covers WP_REST_Attachments_Controller::register_routes * @requires function imagejpeg