Merge pull request #1445 from WordPress/auto-sizes/harden-attribute-l…

…ogic Harden logic to add `auto` keyword to `sizes` attribute to prevent duplicate keyword
WordPress · Aug 12, 2024 · aab5038 · aab5038
2 parents 3b49607 + 18f00cc
commit aab5038
Show file tree

Hide file tree

Showing 2 changed files with 132 additions and 4 deletions.
diff --git a/plugins/auto-sizes/hooks.php b/plugins/auto-sizes/hooks.php
@@ -34,7 +34,7 @@ function auto_sizes_update_image_attributes( $attr ): array {
 	}
 
 	// Don't add 'auto' to the sizes attribute if it already exists.
-	if ( str_contains( $attr['sizes'], 'auto,' ) ) {
+	if ( auto_sizes_attribute_includes_valid_auto( $attr['sizes'] ) ) {
 		return $attr;
 	}
 
@@ -63,12 +63,12 @@ function auto_sizes_update_content_img_tag( $html ): string {
 	}
 
 	// Bail early if the image is not responsive.
-	if ( false === strpos( $html, 'sizes="' ) ) {
+	if ( 1 !== preg_match( '/sizes="([^"]+)"/', $html, $match ) ) {
 		return $html;
 	}
 
-	// Don't double process content images.
-	if ( false !== strpos( $html, 'sizes="auto,' ) ) {
+	// Don't add 'auto' to the sizes attribute if it already exists.
+	if ( auto_sizes_attribute_includes_valid_auto( $match[1] ) ) {
 		return $html;
 	}
 
@@ -78,6 +78,21 @@ function auto_sizes_update_content_img_tag( $html ): string {
 }
 add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' );
 
+/**
+ * Checks whether the given 'sizes' attribute includes the 'auto' keyword as the first item in the list.
+ *
+ * Per the HTML spec, if present it must be the first entry.
+ *
+ * @since n.e.x.t
+ *
+ * @param string $sizes_attr The 'sizes' attribute value.
+ * @return bool True if the 'auto' keyword is present, false otherwise.
+ */
+function auto_sizes_attribute_includes_valid_auto( string $sizes_attr ): bool {
+	$token = strtok( strtolower( $sizes_attr ), ',' );
+	return false !== $token && 'auto' === trim( $token, " \t\f\r\n" );
+}
+
 /**
  * Displays the HTML generator tag for the plugin.
  *

diff --git a/plugins/auto-sizes/tests/test-auto-sizes.php b/plugins/auto-sizes/tests/test-auto-sizes.php
@@ -94,6 +94,119 @@ public function test_content_image_without_lazy_loading_does_not_have_auto_sizes
 		);
 	}
 
+	/**
+	 * Test generated markup for an image with 'auto' keyword already present in sizes does not receive it again.
+	 *
+	 * @covers ::auto_sizes_update_image_attributes
+	 * @covers ::auto_sizes_attribute_includes_valid_auto
+	 * @dataProvider data_image_with_existing_auto_sizes
+	 */
+	public function test_image_with_existing_auto_sizes_is_not_processed_again( string $initial_sizes, bool $expected_processed ): void {
+		$image_tag = wp_get_attachment_image(
+			self::$image_id,
+			'large',
+			false,
+			array(
+				// Force pre-existing 'sizes' attribute and lazy-loading.
+				'sizes'   => $initial_sizes,
+				'loading' => 'lazy',
+			)
+		);
+		if ( $expected_processed ) {
+			$this->assertStringContainsString( 'sizes="auto, ' . $initial_sizes . '"', $image_tag );
+		} else {
+			$this->assertStringContainsString( 'sizes="' . $initial_sizes . '"', $image_tag );
+		}
+	}
+
+	/**
+	 * Test content filtered markup with 'auto' keyword already present in sizes does not receive it again.
+	 *
+	 * @covers ::auto_sizes_update_content_img_tag
+	 * @covers ::auto_sizes_attribute_includes_valid_auto
+	 * @dataProvider data_image_with_existing_auto_sizes
+	 */
+	public function test_content_image_with_existing_auto_sizes_is_not_processed_again( string $initial_sizes, bool $expected_processed ): void {
+		// Force lazy loading attribute.
+		add_filter( 'wp_img_tag_add_loading_attr', '__return_true' );
+
+		add_filter(
+			'get_image_tag',
+			static function ( $html ) use ( $initial_sizes ) {
+				return str_replace(
+					'" />',
+					'" sizes="' . $initial_sizes . '" />',
+					$html
+				);
+			}
+		);
+
+		$image_content = wp_filter_content_tags( $this->get_image_tag( self::$image_id ) );
+		if ( $expected_processed ) {
+			$this->assertStringContainsString( 'sizes="auto, ' . $initial_sizes . '"', $image_content );
+		} else {
+			$this->assertStringContainsString( 'sizes="' . $initial_sizes . '"', $image_content );
+		}
+	}
+
+	/**
+	 * Returns data for the above test methods to assert correct behavior with a pre-existing sizes attribute.
+	 *
+	 * @return array<string, mixed[]> Arguments for the test scenarios.
+	 */
+	public function data_image_with_existing_auto_sizes(): array {
+		return array(
+			'not present'                 => array(
+				'(max-width: 1024px) 100vw, 1024px',
+				true,
+			),
+			'in beginning, without space' => array(
+				'auto,(max-width: 1024px) 100vw, 1024px',
+				false,
+			),
+			'in beginning, with space'    => array(
+				'auto, (max-width: 1024px) 100vw, 1024px',
+				false,
+			),
+			'sole keyword'                => array(
+				'auto',
+				false,
+			),
+			'with space before'           => array(
+				' auto, (max-width: 1024px) 100vw, 1024px',
+				false,
+			),
+			'with uppercase'              => array(
+				'AUTO, (max-width: 1024px) 100vw, 1024px',
+				false,
+			),
+
+			/*
+			 * The following scenarios technically include the 'auto' keyword,
+			 * but it is in the wrong place, as per the HTML spec it must be
+			 * the first entry in the list.
+			 * Therefore in these invalid cases the 'auto' keyword should still
+			 * be added to the beginning of the list.
+			 */
+			'within, without space'       => array(
+				'(max-width: 1024px) 100vw, auto,1024px',
+				true,
+			),
+			'within, with space'          => array(
+				'(max-width: 1024px) 100vw, auto, 1024px',
+				true,
+			),
+			'at the end, without space'   => array(
+				'(max-width: 1024px) 100vw,auto',
+				true,
+			),
+			'at the end, with space'      => array(
+				'(max-width: 1024px) 100vw, auto',
+				true,
+			),
+		);
+	}
+
 	/**
 	 * Test printing the meta generator tag.
 	 *