diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index de5c08c8881..71cf355bd60 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -174,12 +174,15 @@ protected function get_body_node() { * @return float|int|string Returns a numeric dimension value, or an empty string. */ public function sanitize_dimension( $value, $dimension ) { - if ( empty( $value ) ) { + + // Allows 0 to be used as valid dimension. + if ( null === $value ) { return ''; } - if ( false !== filter_var( $value, FILTER_VALIDATE_INT ) ) { - return absint( $value ); + // Accepts both integers and floats & prevents negative values. + if ( is_numeric( $value ) ) { + return max( 0, floatval( $value ) ); } if ( AMP_String_Utils::endswith( $value, 'px' ) ) { @@ -252,7 +255,12 @@ public function enforce_sizes_attribute( $attributes ) { $max_width = $this->args['content_max_width']; } - $attributes['sizes'] = sprintf( '(min-width: %1$dpx) %1$dpx, 100vw', absint( $max_width ) ); + // Allows floats and integers but prevents negative values. + // Uses string format to prevent additional modification. + $attributes['sizes'] = sprintf( + '(min-width: %1$spx) %1$spx, 100vw', + max( 0, floatval( $max_width ) ) + ); $this->add_or_append_attribute( $attributes, 'class', 'amp-wp-enforced-sizes' ); diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 971e8d6cdec..86fe45e2770 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -79,7 +79,7 @@ public function sanitize() { } // Determine which images need their dimensions determined/extracted. - if ( '' === $node->getAttribute( 'width' ) || '' === $node->getAttribute( 'height' ) ) { + if ( ! is_numeric( $node->getAttribute( 'width' ) ) || ! is_numeric( $node->getAttribute( 'height' ) ) ) { $need_dimensions[ $node->getAttribute( 'src' ) ][] = $node; } else { $this->adjust_and_replace_node( $node ); @@ -154,18 +154,30 @@ private function determine_dimensions( $need_dimensions ) { if ( ! $node instanceof DOMElement ) { continue; } - - // Provide default dimensions for images whose dimensions we couldn't fetch. - if ( false !== $dimensions ) { - $node->setAttribute( 'width', $dimensions['width'] ); - $node->setAttribute( 'height', $dimensions['height'] ); - } else { - $width = isset( $this->args['content_max_width'] ) ? $this->args['content_max_width'] : self::FALLBACK_WIDTH; + if ( + ! is_numeric( $node->getAttribute( 'width' ) ) && + ! is_numeric( $node->getAttribute( 'height' ) ) + ) { $height = self::FALLBACK_HEIGHT; + $width = self::FALLBACK_WIDTH; $node->setAttribute( 'width', $width ); $node->setAttribute( 'height', $height ); $class = $node->hasAttribute( 'class' ) ? $node->getAttribute( 'class' ) . ' amp-wp-unknown-size' : 'amp-wp-unknown-size'; $node->setAttribute( 'class', $class ); + } elseif ( + ! is_numeric( $node->getAttribute( 'height' ) ) + ) { + $height = self::FALLBACK_HEIGHT; + $node->setAttribute( 'height', $height ); + $class = $node->hasAttribute( 'class' ) ? $node->getAttribute( 'class' ) . ' amp-wp-unknown-size amp-wp-unknown-height' : 'amp-wp-unknown-size amp-wp-unknown-height'; + $node->setAttribute( 'class', $class ); + } elseif ( + ! is_numeric( $node->getAttribute( 'width' ) ) + ) { + $width = self::FALLBACK_WIDTH; + $node->setAttribute( 'width', $width ); + $class = $node->hasAttribute( 'class' ) ? $node->getAttribute( 'class' ) . ' amp-wp-unknown-size amp-wp-unknown-width' : 'amp-wp-unknown-size amp-wp-unknown-width'; + $node->setAttribute( 'class', $class ); } } } diff --git a/tests/test-amp-img-sanitizer.php b/tests/test-amp-img-sanitizer.php index 2c437b7deff..c0cc0f10a3e 100644 --- a/tests/test-amp-img-sanitizer.php +++ b/tests/test-amp-img-sanitizer.php @@ -12,42 +12,72 @@ public function setUp() { public function get_data() { return array( - 'no_images' => array( + 'no_images' => array( '

Lorem Ipsum Demet Delorit.

', '

Lorem Ipsum Demet Delorit.

', ), - 'image_without_src' => array( + 'image_without_src' => array( '

', '

', ), - 'image_with_empty_src' => array( + 'image_with_empty_src' => array( '

', '

', ), - 'image_with_self_closing_tag' => array( + 'image_with_empty_width_and_height' => array( + '

', + '

', + ), + + 'image_with_empty_width' => array( + '

', + '

', + ), + + 'image_with_empty_height' => array( + '

', + '

', + ), + + 'image_with_zero_width' => array( + '

', + '

', + ), + + 'image_with_zero_width_and_height' => array( + '

', + '

', + ), + + 'image_with_decimal_width' => array( + '

', + '

', + ), + + 'image_with_self_closing_tag' => array( 'Placeholder!', '', ), - 'image_with_no_end_tag' => array( + 'image_with_no_end_tag' => array( 'Placeholder!', '', ), - 'image_with_end_tag' => array( + 'image_with_end_tag' => array( 'Placeholder!', '', ), - 'image_with_on_attribute' => array( + 'image_with_on_attribute' => array( '', '', ), - 'image_with_blacklisted_attribute' => array( + 'image_with_blacklisted_attribute' => array( '', '', ), @@ -62,17 +92,17 @@ public function get_data() { '', ), - 'gif_image_conversion' => array( + 'gif_image_conversion' => array( 'Placeholder!', '', ), - 'gif_image_url_with_querystring' => array( + 'gif_image_url_with_querystring' => array( 'Placeholder!', '', ), - 'multiple_same_image' => array( + 'multiple_same_image' => array( ' @@ -81,7 +111,7 @@ public function get_data() { '', ), - 'multiple_different_images' => array( + 'multiple_different_images' => array( '