Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add guard to avoid fatal error in case logo height/width is zero #7429

Merged
merged 6 commits into from
Jan 30, 2023
Merged

Add guard to avoid fatal error in case logo height/width is zero #7429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff944f9
Add guard to avoid fatal error in case logo height/width is zero
thelovekesh Jan 27, 2023
4d07810
Update logic to bail only if logo height is zero
thelovekesh Jan 27, 2023
6f6b0ea
Add test cases to verify guards when logo height is zero
thelovekesh Jan 27, 2023
bc9bb9f
Remove vague ternary operator
thelovekesh Jan 28, 2023
f025b4b
Update condition to bail early in case of image with zero width
thelovekesh Jan 28, 2023
dea0db7
Update test cases to account zero as image width
thelovekesh Jan 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion includes/sanitizers/class-amp-core-theme-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,12 +591,20 @@ static function( $html ) {
return $html;
}

$img_width = (int) $src[1];
$img_height = (int) $src[2];

// If height is zero, bail.
if ( 0 === $img_height || 0 === $img_width ) {
return $html;
}

if ( 'blank' === get_header_textcolor() && has_custom_header() ) {
$height = 200;
} else {
$height = 80;
}
$width = $height * ( $src[1] / $src[2] ); // Note that float values are allowed.
$width = $height * ( $img_width / $img_height ); // Note that float values are allowed.

$html = preg_replace( '/(?<=width=")\d+(?=")/', $width, $html );
$html = preg_replace( '/(?<=height=")\d+(?=")/', $height, $html );
Expand Down Expand Up @@ -881,6 +889,11 @@ function ( $attr_name ) {
$width = (int) $matches['width'];
$height = (int) $matches['height'];

// If height is zero, bail.
if ( 0 === $height || 0 === $width ) {
return $html;
}

$desktop_height = 9; // in rem; see <https://github.com/WordPress/wordpress-develop/blob/ad8d01a7e9e13144d1676b8e6d70c3e81ef703af/src/wp-content/themes/twentytwenty/style.css#L4887>.
$mobile_height = 6; // in rem; see <https://github.com/WordPress/wordpress-develop/blob/ad8d01a7e9e13144d1676b8e6d70c3e81ef703af/src/wp-content/themes/twentytwenty/style.css#L1424>.

Expand Down
70 changes: 70 additions & 0 deletions tests/php/test-class-amp-core-theme-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,30 @@ public function test_add_twentyseventeen_attachment_image_attributes( $native_im
}
}

/**
* Test add_twentyseventeen_attachment_image_attributes() with zero height logo.
*
* @covers ::add_twentyseventeen_attachment_image_attributes()
*/
public function test_add_twentyseventeen_attachment_image_attributes_with_zero_height_width() {
$attachment_id = self::factory()->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg', 0 );

foreach ( [ 'height', 'width' ] as $dimension ) {
wp_update_attachment_metadata(
$attachment_id,
[
$dimension => 0,
]
);
set_theme_mod( 'custom_logo', $attachment_id );

AMP_Core_Theme_Sanitizer::add_twentyseventeen_attachment_image_attributes( [] );
$logo = get_custom_logo();

$this->assertStringNotContainsString( sprintf( '%s=', $dimension ), $logo );
}
}

/**
* @dataProvider get_data_for_using_native_img
* @covers::add_twentytwenty_masthead_styles()
Expand Down Expand Up @@ -488,6 +512,52 @@ static function () {
}
}

/**
* Get test data for different image sizes.
*
* @return array
*/
public function get_data_for_image_sizes() {
return [
'image_height_is_zaro' => [
0,
100,
],
'image_width_is_zaro' => [
100,
0,
],
'image_height_and_width_is_zaro' => [
0,
0,
],
];
}

/**
* Test add_twentytwenty_custom_logo_fix() with zero height logo.
*
* @dataProvider get_data_for_image_sizes
* @covers ::add_twentytwenty_custom_logo_fix()
*
* @param int $height Height.
* @param int $width Width.
*/
public function test_add_twentytwenty_custom_logo_fix_with_zero_height_width( $height, $width ) {
add_filter(
'get_custom_logo',
static function () use ( $height, $width ) {
return sprintf( '<img src="https://example.com/logo.jpg" width="%d" height="%d">', $width, $height );
}
);

AMP_Core_Theme_Sanitizer::add_twentytwenty_custom_logo_fix( [] );
$logo = get_custom_logo();

$this->assertStringContainsString( sprintf( 'width="%d"', $width ), $logo );
$this->assertStringContainsString( sprintf( 'height="%d"', $height ), $logo );
}

/**
* Tests prevent_sanitize_in_customizer_preview.
*
Expand Down