Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/wp-includes/default-filters.php
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@
add_action( 'transition_post_status', '_wp_customize_publish_changeset', 10, 3 );
add_action( 'admin_enqueue_scripts', '_wp_customize_loader_settings' );
add_action( 'delete_attachment', '_delete_attachment_theme_mod' );
add_action( 'delete_attachment', 'wp_delete_attachment_heic_companion_file' );
add_action( 'transition_post_status', '_wp_keep_alive_customize_changeset_dependent_auto_drafts', 20, 3 );

// Block Theme Previews.
Expand Down
39 changes: 39 additions & 0 deletions src/wp-includes/media.php
Original file line number Diff line number Diff line change
Expand Up @@ -5760,6 +5760,45 @@ function wp_show_heic_upload_error( $plupload_settings ) {
return $plupload_settings;
}

/**
* Deletes the HEIC 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.
*
* @since 7.1.0
*
* @param int $post_id Attachment ID being deleted.
*/
function wp_delete_attachment_heic_companion_file( $post_id ) {
Comment thread
westonruter marked this conversation as resolved.
Outdated
$metadata = wp_get_attachment_metadata( $post_id, true );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DB read on every delete_attachment, even for attachments that never touched HEIC.

wp_get_attachment_metadata() runs here on every delete_attachment action, only to immediately bail on empty( $metadata['original'] ) in the overwhelming majority of cases.

Bulk-deleting 10 000 non-HEIC attachments now triggers 10 000 extra _wp_attachment_metadata post-meta reads. A cheap mime-type pre-filter (get_post_mime_type( $post_id ) against HEIC-paired formats) on entry would avoid the work on the common path.

Altitude note (separate concern): the whole companion-file mechanism is a HEIC-specific bandaid bolted next to wp_show_heic_upload_error(). The right altitude is probably inside wp_delete_attachment_files() in post.php, e.g. a generic $metadata['extra_files'] = [ … ] array. The next format that wants a companion (AVIF originals alongside a JPEG fallback, RAW DNG sidecars, …) will otherwise need another bespoke hook + meta key. Worth weighing.

— 🤖 Claude Code

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the change outlined above will address this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key renamed in 7f976bb. Leaving the per-delete metadata read / move into wp_delete_attachment_files() as a follow-up — note the suggested mime pre-filter won't help here since the attachment is already JPEG post-conversion.


if ( empty( $metadata['original'] ) || ! is_string( $metadata['original'] ) ) {
return;
}

$attached_file = get_attached_file( $post_id, true );

if ( ! $attached_file ) {
return;
}

$uploads = wp_get_upload_dir();

if ( empty( $uploads['basedir'] ) ) {
return;
}

$heic_path = path_join( dirname( $attached_file ), wp_basename( (string) $metadata['original'] ) );
Comment thread
adamsilverstein marked this conversation as resolved.
Outdated

if ( file_exists( $heic_path ) ) {
wp_delete_file_from_directory( $heic_path, $uploads['basedir'] );
}
}

/**
* Allows PHP's getimagesize() to be debuggable when necessary.
*
Expand Down
2 changes: 1 addition & 1 deletion src/wp-includes/rest-api/class-wp-rest-server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
$input_formats = array( 'image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/avif', 'image/heic', 'image/heif' );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image/heic-sequence / image/heif-sequence not added to $input_formats.

The permissions bypass added in this PR uses wp_is_heic_image_mime_type(), which accepts four mime types — including the two -sequence variants (Live Photos / sequences). This $input_formats array adds only image/heif alongside image/heic.

Result: a image/heic-sequence upload passes the permission check (because wp_is_heic_image_mime_type() returns true), but the index endpoint advertises no output-format mapping for the sequence variants. The two lists describe the same surface and should agree.

— 🤖 Claude Code

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, would be good to make sure we have test coverage for these image types.

Copy link
Copy Markdown
Member Author

@adamsilverstein adamsilverstein May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7f976bb — added the -sequence variants to $input_formats.

$output_formats = array();
foreach ( $input_formats as $mime_type ) {
/** This filter is documented in wp-includes/media.php */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ 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';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic string 'original-heic' is duplicated between this enum registration and the dispatch branch at line 2113 with no shared constant. The metadata key 'original' is likewise duplicated across this controller and media.php.

Rename one site, miss the other, and the enum + handler disagree — sideloads of image_size=original-heic silently fall into the generic 'sizes' branch (or vice versa). A class constant (self::IMAGE_SIZE_ORIGINAL_HEIC = 'original-heic') eliminates the drift.

— 🤖 Claude Code

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will update

Copy link
Copy Markdown
Member Author

@adamsilverstein adamsilverstein May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7f976bb — extracted IMAGE_SIZE_SOURCE_ORIGINAL and META_KEY_SOURCE_IMAGE constants so the enum and dispatch can't drift.

// Used for PDF thumbnails.
$valid_image_sizes[] = 'full';
// Client-side big image threshold: sideload the scaled version.
Expand All @@ -82,21 +86,26 @@ 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(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_sub_sizes on /sideload is a no-op.

This arg is added to the sideload route schema, but sideload_item() never reads $request['generate_sub_sizes'] (grep across the function body confirms it). The arg only does real work in create_item() (POST /wp/v2/media).

Failure mode: a client POSTs to /wp/v2/media/{id}/sideload with generate_sub_sizes: true expecting server-side sub-size generation; it is silently ignored.

— 🤖 Claude Code

Copy link
Copy Markdown
Member Author

@adamsilverstein adamsilverstein May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7f976bb — removed from the sideload schema (QUnit fixture synced in df33a07).

'description' => __( 'Whether to generate image sub sizes from the sideloaded file.' ),
'type' => 'boolean',
'default' => false,
),
),
),
'allow_batch' => $this->allow_batch,
Expand Down Expand Up @@ -258,6 +267,17 @@ public function create_item_permissions_check( $request ) {
$prevent_unsupported_uploads = false;
}

// Always allow HEIC/HEIF uploads through even if the server's image
// editor doesn't support them. The client-side canvas fallback will
// handle processing using the browser's native HEVC decoder.
if (
$prevent_unsupported_uploads &&
! empty( $files['file']['type'] ) &&
wp_is_heic_image_mime_type( $files['file']['type'] )
) {
$prevent_unsupported_uploads = false;
}

// If the upload is an image, check if the server can handle the mime type.
if (
$prevent_unsupported_uploads &&
Expand Down Expand Up @@ -2090,6 +2110,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
// web-viewable JPEG derivative. Cleanup on attachment delete
// is handled by wp_delete_attachment_heic_companion_file().
$metadata['original'] = wp_basename( $path );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic metadata key + comment contradicts the code.

The companion HEIC filename is stored under $metadata['original'] — one of the most generic keys in attachment metadata. Any third-party plugin/theme that records a string filename under $metadata['original'] for an unrelated purpose will, on wp_delete_attachment, cause wp_delete_attachment_heic_companion_file() to delete the referenced file from the uploads dir. External data deciding what core deletes.

The comment at lines 2113–2118 (and its twin at line 72) even claims "Stored under its own meta key so it never collides with 'original'" — but the implementation writes to 'original' literally. Suggest renaming the key to something specific (e.g. original_heic) and matching the comment to the code.

— 🤖 Claude Code

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, right - i just commented on this above. this was changed during work on the PR and I agree it became too generic. I'll fix this with a less generic name, not specific to heic though so we can use it with other formats as needed.

Copy link
Copy Markdown
Member Author

@adamsilverstein adamsilverstein May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7f976bb — renamed the key to source_image and corrected the contradicting comments.

} elseif ( 'scaled' === $image_size ) {
// The current attached file is the original; record it as original_image.
$current_file = get_attached_file( $attachment_id, true );
Expand Down
77 changes: 77 additions & 0 deletions tests/phpunit/tests/media/wpDeleteAttachmentHeicCompanionFile.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?php

/**
* Tests for the `wp_delete_attachment_heic_companion_file()` function.
*
* @group media
* @covers ::wp_delete_attachment_heic_companion_file
*/
class Tests_Media_wpDeleteAttachmentHeicCompanionFile extends WP_UnitTestCase {

public function tear_down() {
$this->remove_added_uploads();

parent::tear_down();
}

/**
* @ticket 64915
*/
public function test_deletes_heic_file_recorded_in_metadata_original() {
$attachment_id = $this->factory->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );

$attached_file = get_attached_file( $attachment_id, true );
$dir = dirname( $attached_file );
$heic_name = 'companion-' . wp_generate_password( 6, false ) . '.heic';
$heic_path = $dir . '/' . $heic_name;

// Create a dummy companion file on disk.
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.
$metadata = wp_get_attachment_metadata( $attachment_id, true );
$metadata['original'] = $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.' );
}

/**
* @ticket 64915
*/
public function test_noop_when_metadata_original_is_missing() {
$attachment_id = $this->factory->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );

// Sanity: no 'original' key on freshly-created metadata.
$metadata = wp_get_attachment_metadata( $attachment_id, true );
$this->assertArrayNotHasKey( 'original', $metadata );

// 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.
* the array form some flows write). Regression coverage for GB #78128.
*
* @ticket 64915
*/
public function test_noop_when_metadata_original_is_not_a_string() {
$attachment_id = $this->factory->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
$attached_file = get_attached_file( $attachment_id, true );

$metadata = wp_get_attachment_metadata( $attachment_id, true );
$metadata['original'] = 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 );

$this->assertFileExists( $attached_file, 'Attached file should still be on disk; the hook must bail on non-string original.' );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion can't fail.

The test sets $metadata['original'] = array( 'file' => 'should-not-delete.heic' ) and then asserts that $attached_file (the JPEG) still exists. But the JPEG is unrelated to should-not-delete.heic, so the assertion passes even if the non-string guard at media.php:5779 is removed.

To actually cover the regression you're guarding against (someone simplifies the early return and trips a TypeError on wp_basename( array )), also file_put_contents() a real file named should-not-delete.heic next to the attachment and assert that file survives.

— 🤖 Claude Code

Copy link
Copy Markdown
Member Author

@adamsilverstein adamsilverstein May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7f976bb — the guard test now writes a real should-not-delete.heic and asserts it survives.

}
}
76 changes: 76 additions & 0 deletions tests/phpunit/tests/rest-api/rest-attachments-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -3351,6 +3351,82 @@ public function test_sideload_route_includes_scaled_enum() {
$this->assertContains( 'scaled', $args[ $param_name ]['enum'], 'image_size enum should include scaled.' );
}

/**
* Tests that the sideload endpoint includes 'original-heic' in the image_size enum.
*
* @ticket 64915
*/
public function test_sideload_route_includes_original_heic_enum() {
$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->assertArrayHasKey( 'image_size', $args, 'Route should have image_size arg.' );
$this->assertContains( 'original-heic', $args['image_size']['enum'], 'image_size enum should include original-heic.' );
}

/**
* Tests that the sideload endpoint exposes the generate_sub_sizes arg.
*
* @ticket 64915
*/
public function test_sideload_route_includes_generate_sub_sizes_arg() {
$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->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.' );
}

/**
* Tests sideloading an 'original-heic' companion file alongside its JPEG
* derivative. The HEIC filename is recorded under $metadata['original']
* 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() {
$this->enable_client_side_media_processing();

wp_set_current_user( self::$author_id );

// Create the JPEG attachment that the HEIC will be a companion to.
$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 ) );
$response = rest_get_server()->dispatch( $request );
$attachment_id = $response->get_data()['id'];

$this->assertSame( 201, $response->get_status() );

// Sideload the HEIC companion using the real HEIC fixture. `convert_format`
// is disabled so the default HEIC -> JPEG output mapping does not rename
// the file or append an alt-extension suffix.
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" );
$request->set_header( 'Content-Type', 'image/heic' );
$request->set_header( 'Content-Disposition', 'attachment; filename=canola.heic' );
$request->set_param( 'image_size', 'original-heic' );
$request->set_param( 'convert_format', false );
$request->set_body( file_get_contents( DIR_TESTDATA . '/images/test-image.heic' ) );
$response = rest_get_server()->dispatch( $request );

$this->assertSame( 200, $response->get_status(), 'Sideloading original-heic should succeed.' );

$metadata = wp_get_attachment_metadata( $attachment_id );
$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->assertArrayNotHasKey( 'original_image', $metadata, "Metadata 'original_image' should be untouched by the HEIC sideload." );
}

/**
* Tests the filter_wp_unique_filename method handles the -scaled suffix.
*
Expand Down
7 changes: 7 additions & 0 deletions tests/qunit/fixtures/wp-api-generated.js
Original file line number Diff line number Diff line change
Expand Up @@ -3703,6 +3703,7 @@ mockedApiResponse.Schema = {
"1536x1536",
"2048x2048",
"original",
"original-heic",
"full",
"scaled"
],
Expand All @@ -3713,6 +3714,12 @@ mockedApiResponse.Schema = {
"default": true,
"description": "Whether to convert image formats.",
"required": false
},
"generate_sub_sizes": {
"description": "Whether to generate image sub sizes from the sideloaded file.",
"type": "boolean",
"default": false,
"required": false
}
}
}
Expand Down
Loading