REST API: Address review feedback on HEIC client-side sideload support.

Harden the companion-original handling surfaced in review:

- Rename the companion metadata key from the over-generic 'original' to
  'source_image' so unrelated plugin or theme data stored under 'original'
  can no longer drive file deletion on attachment delete.
- Add IMAGE_SIZE_SOURCE_ORIGINAL and META_KEY_SOURCE_IMAGE class constants
  so the sideload image_size enum and its dispatch branch cannot drift.
- Drop the unused generate_sub_sizes argument from the /sideload route
  schema; only create_item() reads it, so advertising it on sideload
  silently misleads clients.
- Advertise the HEIC/HEIF -sequence variants in the REST index input
  formats so they match wp_is_heic_image_mime_type().
- Return a boolean from wp_delete_attachment_heic_companion_file() and
  strengthen the non-string guard test with a real on-disk bystander file
  so the regression it protects against can actually fail.

Author: adamsilverstein

src/wp-includes/media.php

@@ -5761,42 +5761,47 @@ function wp_show_heic_upload_error( $plupload_settings ) {
 }
 
 /**
- * Deletes the HEIC companion file when its attachment is deleted.
+ * Deletes the source-format companion file when its attachment is deleted.
  *
- * When the client-side media flow sideloads a HEIC original alongside a
- * JPEG derivative, the HEIC filename is recorded in $metadata['original'].
- * WordPress only tracks 'original_image' in wp_delete_attachment_files(),
- * so without this hook the HEIC file would linger on disk after the
- * attachment is deleted.
+ * When the client-side media flow sideloads a source-format original (such as
+ * a HEIC file) alongside a web-viewable derivative, the original's filename is
+ * recorded in the 'source_image' metadata key. WordPress only tracks
+ * 'original_image' in wp_delete_attachment_files(), so without this hook the
+ * companion file would linger on disk after the attachment is deleted.
  *
  * @since 7.1.0
  *
  * @param int $post_id Attachment ID being deleted.
+ * @return bool Whether a companion file was deleted.
  */
-function wp_delete_attachment_heic_companion_file( $post_id ): void {
+function wp_delete_attachment_heic_companion_file( $post_id ): bool {
 	$metadata = wp_get_attachment_metadata( $post_id, true );
 
-	if ( empty( $metadata['original'] ) || ! is_string( $metadata['original'] ) ) {
-		return;
+	if ( empty( $metadata['source_image'] ) || ! is_string( $metadata['source_image'] ) ) {
+		return false;
 	}
 
 	$attached_file = get_attached_file( $post_id, true );
 
 	if ( ! $attached_file ) {
-		return;
+		return false;
 	}
 
 	$uploads = wp_get_upload_dir();
 
 	if ( empty( $uploads['basedir'] ) ) {
-		return;
+		return false;
 	}
 
-	$heic_path = path_join( dirname( $attached_file ), wp_basename( $metadata['original'] ) );
+	$companion_path = path_join( dirname( $attached_file ), wp_basename( $metadata['source_image'] ) );
 
-	if ( file_exists( $heic_path ) ) {
-		wp_delete_file_from_directory( $heic_path, $uploads['basedir'] );
+	if ( ! file_exists( $companion_path ) ) {
+		return false;
 	}
+
+	wp_delete_file_from_directory( $companion_path, $uploads['basedir'] );
+
+	return true;
 }
 
 /**

src/wp-includes/rest-api/class-wp-rest-server.php

@@ -1380,7 +1380,7 @@ public function get_index( $request ) {
 			$available['image_size_threshold'] = (int) apply_filters( 'big_image_size_threshold', 2560, array( 0, 0 ), '', 0 );
 
 			// Image output formats.
-			$input_formats  = array( 'image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/avif', 'image/heic', 'image/heif' );
+			$input_formats  = array( 'image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/avif', 'image/heic', 'image/heif', 'image/heic-sequence', 'image/heif-sequence' );
 			$output_formats = array();
 			foreach ( $input_formats as $mime_type ) {
 				/** This filter is documented in wp-includes/media.php */

src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php

@@ -24,6 +24,29 @@ class WP_REST_Attachments_Controller extends WP_REST_Posts_Controller {
 	 */
 	protected $allow_batch = false;
 
+	/**
+	 * Image size token for the source-format original preserved alongside a
+	 * client-generated derivative (e.g. the HEIC file kept next to its JPEG).
+	 *
+	 * Used both in the `/sideload` route schema and when dispatching the
+	 * sideloaded file to its metadata key, so the two never drift apart.
+	 *
+	 * @since 7.1.0
+	 * @var string
+	 */
+	const IMAGE_SIZE_SOURCE_ORIGINAL = 'original-heic';
+
+	/**
+	 * Metadata key holding the basename of the source-format original.
+	 *
+	 * Deliberately specific so it never collides with the generic `original`
+	 * or `original_image` keys other flows write to.
+	 *
+	 * @since 7.1.0
+	 * @var string
+	 */
+	const META_KEY_SOURCE_IMAGE = 'source_image';
+
 	/**
 	 * Registers the routes for attachments.
 	 *
@@ -68,10 +91,12 @@ public function register_routes() {
 			$valid_image_sizes = array_keys( wp_get_registered_image_subsizes() );
 			// Special case to set 'original_image' in attachment metadata.
 			$valid_image_sizes[] = 'original';
-			// HEIC/HEIF companion original preserved alongside the JPEG derivative.
-			// Stored under its own meta key so it never collides with 'original'
-			// (which the scaled-sideload flow also writes to).
-			$valid_image_sizes[] = 'original-heic';
+			// Source-format original preserved alongside a client-generated
+			// derivative (e.g. the HEIC kept next to its JPEG). Stored under
+			// the dedicated self::META_KEY_SOURCE_IMAGE key so it never
+			// collides with 'original_image' (which the scaled-sideload flow
+			// also writes to).
+			$valid_image_sizes[] = self::IMAGE_SIZE_SOURCE_ORIGINAL;
 			// Used for PDF thumbnails.
 			$valid_image_sizes[] = 'full';
 			// Client-side big image threshold: sideload the scaled version.
@@ -86,26 +111,21 @@ public function register_routes() {
 						'callback'            => array( $this, 'sideload_item' ),
 						'permission_callback' => array( $this, 'sideload_item_permissions_check' ),
 						'args'                => array(
-							'id'                 => array(
+							'id'             => array(
 								'description' => __( 'Unique identifier for the attachment.' ),
 								'type'        => 'integer',
 							),
-							'image_size'         => array(
+							'image_size'     => array(
 								'description' => __( 'Image size.' ),
 								'type'        => 'string',
 								'enum'        => $valid_image_sizes,
 								'required'    => true,
 							),
-							'convert_format'     => array(
+							'convert_format' => array(
 								'type'        => 'boolean',
 								'default'     => true,
 								'description' => __( 'Whether to convert image formats.' ),
 							),
-							'generate_sub_sizes' => array(
-								'description' => __( 'Whether to generate image sub sizes from the sideloaded file.' ),
-								'type'        => 'boolean',
-								'default'     => false,
-							),
 						),
 					),
 					'allow_batch' => $this->allow_batch,
@@ -2110,13 +2130,13 @@ public function sideload_item( WP_REST_Request $request ) {
 
 		if ( 'original' === $image_size ) {
 			$metadata['original_image'] = wp_basename( $path );
-		} elseif ( 'original-heic' === $image_size ) {
-			// HEIC companion original: stored under its own meta key so
-			// the scaled-sideload flow (which writes 'original_image')
-			// cannot clobber it. 'original_image' keeps pointing at the
+		} elseif ( self::IMAGE_SIZE_SOURCE_ORIGINAL === $image_size ) {
+			// Source-format original: stored under its own meta key so the
+			// scaled-sideload flow (which writes 'original_image') cannot
+			// clobber it. 'original_image' keeps pointing at the
 			// web-viewable JPEG derivative. Cleanup on attachment delete
 			// is handled by wp_delete_attachment_heic_companion_file().
-			$metadata['original'] = wp_basename( $path );
+			$metadata[ self::META_KEY_SOURCE_IMAGE ] = wp_basename( $path );
 		} elseif ( 'scaled' === $image_size ) {
 			// The current attached file is the original; record it as original_image.
 			$current_file = get_attached_file( $attachment_id, true );

tests/phpunit/tests/media/wpDeleteAttachmentHeicCompanionFile.php

@@ -17,7 +17,7 @@ public function tear_down(): void {
 	/**
 	 * @ticket 64915
 	 */
-	public function test_deletes_heic_file_recorded_in_metadata_original(): void {
+	public function test_deletes_companion_file_recorded_in_metadata_source_image(): void {
 		$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
 		$this->assertIsInt( $attachment_id );
 
@@ -31,55 +31,66 @@ public function test_deletes_heic_file_recorded_in_metadata_original(): void {
 		file_put_contents( $heic_path, 'test' );
 		$this->assertFileExists( $heic_path, 'Test fixture should be on disk.' );
 
-		// Record the companion under metadata['original'] as the sideload route does.
+		// Record the companion under metadata['source_image'] as the sideload route does.
 		$metadata = wp_get_attachment_metadata( $attachment_id, true );
 		$this->assertIsArray( $metadata );
-		$metadata['original'] = $heic_name;
+		$metadata['source_image'] = $heic_name;
 		wp_update_attachment_metadata( $attachment_id, $metadata );
 
-		wp_delete_attachment( $attachment_id, true );
-
-		$this->assertFileDoesNotExist( $heic_path, 'Companion HEIC file should be deleted alongside the attachment.' );
+		$this->assertTrue(
+			wp_delete_attachment_heic_companion_file( $attachment_id ),
+			'Function should report that a companion file was deleted.'
+		);
+		$this->assertFileDoesNotExist( $heic_path, 'Companion file should be deleted alongside the attachment.' );
 	}
 
 	/**
 	 * @ticket 64915
 	 */
-	public function test_noop_when_metadata_original_is_missing(): void {
+	public function test_noop_when_metadata_source_image_is_missing(): void {
 		$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
 		$this->assertIsInt( $attachment_id );
 
-		// Sanity: no 'original' key on freshly-created metadata.
+		// Sanity: no 'source_image' key on freshly-created metadata.
 		$metadata = wp_get_attachment_metadata( $attachment_id, true );
 		$this->assertIsArray( $metadata );
-		$this->assertArrayNotHasKey( 'original', $metadata );
+		$this->assertArrayNotHasKey( 'source_image', $metadata );
+
+		// Should report no deletion and not raise even though the hook fires.
+		$this->assertFalse( wp_delete_attachment_heic_companion_file( $attachment_id ) );
 
-		// Should not raise even though the hook fires.
 		wp_delete_attachment( $attachment_id, true );
 
 		$this->assertNull( get_post( $attachment_id ) );
 	}
 
 	/**
-	 * Guards against $metadata['original'] holding a non-string value (e.g.
+	 * Guards against $metadata['source_image'] holding a non-string value (e.g.
 	 * the array form some flows write). Regression coverage for GB #78128.
 	 *
 	 * @ticket 64915
 	 */
-	public function test_noop_when_metadata_original_is_not_a_string(): void {
+	public function test_noop_when_metadata_source_image_is_not_a_string(): void {
 		$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
 		$this->assertIsInt( $attachment_id );
 		$attached_file = get_attached_file( $attachment_id, true );
 		$this->assertIsString( $attached_file );
 
+		// Place a real file that a buggy, guard-less implementation could try to
+		// delete after running wp_basename() over the array value below.
+		$bystander_path = dirname( $attached_file ) . '/should-not-delete.heic';
+		file_put_contents( $bystander_path, 'test' );
+		$this->assertFileExists( $bystander_path, 'Test fixture should be on disk.' );
+
 		$metadata = wp_get_attachment_metadata( $attachment_id, true );
 		$this->assertIsArray( $metadata );
-		$metadata['original'] = array( 'file' => 'should-not-delete.heic' );
+		$metadata['source_image'] = array( 'file' => 'should-not-delete.heic' );
 		wp_update_attachment_metadata( $attachment_id, $metadata );
 
-		// Should not raise (no path_join() / file_exists() on an array).
-		wp_delete_attachment_heic_companion_file( $attachment_id );
+		// Should report no deletion and not raise (no path_join() / file_exists() on an array).
+		$this->assertFalse( wp_delete_attachment_heic_companion_file( $attachment_id ) );
 
-		$this->assertFileExists( $attached_file, 'Attached file should still be on disk; the hook must bail on non-string original.' );
+		$this->assertFileExists( $bystander_path, 'The non-string guard must prevent any file deletion.' );
+		$this->assertFileExists( $attached_file, 'Attached file should still be on disk.' );
 	}
 }

tests/phpunit/tests/rest-api/rest-attachments-controller.php

@@ -3368,33 +3368,35 @@ public function test_sideload_route_includes_original_heic_enum(): void {
 	}
 
 	/**
-	 * Tests that the sideload endpoint exposes the generate_sub_sizes arg.
+	 * Tests that the sideload endpoint does not expose a generate_sub_sizes arg.
+	 *
+	 * sideload_item() never reads the parameter, so advertising it on the route
+	 * would silently mislead clients into expecting server-side sub-size
+	 * generation. The arg only does real work on create_item() (POST /wp/v2/media).
 	 *
 	 * @ticket 64915
 	 */
-	public function test_sideload_route_includes_generate_sub_sizes_arg(): void {
+	public function test_sideload_route_excludes_generate_sub_sizes_arg(): void {
 		$this->enable_client_side_media_processing();
 
 		$routes   = rest_get_server()->get_routes();
 		$endpoint = $routes['/wp/v2/media/(?P<id>[\d]+)/sideload'][0];
 		$args     = $endpoint['args'];
 		$this->assertIsArray( $args );
 
-		$this->assertArrayHasKey( 'generate_sub_sizes', $args, 'Route should have generate_sub_sizes arg.' );
-		$this->assertSame( 'boolean', $args['generate_sub_sizes']['type'], 'generate_sub_sizes should be a boolean.' );
-		$this->assertFalse( $args['generate_sub_sizes']['default'], 'generate_sub_sizes should default to false on sideload.' );
+		$this->assertArrayNotHasKey( 'generate_sub_sizes', $args, 'Sideload route should not advertise the unused generate_sub_sizes arg.' );
 	}
 
 	/**
 	 * Tests sideloading an 'original-heic' companion file alongside its JPEG
-	 * derivative. The HEIC filename is recorded under $metadata['original']
+	 * derivative. The HEIC filename is recorded under $metadata['source_image']
 	 * so it does not collide with 'original_image', which the scaled-sideload
 	 * flow owns.
 	 *
 	 * @ticket 64915
 	 * @requires function imagejpeg
 	 */
-	public function test_sideload_original_heic_writes_metadata_original(): void {
+	public function test_sideload_original_heic_writes_metadata_source_image(): void {
 		$this->enable_client_side_media_processing();
 
 		wp_set_current_user( self::$author_id );
@@ -3425,8 +3427,8 @@ public function test_sideload_original_heic_writes_metadata_original(): void {
 
 		$metadata = wp_get_attachment_metadata( $attachment_id );
 		$this->assertIsArray( $metadata );
-		$this->assertArrayHasKey( 'original', $metadata, "Metadata should contain 'original' for the HEIC companion." );
-		$this->assertMatchesRegularExpression( '/canola.*\.heic$/', $metadata['original'], "Metadata 'original' should reference the HEIC filename." );
+		$this->assertArrayHasKey( 'source_image', $metadata, "Metadata should contain 'source_image' for the HEIC companion." );
+		$this->assertMatchesRegularExpression( '/canola.*\.heic$/', $metadata['source_image'], "Metadata 'source_image' should reference the HEIC filename." );
 		$this->assertArrayNotHasKey( 'original_image', $metadata, "Metadata 'original_image' should be untouched by the HEIC sideload." );
 	}