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 f336f321a9ea2..c17193acfc916 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 @@ -13,6 +13,16 @@ * @since 4.7.0 * * @see WP_REST_Posts_Controller + * + * @phpstan-type Image_Sub_Size array{ + * image_size: non-empty-string|non-empty-list, + * width?: positive-int, + * height?: positive-int, + * file?: non-empty-string, + * mime_type?: non-empty-string, + * filesize?: positive-int, + * original_image?: non-empty-string, + * } */ class WP_REST_Attachments_Controller extends WP_REST_Posts_Controller { @@ -47,6 +57,21 @@ class WP_REST_Attachments_Controller extends WP_REST_Posts_Controller { */ const META_KEY_SOURCE_IMAGE = 'source_image'; + /** + * Post meta key recording the file names produced by the sideload endpoint. + * + * Each successful sideload appends the file name(s) it created for an + * attachment under this key. The finalize endpoint reads them back to + * confirm every stored sub-size was actually produced here, rather than + * trusting a client-supplied name that could point at another attachment's + * files. Stored as one row per value (via {@see add_post_meta()}) so concurrent + * sideloads never read-modify-write a shared value. + * + * @since 7.1.0 + * @var string + */ + const META_KEY_SIDELOAD_FILE_NAME = '_wp_sideloaded_file'; + /** * Registers the routes for attachments. * @@ -105,8 +130,11 @@ public function register_routes() { 'description' => __( 'Image size. Can be a single size name or an array of size names to register the same file under multiple sizes.' ), 'type' => array( 'string', 'array' ), 'items' => array( - 'type' => 'string', + 'type' => 'string', + 'minLength' => 1, ), + 'minItems' => 1, + 'minLength' => 1, 'required' => true, /* * A custom callback is used instead of the default enum validation @@ -116,37 +144,18 @@ public function register_routes() { * the current list of registered sizes, which reflects sizes added * after route registration (e.g. via add_image_size()). */ - 'validate_callback' => static function ( $value, $request, $param ) { - $valid_sizes = array_keys( wp_get_registered_image_subsizes() ); - $valid_sizes[] = 'original'; - $valid_sizes[] = 'scaled'; - $valid_sizes[] = 'full'; - // Source-format original (e.g. the HEIC kept alongside its JPEG derivative). - $valid_sizes[] = self::IMAGE_SIZE_SOURCE_ORIGINAL; - // Converted-video companions for an animated GIF (the MP4/WebM and its poster). - $valid_sizes[] = 'animated_video'; - $valid_sizes[] = 'animated_video_poster'; - - $items = is_string( $value ) ? array( $value ) : ( is_array( $value ) ? $value : null ); - if ( null === $items ) { - return new WP_Error( - 'rest_invalid_type', - /* translators: %s: Parameter name. */ - sprintf( __( '%s must be a string or an array of strings.' ), $param ) - ); + 'validate_callback' => static function ( $value, WP_REST_Request $request, string $param ) { + /* + * Providing a custom callback replaces the default schema + * validation, so apply the declared schema (type, minLength, + * minItems) before the enum check below. + */ + $schema_validity = rest_validate_request_arg( $value, $request, $param ); + if ( is_wp_error( $schema_validity ) ) { + return $schema_validity; } - foreach ( $items as $item ) { - if ( ! is_string( $item ) || ! in_array( $item, $valid_sizes, true ) ) { - return new WP_Error( - 'rest_not_in_enum', - /* translators: %s: Parameter name. */ - sprintf( __( '%s contains an invalid image size.' ), $param ) - ); - } - } - - return true; + return self::validate_image_size_names( $value, $param ); }, ), 'convert_format' => array( @@ -175,10 +184,50 @@ public function register_routes() { 'type' => 'integer', ), 'sub_sizes' => array( - 'description' => __( 'Array of sub-size metadata collected from sideload responses.' ), - 'type' => 'array', - 'default' => array(), - 'items' => array( + 'description' => __( 'Array of sub-size metadata collected from sideload responses.' ), + 'type' => 'array', + 'default' => array(), + /* + * A finalize request sends one entry per sideloaded sub-size, so + * the ceiling only needs to clear the number of sizes a site can + * register. Bounding it keeps a request from repeating a name + * across an arbitrary number of entries. + */ + 'maxItems' => 100, + /* + * As on the sideload endpoint, the size names are checked in a + * callback rather than an enum, so the set reflects the sizes + * registered when the request runs. The callback sits on + * sub_sizes because a nested property cannot carry one. + */ + 'validate_callback' => static function ( $value, WP_REST_Request $request, string $param ) { + /* + * Providing a custom callback replaces the default schema + * validation, so apply the declared schema first. That is what + * guarantees each entry is an object carrying an image_size of + * the declared type. + */ + $schema_validity = rest_validate_request_arg( $value, $request, $param ); + if ( is_wp_error( $schema_validity ) ) { + return $schema_validity; + } + + foreach ( (array) $value as $index => $sub_size ) { + $sub_size = (array) $sub_size; + + $validity = self::validate_image_size_names( + $sub_size['image_size'] ?? null, + sprintf( '%s[%s][image_size]', $param, $index ) + ); + + if ( is_wp_error( $validity ) ) { + return $validity; + } + } + + return true; + }, + 'items' => array( 'type' => 'object', 'properties' => array( 'image_size' => array( @@ -186,7 +235,10 @@ public function register_routes() { 'type' => array( 'string', 'array' ), 'items' => array( 'type' => 'string', + 'minLength' => 1, ), + 'minItems' => 1, + 'minLength' => 1, 'required' => true, ), 'width' => array( @@ -198,7 +250,8 @@ public function register_routes() { 'minimum' => 1, ), 'file' => array( - 'type' => 'string', + 'type' => 'string', + 'minLength' => 1, ), 'mime_type' => array( 'type' => 'string', @@ -209,7 +262,8 @@ public function register_routes() { 'minimum' => 1, ), 'original_image' => array( - 'type' => 'string', + 'type' => 'string', + 'minLength' => 1, ), ), ), @@ -597,7 +651,7 @@ public function create_item( $request ) { * @param WP_REST_Request $request Full details about the request. * @return WP_REST_Response|WP_Error Response object on success, WP_Error object on failure. */ - protected function create_item_from_url( $request ) { + protected function create_item_from_url( WP_REST_Request $request ) { // Sideloading downloads and stores a file, so require the upload capability. if ( ! current_user_can( 'upload_files' ) ) { return new WP_Error( @@ -2462,6 +2516,96 @@ public function sideload_item_permissions_check( $request ) { return $this->edit_media_item_permissions_check( $request ); } + /** + * Validates an image size name, or an array of names sharing a single file. + * + * Shared by the sideload endpoint, which names the size a file is produced + * for, and the finalize endpoint, which names the size each submitted entry + * is stored under. Both need the same set, and finalize accepts a payload of + * its own rather than one this class produced, so leaving it unconstrained + * there would let a submission write an arbitrary key into the metadata + * 'sizes' array or route a file into a branch it was never produced for. + * + * @since 7.1.0 + * + * @param mixed $value The image size name, or an array of names. + * @param string $param Parameter name, used in the error messages. + * @return true|WP_Error True when every name is valid, WP_Error otherwise. + */ + private static function validate_image_size_names( $value, string $param ) { + $special_sizes = self::get_special_image_sizes(); + $regular_sizes = array_values( + array_diff( + array_merge( + array_keys( wp_get_registered_image_subsizes() ), + // Not a registered sub-size, but stored as an ordinary + // entry in the metadata 'sizes' array (PDF thumbnails). + array( 'full' ) + ), + $special_sizes + ) + ); + + if ( is_string( $value ) ) { + $items = array( $value ); + $valid_sizes = array_merge( $regular_sizes, $special_sizes ); + } elseif ( is_array( $value ) ) { + /** + * An array registers one sideloaded file under several size names, + * which only makes sense for regular sub-sizes: each special size + * names a single file with its own handling in + * {@see self::sideload_item()} and its own metadata key in + * {@see self::finalize_item()}. Rejecting them here is what lets the + * array branches in both methods treat an array as regular sizes. + */ + $items = $value; + $valid_sizes = $regular_sizes; + } else { + return new WP_Error( + 'rest_invalid_type', + /* translators: %s: Parameter name. */ + sprintf( __( '%s must be a string or an array of strings.' ), $param ) + ); + } + + foreach ( $items as $item ) { + if ( ! in_array( $item, $valid_sizes, true ) ) { + return new WP_Error( + 'rest_not_in_enum', + /* translators: %s: Parameter name. */ + sprintf( __( '%s contains an invalid image size.' ), $param ) + ); + } + } + + return true; + } + + /** + * Returns the image size names which name a single file rather than a sub-size. + * + * Each of these is handled on its own in {@see self::sideload_item()} and stored + * under its own key by {@see self::finalize_item()}, so unlike a regular + * sub-size none of them may appear in an array of names sharing one file. + * + * @since 7.1.0 + * + * @return string[] Special image size names. + * + * @phpstan-return non-empty-list + */ + private static function get_special_image_sizes(): array { + return array( + 'original', + 'scaled', + // Source-format original (e.g. the HEIC kept alongside its JPEG derivative). + self::IMAGE_SIZE_SOURCE_ORIGINAL, + // Converted-video companions for an animated GIF (the MP4/WebM and its poster). + 'animated_video', + 'animated_video_poster', + ); + } + /** * Validates that uploaded image dimensions are appropriate for the specified image size. * @@ -2625,6 +2769,26 @@ public function sideload_item( WP_REST_Request $request ) { ); } + /* + * Sideloaded files are placed in the same directory as the attachment + * they extend, because the file names produced here are later resolved + * against that directory. An attachment stored outside the uploads + * directory has no such directory to use, so there is nowhere the names + * this would produce could resolve. + */ + $attached_file = get_attached_file( $attachment_id, true ); + $subdir = is_string( $attached_file ) && '' !== $attached_file + ? $this->get_attachment_upload_subdir( $attached_file ) + : null; + + if ( ! is_string( $attached_file ) || '' === $attached_file || null === $subdir ) { + return new WP_Error( + 'rest_sideload_attachment_not_in_uploads', + __( 'The attachment is not stored in the uploads directory, so a file cannot be sideloaded for it.' ), + array( 'status' => 403 ) + ); + } + if ( false === $request['convert_format'] ) { // Prevent image conversion as that is done client-side. add_filter( 'image_editor_output_format', '__return_empty_array', 100 ); @@ -2639,8 +2803,7 @@ public function sideload_item( WP_REST_Request $request ) { * See /wp-includes/functions.php. * With the following filter we can work around this safeguard. */ - $attachment_filename = get_attached_file( $attachment_id, true ); - $attachment_filename = $attachment_filename ? wp_basename( $attachment_filename ) : null; + $attachment_filename = wp_basename( $attached_file ); $filter_filename = static function ( $filename, $ext, $dir, $unique_filename_callback, $alt_filenames, $number ) use ( $attachment_filename ) { return self::filter_wp_unique_filename( $filename, $dir, $number, $attachment_filename ); @@ -2648,24 +2811,34 @@ public function sideload_item( WP_REST_Request $request ) { add_filter( 'wp_unique_filename', $filter_filename, 10, 6 ); - $parent_post = get_post_parent( $attachment_id ); - - $time = null; + // Pin the upload to the attachment's own directory, rather than deriving + // it from the parent post's date as media_handle_upload() does for a + // brand new upload. See the note above where $subdir is resolved. + $filter_upload_dir = static function ( $uploads ) use ( $subdir ) { + if ( + is_array( $uploads ) && + isset( $uploads['basedir'], $uploads['baseurl'] ) && + is_string( $uploads['basedir'] ) && + is_string( $uploads['baseurl'] ) + ) { + $uploads['subdir'] = $subdir; + $uploads['path'] = $uploads['basedir'] . $subdir; + $uploads['url'] = $uploads['baseurl'] . $subdir; + } + return $uploads; + }; - // Matches logic in media_handle_upload(). - // The post date doesn't usually matter for pages, so don't backdate this upload. - if ( $parent_post && 'page' !== $parent_post->post_type && ! str_starts_with( $parent_post->post_date, '0000-00-00' ) ) { - $time = $parent_post->post_date; - } + add_filter( 'upload_dir', $filter_upload_dir, 100 ); if ( ! empty( $files ) ) { - $file = $this->upload_from_file( $files, $headers, $time ); + $file = $this->upload_from_file( $files, $headers ); } else { - $file = $this->upload_from_data( $request->get_body(), $headers, $time ); + $file = $this->upload_from_data( $request->get_body(), $headers ); } remove_filter( 'wp_unique_filename', $filter_filename ); remove_filter( 'image_editor_output_format', '__return_empty_array', 100 ); + remove_filter( 'upload_dir', $filter_upload_dir, 100 ); if ( is_wp_error( $file ) ) { return $file; @@ -2674,7 +2847,7 @@ public function sideload_item( WP_REST_Request $request ) { $type = $file['type']; $path = $file['file']; - /** @var non-empty-string $image_size */ + /** @var non-empty-string|non-empty-list $image_size */ $image_size = $request['image_size']; /* @@ -2688,7 +2861,8 @@ public function sideload_item( WP_REST_Request $request ) { * read and rejected if unreadable; validate_image_dimensions() skips * only the registered-size constraint for it. */ - $skip_dimension_read = in_array( $image_size, array( self::IMAGE_SIZE_SOURCE_ORIGINAL, 'animated_video' ), true ); + $skip_dimension_read = self::IMAGE_SIZE_SOURCE_ORIGINAL === $image_size || 'animated_video' === $image_size; + $size = false; if ( ! $skip_dimension_read ) { /* @@ -2709,13 +2883,14 @@ public function sideload_item( WP_REST_Request $request ) { } /* - * Validate the dimensions match the expected size. An array - * $image_size represents multiple registered sizes sharing a single - * file; those are handled by the per-size branch below, so only - * scalar sizes are validated here. + * Validate the dimensions against every size the file is being + * registered under. An array $image_size shares one file among + * several registered sizes, so the file has to satisfy each of + * them; validating only the scalar case would let a name wrapped + * in a one-element array skip the constraint entirely. */ - if ( ! is_array( $image_size ) ) { - $validation = $this->validate_image_dimensions( $size[0], $size[1], $image_size, $attachment_id ); + foreach ( (array) $image_size as $size_name ) { + $validation = $this->validate_image_dimensions( $size[0], $size[1], $size_name, $attachment_id ); if ( is_wp_error( $validation ) ) { // Clean up the uploaded file. wp_delete_file( $path ); @@ -2734,11 +2909,13 @@ public function sideload_item( WP_REST_Request $request ) { ); if ( is_array( $image_size ) ) { - // Multiple registered sizes share these dimensions, so a single - // sideloaded file is reused for all of them. Arrays only carry - // regular sub-sizes; the special keys below are always scalar. - $size = wp_getimagesize( $path ); - + /** + * Multiple registered sizes share these dimensions, so a single + * sideloaded file is reused for all of them. Arrays only carry + * regular sub-sizes; the special keys below are always scalar + * (ref. {@see self::get_special_image_sizes()}). Those never skip + * the read above, so $size already holds the dimensions. + */ $sub_size_data['width'] = $size ? $size[0] : 0; $sub_size_data['height'] = $size ? $size[1] : 0; $sub_size_data['file'] = wp_basename( $path ); @@ -2768,23 +2945,15 @@ public function sideload_item( WP_REST_Request $request ) { * makes when it scales or rotates an image on upload; see * _wp_image_meta_replace_original(). */ - $current_file = get_attached_file( $attachment_id, true ); - - if ( ! $current_file ) { - return new WP_Error( - 'rest_sideload_no_attached_file', - __( 'Unable to retrieve the attached file for this attachment.' ), - array( 'status' => 404 ) - ); - } - - $sub_size_data['original_image'] = wp_basename( $current_file ); + $sub_size_data['original_image'] = $attachment_filename; // Validate the supplied image before updating the attached file. - $size = wp_getimagesize( $path ); + // $size was read above: neither of these sizes skips that read. $filesize = wp_filesize( $path ); if ( ! $size || ! $filesize ) { + // Clean up the uploaded file, which nothing references yet. + wp_delete_file( $path ); return new WP_Error( 'rest_sideload_invalid_image', __( 'Unable to read the sideloaded image file.' ), @@ -2795,9 +2964,11 @@ public function sideload_item( WP_REST_Request $request ) { // Update the attached file to point to the supplied image. // This writes to _wp_attached_file meta, not _wp_attachment_metadata. if ( - get_attached_file( $attachment_id, true ) !== $path && + $attached_file !== $path && ! update_attached_file( $attachment_id, $path ) ) { + // Clean up the uploaded file, which nothing references yet. + wp_delete_file( $path ); return new WP_Error( 'rest_sideload_update_attached_file_failed', __( 'Unable to update the attached file for this attachment.' ), @@ -2810,8 +2981,7 @@ public function sideload_item( WP_REST_Request $request ) { $sub_size_data['filesize'] = $filesize; $sub_size_data['file'] = _wp_relative_upload_path( $path ); } else { - $size = wp_getimagesize( $path ); - + // As above, $size was already read for every size reaching here. $sub_size_data['width'] = $size ? $size[0] : 0; $sub_size_data['height'] = $size ? $size[1] : 0; $sub_size_data['file'] = wp_basename( $path ); @@ -2819,6 +2989,22 @@ public function sideload_item( WP_REST_Request $request ) { $sub_size_data['filesize'] = wp_filesize( $path ); } + /* + * Record the file names produced for this attachment so finalize can + * confirm every stored sub-size was actually sideloaded here. The + * values recorded are exactly the ones handed back to the client, so + * finalize accepts a submission only when it echoes what was produced. + */ + foreach ( array( 'file', 'original_image' ) as $provenance_key ) { + if ( + isset( $sub_size_data[ $provenance_key ] ) && + is_string( $sub_size_data[ $provenance_key ] ) && + '' !== $sub_size_data[ $provenance_key ] + ) { + add_post_meta( $attachment_id, self::META_KEY_SIDELOAD_FILE_NAME, wp_slash( $sub_size_data[ $provenance_key ] ) ); + } + } + return rest_ensure_response( $sub_size_data ); } @@ -2833,6 +3019,13 @@ public function sideload_item( WP_REST_Request $request ) { * However, here it is desired not to add the suffix in order to maintain the same * naming convention as if the file was uploaded regularly. * + * The suffix is only dropped when no file of that name already exists in $dir, + * so this never returns a name that would overwrite one. The unsuffixed name + * must also derive from the attachment's own file name, and + * {@see self::sideload_item()} pins the upload to the attachment's own + * directory, so any name returned here belongs to the attachment being + * extended. + * * @since 7.1.0 * * @link https://github.com/WordPress/wordpress-develop/blob/30954f7ac0840cfdad464928021d7f380940c347/src/wp-includes/functions.php#L2576-L2582 @@ -2868,6 +3061,175 @@ private static function filter_wp_unique_filename( $filename, $dir, $number, $at return $filename; } + /** + * Validates the `sub_sizes` file names against what this attachment produced. + * + * The {@see self::finalize_item()} method stores the client-supplied `file` + * and `original_image` values in the attachment metadata, where they are + * later resolved within the attachment's upload directory and read or deleted + * (for example by {@see wp_get_original_image_path()}, {@see wp_getimagesize()}, + * and {@see wp_delete_attachment_files()}). + * + * Every file the sideload endpoint creates is recorded under + * {@see self::META_KEY_SIDELOAD_FILE_NAME} as it is produced, using + * server-generated names. finalize accepts a `file` or `original_image` + * value only when it matches one of those recorded names (or the + * attachment's own attached file, which it definitionally owns). + * + * @since 7.1.0 + * + * @param int $attachment_id The attachment being finalized. + * @param array $sub_sizes Sub-size metadata collected from sideloads. + * @return true|WP_Error True if every file name was produced here, WP_Error otherwise. + * + * @phpstan-param list $sub_sizes + */ + protected function validate_sub_size_provenance( int $attachment_id, array $sub_sizes ) { + $allowed = $this->get_sideloaded_file_names( $attachment_id ); + + foreach ( $sub_sizes as $sub_size ) { + foreach ( array( 'file', 'original_image' ) as $key ) { + /* + * Every value that was sent is checked, no matter how unlikely + * a name it looks. A loose emptiness test would wave through + * '0', which is a valid one-character name as far as the schema + * is concerned and is stored like any other. A value the schema + * types as a string but which arrives as something else is + * rejected rather than skipped, so a subclass which widens the + * schema cannot pass an unchecked value on to the metadata. + */ + if ( ! isset( $sub_size[ $key ] ) ) { + continue; + } + + if ( ! is_string( $sub_size[ $key ] ) || ! in_array( $sub_size[ $key ], $allowed, true ) ) { + return new WP_Error( + 'rest_invalid_sub_size_file', + __( 'Invalid sub-size file name. File names must have been produced by a prior sideload for this attachment.' ), + array( 'status' => 400 ) + ); + } + } + } + + return true; + } + + /** + * Returns the file names which a finalize request may store for an attachment. + * + * The set is the file names the sideload endpoint recorded as it produced + * them (ref. {@see self::META_KEY_SIDELOAD_FILE_NAME}), plus the attachment's own + * attached file - accepted in both its uploads-relative and basename form so + * a scaled main-file pointer validates regardless of which the client + * echoes - plus the names already stored in the attachment's own metadata. + * + * @since 7.1.0 + * + * @param int $attachment_id The attachment being finalized. + * @param bool $include_provenance Whether to include the sideload provenance rows. + * Pass false to get only the names recoverable from + * the attached file and stored metadata, e.g. to decide + * whether a provenance row is still needed. Default true. + * @return string[] File names that may appear in the finalize submission. + * + * @phpstan-return list + */ + protected function get_sideloaded_file_names( int $attachment_id, bool $include_provenance = true ): array { + $allowed = array(); + + if ( $include_provenance ) { + foreach ( (array) get_post_meta( $attachment_id, self::META_KEY_SIDELOAD_FILE_NAME ) as $name ) { + if ( is_string( $name ) && '' !== $name ) { + $allowed[] = $name; + } + } + } + + $attached_file = get_post_meta( $attachment_id, '_wp_attached_file', true ); + if ( is_string( $attached_file ) && strlen( $attached_file ) > 0 ) { + $allowed[] = $attached_file; + $allowed[] = wp_basename( $attached_file ); + } + + /* + * Names already stored in this attachment's metadata passed this same + * check when they were written, so accepting them again introduces + * nothing new. + */ + $metadata = wp_get_attachment_metadata( $attachment_id, true ); + if ( is_array( $metadata ) ) { + $stored = array( + $metadata['file'] ?? null, + $metadata['original_image'] ?? null, + $metadata[ self::META_KEY_SOURCE_IMAGE ] ?? null, + $metadata['animated_video'] ?? null, + $metadata['animated_video_poster'] ?? null, + ); + + if ( ! empty( $metadata['sizes'] ) && is_array( $metadata['sizes'] ) ) { + foreach ( $metadata['sizes'] as $size ) { + $stored[] = is_array( $size ) ? ( $size['file'] ?? null ) : null; + } + } + + foreach ( $stored as $name ) { + if ( is_string( $name ) && '' !== $name ) { + $allowed[] = $name; + $allowed[] = wp_basename( $name ); + } + } + } + + return array_values( array_unique( $allowed ) ); + } + + /** + * Returns the uploads subdirectory an attachment is stored in. + * + * Used to place a sideloaded file alongside the attachment it extends. The + * result is concatenated into a filesystem path by the caller, so it is + * returned only when the attachment resolves inside the uploads directory + * and the stored path is well formed. + * + * @since 7.1.0 + * + * @param string $attached_file Absolute path to the attached file. + * @return string|null Subdirectory beginning with a slash, an empty string when the + * attachment sits in the base directory, or null when the + * attachment is not inside the uploads directory. + * + * @phpstan-param non-empty-string $attached_file + */ + protected function get_attachment_upload_subdir( string $attached_file ): ?string { + $uploads = wp_get_upload_dir(); + if ( empty( $uploads['basedir'] ) ) { + return null; + } + + $basedir = untrailingslashit( wp_normalize_path( $uploads['basedir'] ) ); + $file_dir = wp_normalize_path( dirname( $attached_file ) ); + + /* + * The attachment's directory must be the uploads base directory itself + * or a directory inside it. The trailing slash in the prefix comparison + * keeps a sibling directory that merely shares the prefix (for example + * 'uploads-elsewhere' next to 'uploads') from matching. + */ + if ( $file_dir !== $basedir && ! str_starts_with( $file_dir, trailingslashit( $basedir ) ) ) { + return null; + } + + $subdir = (string) substr( $file_dir, strlen( $basedir ) ); + + // A prefix match alone does not rule out a path that climbs back out. + if ( in_array( '..', explode( '/', $subdir ), true ) ) { + return null; + } + + return $subdir; + } + /** * Finalizes an attachment after client-side media processing. * @@ -2890,28 +3252,60 @@ public function finalize_item( WP_REST_Request $request ) { return $post; } + /** + * Sub-size metadata collected from sideload responses. Confirm every + * file name was produced by a prior sideload for this attachment before + * storing it, so a client cannot make finalize record (and later read or + * delete) another attachment's files. + * + * @var list $sub_sizes + */ + $sub_sizes = $request['sub_sizes'] ?? array(); + $provenance = $this->validate_sub_size_provenance( $attachment_id, $sub_sizes ); + if ( is_wp_error( $provenance ) ) { + return $provenance; + } + $metadata = wp_get_attachment_metadata( $attachment_id ); if ( ! is_array( $metadata ) ) { $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']; // When multiple size names share identical dimensions the client // sends a single sub-size entry with an array of names. Register the - // same file under each name. Arrays only contain regular sizes. + // same file under each name. if ( is_array( $image_size ) ) { + /* + * Arrays carry regular sizes only, as the sideload endpoint + * enforces. Each special size names a single file handled by one + * of the branches below, so grouping one under a shared file + * would write it to the wrong place; reject rather than guess. + */ + if ( array_intersect( $image_size, self::get_special_image_sizes() ) ) { + return new WP_Error( + 'rest_invalid_sub_size_name', + __( 'A grouped sub-size entry may only name regular image sizes.' ), + array( 'status' => 400 ) + ); + } + + // As below: `file` is not required by the schema, and a size + // entry that names no file is not worth recording. + if ( empty( $sub_size['file'] ) ) { + continue; + } + $metadata['sizes'] = $metadata['sizes'] ?? array(); foreach ( $image_size as $name ) { $metadata['sizes'][ $name ] = array( 'width' => $sub_size['width'] ?? 0, 'height' => $sub_size['height'] ?? 0, - 'file' => $sub_size['file'] ?? '', + 'file' => $sub_size['file'], 'mime-type' => $sub_size['mime_type'] ?? '', 'filesize' => $sub_size['filesize'] ?? 0, ); @@ -2953,6 +3347,12 @@ public function finalize_item( WP_REST_Request $request ) { $metadata['image_meta']['orientation'] = 1; } } elseif ( self::IMAGE_SIZE_SOURCE_ORIGINAL === $image_size ) { + // As above: `file` is not required by the schema, and each of + // these sizes is nothing but the file it names. + if ( empty( $sub_size['file'] ) ) { + continue; + } + /* * Source-format original: stored under its own meta key so the * scaled-sideload flow (which writes 'original_image') cannot @@ -2962,6 +3362,10 @@ public function finalize_item( WP_REST_Request $request ) { */ $metadata[ self::META_KEY_SOURCE_IMAGE ] = $sub_size['file']; } elseif ( 'animated_video' === $image_size ) { + if ( empty( $sub_size['file'] ) ) { + continue; + } + /* * Converted-video companion of an animated GIF. Stored under its * own meta key; 'original_image' keeps pointing at the GIF. Cleanup @@ -2969,15 +3373,23 @@ public function finalize_item( WP_REST_Request $request ) { */ $metadata['animated_video'] = $sub_size['file']; } elseif ( 'animated_video_poster' === $image_size ) { + if ( empty( $sub_size['file'] ) ) { + continue; + } + // Static first-frame poster for the converted video. $metadata['animated_video_poster'] = $sub_size['file']; } else { + if ( empty( $sub_size['file'] ) ) { + continue; + } + $metadata['sizes'] = $metadata['sizes'] ?? array(); $metadata['sizes'][ $image_size ] = array( 'width' => $sub_size['width'] ?? 0, 'height' => $sub_size['height'] ?? 0, - 'file' => $sub_size['file'] ?? '', + 'file' => $sub_size['file'], 'mime-type' => $sub_size['mime_type'] ?? '', 'filesize' => $sub_size['filesize'] ?? 0, ); @@ -2989,6 +3401,47 @@ public function finalize_item( WP_REST_Request $request ) { wp_update_attachment_metadata( $attachment_id, $metadata ); + /* + * Drop only the provenance rows this request consumed, now that the + * names are recorded in the metadata itself. A row is dropped only once + * its name is recoverable from the stored metadata, so a name the + * 'wp_generate_attachment_metadata' filter removed - or that a failed + * update never persisted - keeps its row and the retried request the + * endpoint documents as idempotent still validates. Rows for sideloads + * that have not been finalized yet survive for a later call, and passing + * the value makes the delete a no-op when the row is already gone, so a + * retried request cleans up without error. Any rows left behind by an + * abandoned upload are removed with the attachment itself. + * + * Retrying is idempotent for the request as it was sent. A name is only + * unavailable to a retry once a later finalize has overwritten the same + * size with a newly sideloaded file, which drops the earlier name from + * the metadata the retry recovers it from. + * + * The names are collected before deleting so a request which repeats + * the same name across many sub-sizes still issues one query per + * distinct name. + */ + $recoverable = $this->get_sideloaded_file_names( $attachment_id, false ); + $consumed = array(); + foreach ( $sub_sizes as $sub_size ) { + foreach ( array( 'file', 'original_image' ) as $key ) { + // Matches the set validate_sub_size_provenance() checked, so + // every name a request was allowed to store is also cleaned up. + if ( + isset( $sub_size[ $key ] ) && + is_string( $sub_size[ $key ] ) && + in_array( $sub_size[ $key ], $recoverable, true ) + ) { + $consumed[] = $sub_size[ $key ]; + } + } + } + + foreach ( array_unique( $consumed ) as $file_name ) { + delete_post_meta( $attachment_id, self::META_KEY_SIDELOAD_FILE_NAME, wp_slash( $file_name ) ); + } + $response_request = new WP_REST_Request( WP_REST_Server::READABLE, rest_get_route_for_post( $attachment_id ) diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 7f4dcd06c2f71..4dd0b60172cb4 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -4102,7 +4102,8 @@ public function test_sideload_scaled_image_requires_auth() { * The image_size argument accepts either a single size name or an array of * size names, so it validates via a custom callback rather than an enum. The * callback must accept 'scaled' and the 'source_original' source-format size, - * and reject unknown sizes. + * reject unknown sizes, and reject a special size sent as an array, since an + * array registers one file under several regular sub-sizes. * * sideload_item() never reads generate_sub_sizes, so advertising it on the * route would silently mislead clients into expecting server-side sub-size @@ -4145,13 +4146,21 @@ public function test_sideload_route_accepts_expected_image_sizes() { 'image_size validation should accept the source_original source-format size.' ); $this->assertTrue( - $validate( array( 'scaled' ), $request, $param_name ), + $validate( array( 'thumbnail', 'medium' ), $request, $param_name ), 'image_size validation should accept an array of size names.' ); + $this->assertTrue( + $validate( array( 'full', 'large' ), $request, $param_name ), + 'image_size validation should accept the full size grouped with a registered size.' + ); $this->assertWPError( $validate( 'not-a-real-size', $request, $param_name ), 'image_size validation should reject an unknown size.' ); + $this->assertWPError( + $validate( array( 'scaled' ), $request, $param_name ), + 'image_size validation should reject a special size sent as an array.' + ); $this->assertArrayNotHasKey( 'generate_sub_sizes', $args, 'Sideload route should not advertise the unused generate_sub_sizes arg.' ); } @@ -4991,20 +5000,19 @@ public function test_finalize_scaled_resets_orientation(): void { $metadata['image_meta']['orientation'] = 6; wp_update_attachment_metadata( $attachment_id, $metadata ); + // Sideload the client-scaled image so finalize has a provenance-backed + // 'scaled' entry to store. + $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-rotated-photo-scaled.jpg' ); + $request->set_param( 'image_size', 'scaled' ); + $request->set_body( (string) file_get_contents( self::$test_file ) ); + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 200, $response->get_status(), 'Sideloading the scaled image should succeed.' ); + $sub_size = $response->get_data(); + $request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" ); - $request->set_param( - 'sub_sizes', - array( - array( - 'image_size' => 'scaled', - 'width' => 1920, - 'height' => 2560, - 'file' => '2026/07/big-rotated-photo-scaled.jpg', - 'filesize' => 500000, - 'original_image' => 'big-rotated-photo.jpg', - ), - ) - ); + $request->set_param( 'sub_sizes', array( $sub_size ) ); $response = rest_get_server()->dispatch( $request ); $this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' ); @@ -5090,21 +5098,20 @@ public function test_finalize_preserves_image_meta(): void { $original_image_meta = wp_get_attachment_metadata( $attachment_id, true )['image_meta']; - // Finalize with a thumbnail sub-size. + // Sideload a thumbnail sub-size so finalize has a provenance-backed file + // to store. test-image.jpg is 50x50, within the thumbnail maximum. + $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=2004-07-22-DSC_0008-thumb.jpg' ); + $request->set_param( 'image_size', 'thumbnail' ); + $request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/test-image.jpg' ) ); + $response = rest_get_server()->dispatch( $request ); + $this->assertSame( 200, $response->get_status(), 'Sideloading a thumbnail should succeed.' ); + $sub_size = $response->get_data(); + + // Finalize with the sideloaded 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, - ), - ) - ); + $request->set_param( 'sub_sizes', array( $sub_size ) ); $response = rest_get_server()->dispatch( $request ); $this->assertSame( 200, $response->get_status(), 'Finalize should succeed.' ); @@ -5247,12 +5254,16 @@ public function test_sideload_image_size_array() { $this->assertSame( 201, $response->get_status() ); - // Sideload a single file registered under multiple sizes. + /* + * Sideload a single file registered under multiple sizes. The file is + * 50x50 so that it satisfies the registered maximum for every size in + * the group, which is what sharing one file among them requires. + */ $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-dup.jpg' ); $request->set_param( 'image_size', array( 'thumbnail', 'medium' ) ); - $request->set_body( (string) file_get_contents( self::$test_file ) ); + $request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/test-image.jpg' ) ); $response = rest_get_server()->dispatch( $request ); $this->assertSame( 200, $response->get_status(), 'Sideloading with an array of sizes should succeed.' ); diff --git a/tests/qunit/fixtures/wp-api-generated.js b/tests/qunit/fixtures/wp-api-generated.js index 1b0eaac6cccd1..4a2d5a3ac7ea8 100644 --- a/tests/qunit/fixtures/wp-api-generated.js +++ b/tests/qunit/fixtures/wp-api-generated.js @@ -3705,8 +3705,11 @@ mockedApiResponse.Schema = { "array" ], "items": { - "type": "string" + "type": "string", + "minLength": 1 }, + "minItems": 1, + "minLength": 1, "required": true }, "convert_format": { @@ -3739,6 +3742,7 @@ mockedApiResponse.Schema = { "description": "Array of sub-size metadata collected from sideload responses.", "type": "array", "default": [], + "maxItems": 100, "items": { "type": "object", "properties": { @@ -3749,8 +3753,11 @@ mockedApiResponse.Schema = { "array" ], "items": { - "type": "string" + "type": "string", + "minLength": 1 }, + "minItems": 1, + "minLength": 1, "required": true }, "width": { @@ -3762,7 +3769,8 @@ mockedApiResponse.Schema = { "minimum": 1 }, "file": { - "type": "string" + "type": "string", + "minLength": 1 }, "mime_type": { "type": "string", @@ -3773,7 +3781,8 @@ mockedApiResponse.Schema = { "minimum": 1 }, "original_image": { - "type": "string" + "type": "string", + "minLength": 1 } } },