From 946c629688ba18ba4693fcce4d4c38fa60cf1973 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Tue, 29 Apr 2025 20:56:34 +0100 Subject: [PATCH 1/7] Ensure width and height attributes are sanitized for images Added validation to sanitize 'width' and 'height' attributes before use and updated the default attributes accordingly. This improves security and prevents potential issues with invalid values in generated image markup. --- src/wp-includes/media.php | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index da46ef90a7dec..f05a802dfec5a 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -1070,7 +1070,6 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f list( $src, $width, $height ) = $image; $attachment = get_post( $attachment_id ); - $hwstring = image_hwstring( $width, $height ); $size_class = $size; if ( is_array( $size_class ) ) { @@ -1078,9 +1077,11 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f } $default_attr = array( - 'src' => $src, - 'class' => "attachment-$size_class size-$size_class", - 'alt' => trim( strip_tags( get_post_meta( $attachment_id, '_wp_attachment_image_alt', true ) ) ), + 'src' => $src, + 'class' => "attachment-$size_class size-$size_class", + 'alt' => trim( strip_tags( get_post_meta( $attachment_id, '_wp_attachment_image_alt', true ) ) ), + 'width' => $width, + 'height' => $height, ); /** @@ -1169,8 +1170,17 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f */ $attr = apply_filters( 'wp_get_attachment_image_attributes', $attr, $attachment, $size ); - $attr = array_map( 'esc_attr', $attr ); - $html = rtrim( " $value ) { $html .= " $name=" . '"' . $value . '"'; From 7dc00e9dee0d2267ae0855c7f9c7850c47084b13 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Tue, 29 Apr 2025 21:00:22 +0100 Subject: [PATCH 2/7] Optimize image loading attribute handling in `wp_get_attachment_image`. Simplified the code by directly passing `$attr` instead of creating and assigning a redundant `$loading_attr` array. This improves readability and eliminates unnecessary variable usage while maintaining functionality. --- src/wp-includes/media.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index f05a802dfec5a..d68cdba87dfbe 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -1094,12 +1094,9 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f $context = apply_filters( 'wp_get_attachment_image_context', 'wp_get_attachment_image' ); $attr = wp_parse_args( $attr, $default_attr ); - $loading_attr = $attr; - $loading_attr['width'] = $width; - $loading_attr['height'] = $height; $loading_optimization_attr = wp_get_loading_optimization_attributes( 'img', - $loading_attr, + $attr, $context ); From 78f42287cce7bfe546edea55c6ffbce89bd1ce89 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Tue, 29 Apr 2025 21:06:19 +0100 Subject: [PATCH 3/7] Normalize image attributes before unset Refactored the handling of `height` and `width` attributes by storing their sanitized values in separate variables before unsetting them. This aims to improve code readability and maintainability. --- src/wp-includes/media.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index d68cdba87dfbe..828bf5a207fbb 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -1168,11 +1168,11 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f $attr = apply_filters( 'wp_get_attachment_image_attributes', $attr, $attachment, $size ); if ( isset( $attr['height'] ) ) { - $attr['height'] = absint( $attr['height'] ); + $height = absint( $attr['height'] ); unset( $attr['height'] ); } if ( isset( $attr['width'] ) ) { - $attr['width'] = absint( $attr['width'] ); + $width = absint( $attr['width'] ); unset( $attr['width'] ); } $attr = array_map( 'esc_attr', $attr ); From 91b96daaa235ced4ab5749494e3a473e91a618b8 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Tue, 29 Apr 2025 21:33:47 +0100 Subject: [PATCH 4/7] Ensure height and width attributes are numeric before sanitizing. Previously, the height and width attributes were directly sanitized using `absint`, which could lead to unexpected behavior if non-numeric values were passed. This update adds a check to confirm the values are numeric before sanitizing, improving data validation and preventing potential errors. --- src/wp-includes/media.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index 828bf5a207fbb..8aa13f44ea14a 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -1168,11 +1168,15 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f $attr = apply_filters( 'wp_get_attachment_image_attributes', $attr, $attachment, $size ); if ( isset( $attr['height'] ) ) { - $height = absint( $attr['height'] ); + if ( is_numeric( $attr['height'] ) ) { + $height = absint( $attr['height'] ); + } unset( $attr['height'] ); } if ( isset( $attr['width'] ) ) { - $width = absint( $attr['width'] ); + if ( is_numeric( $attr['width'] ) ) { + $width = absint( $attr['width'] ); + } unset( $attr['width'] ); } $attr = array_map( 'esc_attr', $attr ); From 1b98fc4637449667e44ab3b9a32b5104a37ccef0 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Tue, 29 Apr 2025 22:43:54 +0100 Subject: [PATCH 5/7] Simplify height and width attribute handling in media.php. Consolidated logic for processing height and width attributes by combining conditions and reducing redundant checks. Unset operations for these attributes are now grouped for clarity. This improves code readability and maintainability. --- src/wp-includes/media.php | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index 8aa13f44ea14a..92f4b30c6a7a0 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -1167,18 +1167,13 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f */ $attr = apply_filters( 'wp_get_attachment_image_attributes', $attr, $attachment, $size ); - if ( isset( $attr['height'] ) ) { - if ( is_numeric( $attr['height'] ) ) { - $height = absint( $attr['height'] ); - } - unset( $attr['height'] ); + if ( isset( $attr['height'] ) && is_numeric( $attr['height'] ) ) { + $height = absint( $attr['height'] ); } - if ( isset( $attr['width'] ) ) { - if ( is_numeric( $attr['width'] ) ) { - $width = absint( $attr['width'] ); - } - unset( $attr['width'] ); + if ( isset( $attr['width'] ) && is_numeric( $attr['width'] ) ) { + $width = absint( $attr['width'] ); } + unset( $attr['height'], $attr['width'] ); $attr = array_map( 'esc_attr', $attr ); $hwstring = image_hwstring( $width, $height ); $html = rtrim( " Date: Thu, 29 May 2025 11:10:41 +0100 Subject: [PATCH 6/7] Include `width` and `height` in `$attr` after merging with defaults. Ensured `width` and `height` attributes are always set in the final `$attr` array for consistent handling of image dimensions. Improved code readability by aligning array keys. --- src/wp-includes/media.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/wp-includes/media.php b/src/wp-includes/media.php index 92f4b30c6a7a0..63b7fa671a42d 100644 --- a/src/wp-includes/media.php +++ b/src/wp-includes/media.php @@ -1077,11 +1077,9 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f } $default_attr = array( - 'src' => $src, - 'class' => "attachment-$size_class size-$size_class", - 'alt' => trim( strip_tags( get_post_meta( $attachment_id, '_wp_attachment_image_alt', true ) ) ), - 'width' => $width, - 'height' => $height, + 'src' => $src, + 'class' => "attachment-$size_class size-$size_class", + 'alt' => trim( strip_tags( get_post_meta( $attachment_id, '_wp_attachment_image_alt', true ) ) ), ); /** @@ -1091,8 +1089,10 @@ function wp_get_attachment_image( $attachment_id, $size = 'thumbnail', $icon = f * * @param string $context The context. Default 'wp_get_attachment_image'. */ - $context = apply_filters( 'wp_get_attachment_image_context', 'wp_get_attachment_image' ); - $attr = wp_parse_args( $attr, $default_attr ); + $context = apply_filters( 'wp_get_attachment_image_context', 'wp_get_attachment_image' ); + $attr = wp_parse_args( $attr, $default_attr ); + $attr['width'] = $width; + $attr['height'] = $height; $loading_optimization_attr = wp_get_loading_optimization_attributes( 'img', From a3e5ac08acdef081abad6ddddf49f942e183b638 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Sun, 22 Jun 2025 14:07:11 +0100 Subject: [PATCH 7/7] Add PHPUnit tests for `wp_get_attachment_image` attribute filters Introduced tests to validate the behavior of `wp_get_attachment_image_attributes` filter, ensuring proper handling of `width` and `height` attributes. Includes tests for default values, modified values, and unset scenarios to ensure consistent attribute handling. --- tests/phpunit/tests/media.php | 47 +++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/phpunit/tests/media.php b/tests/phpunit/tests/media.php index a1e70c2ff1a3c..902c704b39988 100644 --- a/tests/phpunit/tests/media.php +++ b/tests/phpunit/tests/media.php @@ -1583,6 +1583,53 @@ public function test_wp_get_attachment_image_filter_output() { $this->assertSame( $expected, $output ); } + /** + * @ticket 14110 + */ + public function test_wp_get_attachment_image_filter_with_height_width() { + $mock_action = new MockAction(); + add_filter( 'wp_get_attachment_image_attributes', array( $mock_action, 'filter' ) ); + wp_get_attachment_image( self::$large_id ); + $args = $mock_action->get_args(); + $this->assertArrayHasKey( '0', $args, 'First argument should be an array.' ); + $this->assertArrayHasKey( '0', $args[0], 'First argument should be an array.' ); + $this->assertArrayHasKey( 'width', $args[0][0], 'Width should be set.' ); + $this->assertArrayHasKey( 'height', $args[0][0], 'Height should be set.' ); + } + + /** + * @ticket 14110 + */ + public function test_wp_get_attachment_image_filter_change_height_width() { + add_filter( + 'wp_get_attachment_image_attributes', + static function ( $args ) { + $args['height'] = '999'; + $args['width'] = '999'; + return $args; + } + ); + $output = wp_get_attachment_image( self::$large_id ); + $this->assertStringContainsString( 'width="999"', $output, 'Width should be changed.' ); + $this->assertStringContainsString( 'height="999"', $output, 'Height should be changed.' ); + } + + /** + * @ticket 14110 + */ + public function test_wp_get_attachment_image_filter_unset_height_width() { + add_filter( + 'wp_get_attachment_image_attributes', + static function ( $args ) { + unset( $args['height'], $args['width'] ); + return $args; + } + ); + $output = wp_get_attachment_image( self::$large_id ); + $this->assertStringContainsString( 'width="150"', $output, 'Width should not be changed.' ); + $this->assertStringContainsString( 'height="150"', $output, 'Height should not be changed.' ); + } + public function filter_wp_get_attachment_image() { return 'Override wp_get_attachment_image'; }