Skip to content

Commit

Permalink
Media: Fix TypeError and improve wp_exif_frac2dec() to only retur…
Browse files Browse the repository at this point in the history 
…n `int` or `float`.

For certain images, `wp_exif_frac2dec()` unexpectedly returned a string instead of `int` or `float`. This can occur when an image is missing meta and calls the function with `'0/0'`. For those images, a fatal error was thrown on PHP 8.0+:

{{{
TypeError: round(): Argument #1 ($num) must be of type int|float, string given
}}}

Upon deeper review, inconsistent and unexpected results were returned from different types of input values passed to the function.

Changes are:

* Maintains backwards-compatibility for valid input values.
* Fixes handling of invalid input values by bailing out to return the documented type of `int|float` by returning `0`.
* Improves the fractional conditional check.
* Improves the calculated fraction handling to ensure (a) the numerator and denominator are both numeric and (b) the denominator is not equal to zero.
* Safeguards the behavior via tests for all possible ways code could flow through the function.
* Safeguards the backwards-compatibility of the `wp_read_image_metadata()` by adding some defensive coding around the calls to the `wp_exif_frac2dec()` function.

These changes fix the fatal error and make the function more secure, stable, and predictable while maintaining backwards-compatibility for valid input values.

Follow-up to [6313], [9119], [22319], [28367], [45611], [47287].

Props adamsilverstein, jrf, peterwilsoncc, praem90, stevegs, tobiasbg.
Fixes #54385.

git-svn-id: https://develop.svn.wordpress.org/trunk@52269 602fd350-edb4-49c9-b593-d223f7449a82
  • Loading branch information
@hellofromtonya
hellofromtonya committed Nov 29, 2021
1 parent 4ff41ed commit 8aa625b
Show file tree
Hide file tree
Showing 4 changed files with 254 additions and 83 deletions.
47 changes: 37 additions & 10 deletions src/wp-admin/includes/image.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -646,19 +646,40 @@ function wp_generate_attachment_metadata( $attachment_id, $file ) {
*
* @since 2.5.0
*
* @param string $str
* @return int|float
* @param string $str Fraction string.
* @return int|float Returns calculated fraction or integer 0 on invalid input.
*/
function wp_exif_frac2dec( $str ) {
if ( false === strpos( $str, '/' ) ) {
return $str;
if ( ! is_scalar( $str ) || is_bool( $str ) ) {
return 0;
}

if ( ! is_string( $str ) ) {
return $str; // This can only be an integer or float, so this is fine.
}

// Fractions passed as a string must contain a single `/`.
if ( substr_count( $str, '/' ) !== 1 ) {
if ( is_numeric( $str ) ) {
return (float) $str;
}

return 0;
}

list( $numerator, $denominator ) = explode( '/', $str );
if ( ! empty( $denominator ) ) {
return $numerator / $denominator;

// Both the numerator and the denominator must be numbers.
if ( ! is_numeric( $numerator ) || ! is_numeric( $denominator ) ) {
return 0;
}

// The denominator must not be zero.
if ( 0 == $denominator ) { // phpcs:ignore WordPress.PHP.StrictComparisons.LooseComparison -- Deliberate loose comparison.
return 0;
}
return $str;

return $numerator / $denominator;
}

/**
Expand Down Expand Up @@ -840,7 +861,7 @@ function wp_read_image_metadata( $file ) {
if ( empty( $meta['copyright'] ) && ! empty( $exif['Copyright'] ) ) {
$meta['copyright'] = trim( $exif['Copyright'] );
}
if ( ! empty( $exif['FNumber'] ) ) {
if ( ! empty( $exif['FNumber'] ) && is_scalar( $exif['FNumber'] ) ) {
$meta['aperture'] = round( wp_exif_frac2dec( $exif['FNumber'] ), 2 );
}
if ( ! empty( $exif['Model'] ) ) {
Expand All @@ -850,14 +871,20 @@ function wp_read_image_metadata( $file ) {
$meta['created_timestamp'] = wp_exif_date2ts( $exif['DateTimeDigitized'] );
}
if ( ! empty( $exif['FocalLength'] ) ) {
$meta['focal_length'] = (string) wp_exif_frac2dec( $exif['FocalLength'] );
$meta['focal_length'] = (string) $exif['FocalLength'];
if ( is_scalar( $exif['FocalLength'] ) ) {
$meta['focal_length'] = (string) wp_exif_frac2dec( $exif['FocalLength'] );
}
}
if ( ! empty( $exif['ISOSpeedRatings'] ) ) {
$meta['iso'] = is_array( $exif['ISOSpeedRatings'] ) ? reset( $exif['ISOSpeedRatings'] ) : $exif['ISOSpeedRatings'];
$meta['iso'] = trim( $meta['iso'] );
}
if ( ! empty( $exif['ExposureTime'] ) ) {
$meta['shutter_speed'] = (string) wp_exif_frac2dec( $exif['ExposureTime'] );
$meta['shutter_speed'] = (string) $exif['ExposureTime'];
if ( is_scalar( $exif['ExposureTime'] ) ) {
$meta['shutter_speed'] = (string) wp_exif_frac2dec( $exif['ExposureTime'] );
}
}
if ( ! empty( $exif['Orientation'] ) ) {
$meta['orientation'] = $exif['Orientation'];
Expand Down
Empty file added tests/phpunit/data/images/sugarloaf-mountain.jpg
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
126 changes: 126 additions & 0 deletions tests/phpunit/tests/image/functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,4 +700,130 @@ public function test_pdf_preview_doesnt_overwrite_existing_jpeg() {
unlink( $temp_dir . $size['file'] );
}
}

/**
* Test for wp_exif_frac2dec verified that it properly handles edge cases
* and always returns an int or float, or 0 for failures.
*
* @param mixed $fraction The fraction to convert.
* @param int|float $expect The expected result.
*
* @ticket 54385
* @dataProvider data_wp_exif_frac2dec
*
* @covers ::wp_exif_frac2dec
*/
public function test_wp_exif_frac2dec( $fraction, $expect ) {
$this->assertSame( $expect, wp_exif_frac2dec( $fraction ) );
}

/**
* Data provider for testing `wp_exif_frac2dec()`.
*
* @return array
*/
public function data_wp_exif_frac2dec() {
return array(
'invalid input: null' => array(
'fraction' => null,
'expect' => 0,
),
'invalid input: boolean true' => array(
'fraction' => null,
'expect' => 0,
),
'invalid input: empty array value' => array(
'fraction' => array(),
'expect' => 0,
),
'input is already integer' => array(
'fraction' => 12,
'expect' => 12,
),
'input is already float' => array(
'fraction' => 10.123,
'expect' => 10.123,
),
'string input is not a fraction - no slash, not numeric' => array(
'fraction' => '123notafraction',
'expect' => 0,
),
'string input is not a fraction - no slash, numeric integer' => array(
'fraction' => '48',
'expect' => 48.0,
),
'string input is not a fraction - no slash, numeric integer (integer 0)' => array(
'fraction' => '0',
'expect' => 0.0,
),
'string input is not a fraction - no slash, octal numeric integer' => array(
'fraction' => '010',
'expect' => 10.0,
),
'string input is not a fraction - no slash, numeric float (float 0)' => array(
'fraction' => '0.0',
'expect' => 0.0,
),
'string input is not a fraction - no slash, numeric float (typical fnumber)' => array(
'fraction' => '4.8',
'expect' => 4.8,
),
'string input is not a fraction - more than 1 slash with text' => array(
'fraction' => 'path/to/file',
'expect' => 0,
),
'string input is not a fraction - more than 1 slash with numbers' => array(
'fraction' => '1/2/3',
'expect' => 0,
),
'string input is not a fraction - only a slash' => array(
'fraction' => '/',
'expect' => 0,
),
'string input is not a fraction - only slashes' => array(
'fraction' => '///',
'expect' => 0,
),
'string input is not a fraction - left/right is not numeric' => array(
'fraction' => 'path/to',
'expect' => 0,
),
'string input is not a fraction - left is not numeric' => array(
'fraction' => 'path/10',
'expect' => 0,
),
'string input is not a fraction - right is not numeric' => array(
'fraction' => '0/abc',
'expect' => 0,
),
'division by zero is prevented 1' => array(
'fraction' => '0/0',
'expect' => 0,
),
'division by zero is prevented 2' => array(
'fraction' => '100/0.0',
'expect' => 0,
),
'typical focal length' => array(
'fraction' => '37 mm',
'expect' => 0,
),
'typical exposure time' => array(
'fraction' => '1/350',
'expect' => 0.002857142857142857,
),
'valid fraction 1' => array(
'fraction' => '50/100',
'expect' => 0.5,
),
'valid fraction 2' => array(
'fraction' => '25/100',
'expect' => .25,
),
'valid fraction 3' => array(
'fraction' => '4/2',
'expect' => 2,
),
);
}
}
Loading

0 comments on commit 8aa625b

Please sign in to comment.