Media: Cover client-side big image scaling and orphaned files

backport-changelog/7.1/12689.md

@@ -0,0 +1,3 @@
+https://github.com/WordPress/wordpress-develop/pull/12689
+
+* https://github.com/WordPress/gutenberg/pull/81061

lib/media/class-gutenberg-rest-attachments-controller.php

@@ -578,8 +578,10 @@ public function create_item( $request ) {
 			// Disable server-side EXIF rotation so the client can handle it.
 			// This preserves the original orientation value in the metadata.
 			add_filter( 'wp_image_maybe_exif_rotate', '__return_false', 100 );
-			// Disable server-side big image scaling since the client handles it.
-			add_filter( 'big_image_size_threshold', '__return_zero', 100 );
+			// Disable server-side "big image" downscaling; the client supplies its
+			// own scaled version via the sideload endpoint. Scaling here would
+			// create a conflicting "-scaled" file and orphan the full-size upload.
+			add_filter( 'big_image_size_threshold', '__return_false', 100 );
 		}
 
 		if ( ! $request['convert_format'] ) {
@@ -601,7 +603,7 @@ public function create_item( $request ) {
 		remove_filter( 'intermediate_image_sizes_advanced', '__return_empty_array', 100 );
 		remove_filter( 'fallback_intermediate_image_sizes', '__return_empty_array', 100 );
 		remove_filter( 'wp_image_maybe_exif_rotate', '__return_false', 100 );
-		remove_filter( 'big_image_size_threshold', '__return_zero', 100 );
+		remove_filter( 'big_image_size_threshold', '__return_false', 100 );
 		remove_filter( 'image_editor_output_format', '__return_empty_array', 100 );
 
 		// Recompute image_output_format now that __return_empty_array is removed.

phpunit/media/class-gutenberg-rest-attachments-controller-test.php

@@ -1185,6 +1185,134 @@ public function test_sideload_scaled_filename_not_suffixed() {
 		$this->assertStringNotContainsString( '-scaled-1', wp_basename( $attached_file ) );
 	}
 
+	/**
+	 * Verifies that the server leaves the uploaded full-size image untouched
+	 * when the client generates the sub-sizes.
+	 *
+	 * Scaling it here would create a `-scaled` file that the client's own
+	 * scaled sideload then collides with.
+	 *
+	 * @link https://core.trac.wordpress.org/ticket/65708
+	 *
+	 * @covers ::create_item
+	 */
+	public function test_create_item_does_not_scale_when_client_generates_sub_sizes() {
+		wp_set_current_user( self::$admin_id );
+
+		// canola.jpg is 640x480, so a 100px threshold would scale it down.
+		add_filter( 'big_image_size_threshold', array( $this, 'filter_low_big_image_size_threshold' ) );
+
+		$request = new WP_REST_Request( 'POST', '/wp/v2/media' );
+		$request->set_header( 'Content-Type', 'image/jpeg' );
+		$request->set_header( 'Content-Disposition', 'attachment; filename=big-photo.jpg' );
+		$request->set_param( 'generate_sub_sizes', false );
+		$request->set_body( file_get_contents( DIR_TESTDATA . '/images/canola.jpg' ) );
+		$response = rest_get_server()->dispatch( $request );
+
+		$this->assertSame( 201, $response->get_status() );
+		$attachment_id = $response->get_data()['id'];
+
+		$this->assertSame( 'big-photo.jpg', wp_basename( get_attached_file( $attachment_id, true ) ), 'The upload should be stored untouched.' );
+
+		$metadata = wp_get_attachment_metadata( $attachment_id, true );
+		$this->assertArrayNotHasKey( 'original_image', $metadata, 'No original_image should be recorded when the server does not scale the upload.' );
+	}
+
+	/**
+	 * Verifies that the full client-side flow for an image over the "big image"
+	 * threshold writes only files that the metadata tracks.
+	 *
+	 * When the server scales the upload as well, its full-size file is left
+	 * orphaned on disk, the client's scaled sideload is renamed `-scaled-1`,
+	 * and the sub-sizes inherit the numbered name.
+	 *
+	 * @link https://core.trac.wordpress.org/ticket/65708
+	 *
+	 * @covers ::create_item
+	 * @covers ::sideload_item
+	 * @covers ::finalize_item
+	 */
+	public function test_client_side_big_image_flow_leaves_no_orphaned_files() {
+		wp_set_current_user( self::$admin_id );
+
+		add_filter( 'big_image_size_threshold', array( $this, 'filter_low_big_image_size_threshold' ) );
+
+		$upload_dir   = wp_upload_dir();
+		$files_before = glob( $upload_dir['path'] . '/*' );
+
+		// 1. Upload the full-size image; the client owns all derivatives.
+		$request = new WP_REST_Request( 'POST', '/wp/v2/media' );
+		$request->set_header( 'Content-Type', 'image/jpeg' );
+		$request->set_header( 'Content-Disposition', 'attachment; filename=big-photo.jpg' );
+		$request->set_param( 'generate_sub_sizes', false );
+		$request->set_body( file_get_contents( DIR_TESTDATA . '/images/canola.jpg' ) );
+		$response = rest_get_server()->dispatch( $request );
+
+		$this->assertSame( 201, $response->get_status() );
+		$attachment_id = $response->get_data()['id'];
+
+		// 2. Sideload a thumbnail, as the client does for each missing size.
+		$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=big-photo-150x150.jpg' );
+		$request->set_param( 'image_size', 'thumbnail' );
+		$request->set_body( file_get_contents( DIR_TESTDATA . '/images/test-image.jpg' ) );
+		$response       = rest_get_server()->dispatch( $request );
+		$thumbnail_data = $response->get_data();
+
+		$this->assertSame( 200, $response->get_status() );
+		$this->assertSame( 'big-photo-150x150.jpg', $thumbnail_data['file'], 'The thumbnail should not inherit a numeric collision suffix.' );
+
+		// 3. Sideload the scaled full-size image.
+		$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=big-photo-scaled.jpg' );
+		$request->set_param( 'image_size', 'scaled' );
+		$request->set_body( file_get_contents( DIR_TESTDATA . '/images/canola.jpg' ) );
+		$response    = rest_get_server()->dispatch( $request );
+		$scaled_data = $response->get_data();
+
+		$this->assertSame( 200, $response->get_status() );
+
+		// 4. Finalize, writing the collected sub-size metadata in one pass.
+		$request = new WP_REST_Request( 'POST', "/wp/v2/media/$attachment_id/finalize" );
+		$request->set_param( 'sub_sizes', array( $thumbnail_data, $scaled_data ) );
+		$response = rest_get_server()->dispatch( $request );
+
+		$this->assertSame( 200, $response->get_status() );
+
+		$metadata = wp_get_attachment_metadata( $attachment_id, true );
+
+		$this->assertSame( 'big-photo.jpg', $metadata['original_image'], 'The untouched upload should be recorded as original_image.' );
+		$this->assertSame( 'big-photo-scaled.jpg', wp_basename( $metadata['file'] ), 'The scaled image should become the attached file.' );
+		$this->assertSame( 'big-photo-150x150.jpg', $metadata['sizes']['thumbnail']['file'], 'The thumbnail should keep its dimension-based name.' );
+
+		// Every file written for this attachment must be reachable from the
+		// metadata, otherwise it is orphaned on disk.
+		$written = array_map( 'wp_basename', array_diff( glob( $upload_dir['path'] . '/*' ), (array) $files_before ) );
+		sort( $written );
+		$this->assertSame(
+			array( 'big-photo-150x150.jpg', 'big-photo-scaled.jpg', 'big-photo.jpg' ),
+			$written,
+			'The flow should write only the full-size upload, its scaled copy, and the sub-sizes.'
+		);
+
+		// Deleting the attachment should clean all of them up.
+		wp_delete_attachment( $attachment_id, true );
+
+		$remaining = array_diff( glob( $upload_dir['path'] . '/*' ), (array) $files_before );
+		$this->assertSame( array(), array_values( $remaining ), 'Deleting the attachment should leave no files behind.' );
+	}
+
+	/**
+	 * Returns a "big image" threshold low enough that the test images exceed it.
+	 *
+	 * @return int Threshold in pixels.
+	 */
+	public function filter_low_big_image_size_threshold() {
+		return 100;
+	}
+
 	/**
 	 * Verifies that the sideload route declares `convert_format` as a boolean arg.
 	 *

test/e2e/specs/editor/various/big-image-size-threshold.spec.js

@@ -15,6 +15,16 @@ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );
  * @typedef {import('@playwright/test').Page} Page
  */
 
+/**
+ * Returns the file name of an upload-relative path such as `2026/08/image.jpeg`.
+ *
+ * @param {string} file Upload-relative file path.
+ * @return {string} File name.
+ */
+function wpBasename( file ) {
+	return file.split( '/' ).pop();
+}
+
 test.use( {
 	imageBlockUtils: async ( { page }, use ) => {
 		await use( new ImageBlockUtils( { page } ) );
@@ -67,7 +77,7 @@ test.describe( 'Big image size threshold', () => {
 		await expect( imageBlock ).toBeVisible();
 
 		// Upload a large image (3200x2400) that exceeds the default threshold (2560).
-		await imageBlockUtils.upload(
+		const fileName = await imageBlockUtils.upload(
 			imageBlock.locator( 'data-testid=form-file-upload-input' ),
 			'3200x2400_e2e_test_image_responsive_lightbox.jpeg'
 		);
@@ -100,49 +110,46 @@ test.describe( 'Big image size threshold', () => {
 					?.attributes?.id
 		);
 
-		if ( imageId ) {
-			// Fetch the attachment details from the REST API.
-			const media = await requestUtils.rest( {
-				method: 'GET',
-				path: `/wp/v2/media/${ imageId }`,
-			} );
+		expect( imageId ).toBeDefined();
 
-			// The image should be scaled down (either client-side or server-side).
-			expect( media.media_details.width ).toBeLessThanOrEqual( 2560 );
-			expect( media.media_details.height ).toBeLessThanOrEqual( 2560 );
+		// Fetch the attachment details from the REST API.
+		const media = await requestUtils.rest( {
+			method: 'GET',
+			path: `/wp/v2/media/${ imageId }`,
+		} );
 
-			if ( media.source_url.includes( '-scaled' ) ) {
-				// When client-side scaling adds the -scaled suffix,
-				// original_image may or may not be set depending on whether
-				// the server also processes the image. Only check if present.
-				if ( media.media_details.original_image ) {
-					expect( media.media_details.original_image ).toBeDefined();
-				}
+		// The image should be scaled down to the threshold.
+		expect( media.media_details.width ).toBeLessThanOrEqual( 2560 );
+		expect( media.media_details.height ).toBeLessThanOrEqual( 2560 );
+
+		/*
+		 * Only the scaled-down copy carries the `-scaled` suffix, and the
+		 * untouched upload is kept alongside it as `original_image`. If the
+		 * server also scales the upload while the client owns the derivatives,
+		 * the client's scaled file collides with the server's and is stored as
+		 * `-scaled-1`, the sub-sizes inherit the numbered name, and the
+		 * server's full-size file is orphaned on disk.
+		 * See https://core.trac.wordpress.org/ticket/65708.
+		 */
+		expect( wpBasename( media.media_details.file ) ).toBe(
+			`${ fileName }-scaled.jpeg`
+		);
+		expect( media.media_details.original_image ).toBe(
+			`${ fileName }.jpeg`
+		);
 
-				// Verify thumbnails were generated.
-				const sizes = media.media_details.sizes;
-				expect( sizes ).toBeDefined();
-
-				// Check that at least some standard sizes were created.
-				// The exact sizes depend on theme/site configuration.
-				const hasStandardSizes =
-					sizes.thumbnail || sizes.medium || sizes.large;
-				expect( hasStandardSizes ).toBeTruthy();
-
-				// If thumbnail exists, verify it has reasonable dimensions.
-				// Default thumbnail size is 150x150.
-				if ( sizes.thumbnail ) {
-					expect( sizes.thumbnail.width ).toBeLessThanOrEqual( 150 );
-					expect( sizes.thumbnail.height ).toBeLessThanOrEqual( 150 );
-				}
+		// Sub-sizes are named after the original, with no collision suffix.
+		// The `full` entry is the attached file itself, so it is skipped.
+		const sizes = Object.entries( media.media_details.sizes ).filter(
+			( [ name ] ) => name !== 'full'
+		);
+		expect( sizes.length ).toBeGreaterThan( 0 );
 
-				// If medium exists, verify dimensions.
-				// Default medium size is 300x300.
-				if ( sizes.medium ) {
-					expect( sizes.medium.width ).toBeLessThanOrEqual( 300 );
-					expect( sizes.medium.height ).toBeLessThanOrEqual( 300 );
-				}
-			}
+		for ( const [ name, size ] of sizes ) {
+			expect(
+				size.file,
+				`the "${ name }" sub-size should be named after the original`
+			).toBe( `${ fileName }-${ size.width }x${ size.height }.jpeg` );
 		}
 	} );
 
@@ -203,19 +210,20 @@ test.describe( 'Big image size threshold', () => {
 					?.attributes?.id
 		);
 
-		if ( imageId ) {
-			// Fetch the attachment details from the REST API.
-			const media = await requestUtils.rest( {
-				method: 'GET',
-				path: `/wp/v2/media/${ imageId }`,
-			} );
+		expect( imageId ).toBeDefined();
 
-			// The image should NOT be scaled since it's below the threshold.
-			expect( media.source_url ).not.toContain( '-scaled' );
-			// Original dimensions should be preserved.
-			expect( media.media_details.width ).toBe( 1024 );
-			expect( media.media_details.height ).toBe( 768 );
-		}
+		// Fetch the attachment details from the REST API.
+		const media = await requestUtils.rest( {
+			method: 'GET',
+			path: `/wp/v2/media/${ imageId }`,
+		} );
+
+		// The image should NOT be scaled since it's below the threshold.
+		expect( media.source_url ).not.toContain( '-scaled' );
+		expect( media.media_details.original_image ).toBeUndefined();
+		// Original dimensions should be preserved.
+		expect( media.media_details.width ).toBe( 1024 );
+		expect( media.media_details.height ).toBe( 768 );
 	} );
 } );