Make WordPress Core

Changeset 52269


Ignore:
Timestamp:
11/29/2021 07:34:51 PM (2 years ago)
Author:
hellofromTonya
Message:

Media: Fix TypeError and improve wp_exif_frac2dec() to only return 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.

Location:
trunk
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/wp-admin/includes/image.php

    r51166 r52269  
    647647 * @since 2.5.0
    648648 *
    649  * @param string $str
    650  * @return int|float
     649 * @param string $str Fraction string.
     650 * @return int|float Returns calculated fraction or integer 0 on invalid input.
    651651 */
    652652function wp_exif_frac2dec( $str ) {
    653     if ( false === strpos( $str, '/' ) ) {
    654         return $str;
     653    if ( ! is_scalar( $str ) || is_bool( $str ) ) {
     654        return 0;
     655    }
     656
     657    if ( ! is_string( $str ) ) {
     658        return $str; // This can only be an integer or float, so this is fine.
     659    }
     660
     661    // Fractions passed as a string must contain a single `/`.
     662    if ( substr_count( $str, '/' ) !== 1 ) {
     663        if ( is_numeric( $str ) ) {
     664            return (float) $str;
     665        }
     666
     667        return 0;
    655668    }
    656669
    657670    list( $numerator, $denominator ) = explode( '/', $str );
    658     if ( ! empty( $denominator ) ) {
    659         return $numerator / $denominator;
    660     }
    661     return $str;
     671
     672    // Both the numerator and the denominator must be numbers.
     673    if ( ! is_numeric( $numerator ) || ! is_numeric( $denominator ) ) {
     674        return 0;
     675    }
     676
     677    // The denominator must not be zero.
     678    if ( 0 == $denominator ) { // phpcs:ignore WordPress.PHP.StrictComparisons.LooseComparison -- Deliberate loose comparison.
     679        return 0;
     680    }
     681
     682    return $numerator / $denominator;
    662683}
    663684
     
    841862            $meta['copyright'] = trim( $exif['Copyright'] );
    842863        }
    843         if ( ! empty( $exif['FNumber'] ) ) {
     864        if ( ! empty( $exif['FNumber'] ) && is_scalar( $exif['FNumber'] ) ) {
    844865            $meta['aperture'] = round( wp_exif_frac2dec( $exif['FNumber'] ), 2 );
    845866        }
     
    851872        }
    852873        if ( ! empty( $exif['FocalLength'] ) ) {
    853             $meta['focal_length'] = (string) wp_exif_frac2dec( $exif['FocalLength'] );
     874            $meta['focal_length'] = (string) $exif['FocalLength'];
     875            if ( is_scalar( $exif['FocalLength'] ) ) {
     876                $meta['focal_length'] = (string) wp_exif_frac2dec( $exif['FocalLength'] );
     877            }
    854878        }
    855879        if ( ! empty( $exif['ISOSpeedRatings'] ) ) {
     
    858882        }
    859883        if ( ! empty( $exif['ExposureTime'] ) ) {
    860             $meta['shutter_speed'] = (string) wp_exif_frac2dec( $exif['ExposureTime'] );
     884            $meta['shutter_speed'] = (string) $exif['ExposureTime'];
     885            if ( is_scalar( $exif['ExposureTime'] ) ) {
     886                $meta['shutter_speed'] = (string) wp_exif_frac2dec( $exif['ExposureTime'] );
     887            }
    861888        }
    862889        if ( ! empty( $exif['Orientation'] ) ) {
  • trunk/tests/phpunit/tests/image/functions.php

    r52248 r52269  
    701701        }
    702702    }
     703
     704    /**
     705     * Test for wp_exif_frac2dec verified that it properly handles edge cases
     706     * and always returns an int or float, or 0 for failures.
     707     *
     708     * @param mixed     $fraction The fraction to convert.
     709     * @param int|float $expect   The expected result.
     710     *
     711     * @ticket 54385
     712     * @dataProvider data_wp_exif_frac2dec
     713     *
     714     * @covers ::wp_exif_frac2dec
     715     */
     716    public function test_wp_exif_frac2dec( $fraction, $expect ) {
     717        $this->assertSame( $expect, wp_exif_frac2dec( $fraction ) );
     718    }
     719
     720    /**
     721     * Data provider for testing `wp_exif_frac2dec()`.
     722     *
     723     * @return array
     724     */
     725    public function data_wp_exif_frac2dec() {
     726        return array(
     727            'invalid input: null'              => array(
     728                'fraction' => null,
     729                'expect'   => 0,
     730            ),
     731            'invalid input: boolean true'      => array(
     732                'fraction' => null,
     733                'expect'   => 0,
     734            ),
     735            'invalid input: empty array value' => array(
     736                'fraction' => array(),
     737                'expect'   => 0,
     738            ),
     739            'input is already integer'         => array(
     740                'fraction' => 12,
     741                'expect'   => 12,
     742            ),
     743            'input is already float'           => array(
     744                'fraction' => 10.123,
     745                'expect'   => 10.123,
     746            ),
     747            'string input is not a fraction - no slash, not numeric' => array(
     748                'fraction' => '123notafraction',
     749                'expect'   => 0,
     750            ),
     751            'string input is not a fraction - no slash, numeric integer' => array(
     752                'fraction' => '48',
     753                'expect'   => 48.0,
     754            ),
     755            'string input is not a fraction - no slash, numeric integer (integer 0)' => array(
     756                'fraction' => '0',
     757                'expect'   => 0.0,
     758            ),
     759            'string input is not a fraction - no slash, octal numeric integer' => array(
     760                'fraction' => '010',
     761                'expect'   => 10.0,
     762            ),
     763            'string input is not a fraction - no slash, numeric float (float 0)' => array(
     764                'fraction' => '0.0',
     765                'expect'   => 0.0,
     766            ),
     767            'string input is not a fraction - no slash, numeric float (typical fnumber)' => array(
     768                'fraction' => '4.8',
     769                'expect'   => 4.8,
     770            ),
     771            'string input is not a fraction - more than 1 slash with text' => array(
     772                'fraction' => 'path/to/file',
     773                'expect'   => 0,
     774            ),
     775            'string input is not a fraction - more than 1 slash with numbers' => array(
     776                'fraction' => '1/2/3',
     777                'expect'   => 0,
     778            ),
     779            'string input is not a fraction - only a slash' => array(
     780                'fraction' => '/',
     781                'expect'   => 0,
     782            ),
     783            'string input is not a fraction - only slashes' => array(
     784                'fraction' => '///',
     785                'expect'   => 0,
     786            ),
     787            'string input is not a fraction - left/right is not numeric' => array(
     788                'fraction' => 'path/to',
     789                'expect'   => 0,
     790            ),
     791            'string input is not a fraction - left is not numeric' => array(
     792                'fraction' => 'path/10',
     793                'expect'   => 0,
     794            ),
     795            'string input is not a fraction - right is not numeric' => array(
     796                'fraction' => '0/abc',
     797                'expect'   => 0,
     798            ),
     799            'division by zero is prevented 1'  => array(
     800                'fraction' => '0/0',
     801                'expect'   => 0,
     802            ),
     803            'division by zero is prevented 2'  => array(
     804                'fraction' => '100/0.0',
     805                'expect'   => 0,
     806            ),
     807            'typical focal length'             => array(
     808                'fraction' => '37 mm',
     809                'expect'   => 0,
     810            ),
     811            'typical exposure time'            => array(
     812                'fraction' => '1/350',
     813                'expect'   => 0.002857142857142857,
     814            ),
     815            'valid fraction 1'                 => array(
     816                'fraction' => '50/100',
     817                'expect'   => 0.5,
     818            ),
     819            'valid fraction 2'                 => array(
     820                'fraction' => '25/100',
     821                'expect'   => .25,
     822            ),
     823            'valid fraction 3'                 => array(
     824                'fraction' => '4/2',
     825                'expect'   => 2,
     826            ),
     827        );
     828    }
    703829}
  • trunk/tests/phpunit/tests/image/meta.php

    r52010 r52269  
    3333        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/2004-07-22-DSC_0008.jpg' );
    3434
    35         $this->assertEquals( 6.3, $out['aperture'] );
    36         $this->assertSame( '', $out['credit'] );
    37         $this->assertSame( 'NIKON D70', $out['camera'] );
    38         $this->assertSame( '', $out['caption'] );
    39         $this->assertEquals( strtotime( '2004-07-22 17:14:59' ), $out['created_timestamp'] );
    40         $this->assertSame( '', $out['copyright'] );
    41         $this->assertEquals( 27, $out['focal_length'] );
    42         $this->assertEquals( 400, $out['iso'] );
    43         $this->assertEquals( 1 / 40, $out['shutter_speed'] );
    44         $this->assertSame( '', $out['title'] );
     35        $this->assertEquals( 6.3, $out['aperture'], 'Aperture value not equivalent' );
     36        $this->assertSame( '', $out['credit'], 'Credit value not the same' );
     37        $this->assertSame( 'NIKON D70', $out['camera'], 'Camera value not the same' );
     38        $this->assertSame( '', $out['caption'], 'Caption value not the same' );
     39        $this->assertEquals( strtotime( '2004-07-22 17:14:59' ), $out['created_timestamp'], 'Timestamp value not equivalent' );
     40        $this->assertSame( '', $out['copyright'], 'Copyright value not the same' );
     41        $this->assertEquals( 27, $out['focal_length'], 'Focal length value not equivalent' );
     42        $this->assertEquals( 400, $out['iso'], 'Iso value not equivalent' );
     43        $this->assertEquals( 1 / 40, $out['shutter_speed'], 'Shutter speed value not equivalent' );
     44        $this->assertSame( '', $out['title'], 'Title value not the same' );
    4545    }
    4646
     
    4949        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/2007-06-17DSC_4173.JPG' );
    5050
    51         $this->assertSame( '0', $out['aperture'] );
    52         $this->assertSame( '', $out['credit'] );
    53         $this->assertSame( 'NIKON D70', $out['camera'] );
    54         $this->assertSame( '', $out['caption'] );
    55         $this->assertEquals( strtotime( '2007-06-17 21:18:00' ), $out['created_timestamp'] );
    56         $this->assertSame( '', $out['copyright'] );
    57         $this->assertEquals( 0, $out['focal_length'] );
    58         $this->assertEquals( 0, $out['iso'] ); // Interesting - a Nikon bug?
    59         $this->assertEquals( 1 / 500, $out['shutter_speed'] );
    60         $this->assertSame( '', $out['title'] );
     51        $this->assertSame( '0', $out['aperture'], 'Aperture value not the same' );
     52        $this->assertSame( '', $out['credit'], 'Credit value not the same' );
     53        $this->assertSame( 'NIKON D70', $out['camera'], 'Camera value not the same' );
     54        $this->assertSame( '', $out['caption'], 'Caption value not the same' );
     55        $this->assertEquals( strtotime( '2007-06-17 21:18:00' ), $out['created_timestamp'], 'Timestamp value not equivalent' );
     56        $this->assertSame( '', $out['copyright'], 'Copyright value not the same' );
     57        $this->assertEquals( 0, $out['focal_length'], 'Focal length value not equivalent' );
     58        $this->assertEquals( 0, $out['iso'], 'Iso value not equivalent' ); // Interesting - a Nikon bug?
     59        $this->assertEquals( 1 / 500, $out['shutter_speed'], 'Shutter speed value not equivalent' );
     60        $this->assertSame( '', $out['title'], 'Title value not the same' );
    6161        // $this->assertSame( array( 'Flowers' ), $out['keywords'] );
    6262    }
     
    6666        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/2004-07-22-DSC_0007.jpg' );
    6767
    68         $this->assertEquals( 6.3, $out['aperture'] );
    69         $this->assertSame( 'IPTC Creator', $out['credit'] );
    70         $this->assertSame( 'NIKON D70', $out['camera'] );
    71         $this->assertSame( 'IPTC Caption', $out['caption'] );
    72         $this->assertEquals( strtotime( '2004-07-22 17:14:35' ), $out['created_timestamp'] );
    73         $this->assertSame( 'IPTC Copyright', $out['copyright'] );
    74         $this->assertEquals( 18, $out['focal_length'] );
    75         $this->assertEquals( 200, $out['iso'] );
    76         $this->assertEquals( 1 / 25, $out['shutter_speed'] );
    77         $this->assertSame( 'IPTC Headline', $out['title'] );
     68        $this->assertEquals( 6.3, $out['aperture'], 'Aperture value not equivalent' );
     69        $this->assertSame( 'IPTC Creator', $out['credit'], 'Credit value not the same' );
     70        $this->assertSame( 'NIKON D70', $out['camera'], 'Camera value not the same' );
     71        $this->assertSame( 'IPTC Caption', $out['caption'], 'Caption value not the same' );
     72        $this->assertEquals( strtotime( '2004-07-22 17:14:35' ), $out['created_timestamp'], 'Timestamp value not equivalent' );
     73        $this->assertSame( 'IPTC Copyright', $out['copyright'], 'Copyright value not the same' );
     74        $this->assertEquals( 18, $out['focal_length'], 'Focal length value not equivalent' );
     75        $this->assertEquals( 200, $out['iso'], 'Iso value not equivalent' );
     76        $this->assertEquals( 1 / 25, $out['shutter_speed'], 'Shutter speed value not equivalent' );
     77        $this->assertSame( 'IPTC Headline', $out['title'], 'Title value not the same' );
    7878    }
    7979
     
    8282        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/a2-small.jpg' );
    8383
    84         $this->assertEquals( 4.5, $out['aperture'] );
    85         $this->assertSame( '', $out['credit'] );
    86         $this->assertSame( 'FinePix S5600', $out['camera'] );
    87         $this->assertSame( '', $out['caption'] );
    88         $this->assertEquals( strtotime( '2007-09-03 10:17:03' ), $out['created_timestamp'] );
    89         $this->assertSame( '', $out['copyright'] );
    90         $this->assertEquals( 6.3, $out['focal_length'] );
    91         $this->assertEquals( 64, $out['iso'] );
    92         $this->assertEquals( 1 / 320, $out['shutter_speed'] );
    93         $this->assertSame( '', $out['title'] );
    94 
     84        $this->assertEquals( 4.5, $out['aperture'], 'Aperture value not equivalent' );
     85        $this->assertSame( '', $out['credit'], 'Credit value not the same' );
     86        $this->assertSame( 'FinePix S5600', $out['camera'], 'Camera value not the same' );
     87        $this->assertSame( '', $out['caption'], 'Caption value not the same' );
     88        $this->assertEquals( strtotime( '2007-09-03 10:17:03' ), $out['created_timestamp'], 'Timestamp value not equivalent' );
     89        $this->assertSame( '', $out['copyright'], 'Copyright value not the same' );
     90        $this->assertEquals( 6.3, $out['focal_length'], 'Focal length value not equivalent' );
     91        $this->assertEquals( 64, $out['iso'], 'Iso value not equivalent' );
     92        $this->assertEquals( 1 / 320, $out['shutter_speed'], 'Shutter speed value not equivalent' );
     93        $this->assertSame( '', $out['title'], 'Title value not the same' );
    9594    }
    9695
     
    103102        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/waffles.jpg' );
    104103
    105         $this->assertEquals( 0, $out['aperture'] );
    106         $this->assertSame( '', $out['credit'] );
    107         $this->assertSame( '', $out['camera'] );
    108         $this->assertSame( '', $out['caption'] );
    109         $this->assertEquals( 0, $out['created_timestamp'] );
    110         $this->assertSame( '', $out['copyright'] );
    111         $this->assertEquals( 0, $out['focal_length'] );
    112         $this->assertEquals( 0, $out['iso'] );
    113         $this->assertEquals( 0, $out['shutter_speed'] );
    114         $this->assertSame( '', $out['title'] );
     104        $this->assertEquals( 0, $out['aperture'], 'Aperture value not equivalent' );
     105        $this->assertSame( '', $out['credit'], 'Credit value not the same' );
     106        $this->assertSame( '', $out['camera'], 'Camera value not the same' );
     107        $this->assertSame( '', $out['caption'], 'Caption value not the same' );
     108        $this->assertEquals( 0, $out['created_timestamp'], 'Timestamp value not equivalent' );
     109        $this->assertSame( '', $out['copyright'], 'Copyright value not the same' );
     110        $this->assertEquals( 0, $out['focal_length'], 'Focal length value not equivalent' );
     111        $this->assertEquals( 0, $out['iso'], 'Iso value not equivalent' );
     112        $this->assertEquals( 0, $out['shutter_speed'], 'Shutter speed value not equivalent' );
     113        $this->assertSame( '', $out['title'], 'Title value not the same' );
    115114    }
    116115
     
    119118        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/canola.jpg' );
    120119
    121         $this->assertEquals( 0, $out['aperture'] );
    122         $this->assertSame( '', $out['credit'] );
    123         $this->assertSame( '', $out['camera'] );
    124         $this->assertSame( '', $out['caption'] );
    125         $this->assertEquals( 0, $out['created_timestamp'] );
    126         $this->assertSame( '', $out['copyright'] );
    127         $this->assertEquals( 0, $out['focal_length'] );
    128         $this->assertEquals( 0, $out['iso'] );
    129         $this->assertEquals( 0, $out['shutter_speed'] );
    130         $this->assertSame( '', $out['title'] );
     120        $this->assertEquals( 0, $out['aperture'], 'Aperture value not equivalent' );
     121        $this->assertSame( '', $out['credit'], 'Credit value not the same' );
     122        $this->assertSame( '', $out['camera'], 'Camera value not the same' );
     123        $this->assertSame( '', $out['caption'], 'Caption value not the same' );
     124        $this->assertEquals( 0, $out['created_timestamp'], 'Timestamp value not equivalent' );
     125        $this->assertSame( '', $out['copyright'], 'Copyright value not the same' );
     126        $this->assertEquals( 0, $out['focal_length'], 'Focal length value not equivalent' );
     127        $this->assertEquals( 0, $out['iso'], 'Iso value not equivalent' );
     128        $this->assertEquals( 0, $out['shutter_speed'], 'Shutter speed value not equivalent' );
     129        $this->assertSame( '', $out['title'], 'Title value not the same' );
    131130    }
    132131
     
    156155        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/33772.jpg' );
    157156
    158         $this->assertSame( '8', $out['aperture'] );
    159         $this->assertSame( 'Photoshop Author', $out['credit'] );
    160         $this->assertSame( 'DMC-LX2', $out['camera'] );
    161         $this->assertSame( 'Photoshop Description', $out['caption'] );
    162         $this->assertEquals( 1306315327, $out['created_timestamp'] );
    163         $this->assertSame( 'Photoshop Copyrright Notice', $out['copyright'] );
    164         $this->assertSame( '6.3', $out['focal_length'] );
    165         $this->assertSame( '100', $out['iso'] );
    166         $this->assertSame( '0.0025', $out['shutter_speed'] );
    167         $this->assertSame( 'Photoshop Document Ttitle', $out['title'] );
    168         $this->assertEquals( 1, $out['orientation'] );
    169         $this->assertSame( array( 'beach', 'baywatch', 'LA', 'sunset' ), $out['keywords'] );
     157        $this->assertSame( '8', $out['aperture'], 'Aperture value not the same' );
     158        $this->assertSame( 'Photoshop Author', $out['credit'], 'Credit value not the same' );
     159        $this->assertSame( 'DMC-LX2', $out['camera'], 'Camera value not the same' );
     160        $this->assertSame( 'Photoshop Description', $out['caption'], 'Caption value not the same' );
     161        $this->assertEquals( 1306315327, $out['created_timestamp'], 'Timestamp value not equivalent' );
     162        $this->assertSame( 'Photoshop Copyrright Notice', $out['copyright'], 'Copyright value not the same' );
     163        $this->assertSame( '6.3', $out['focal_length'], 'Focal length value not the same' );
     164        $this->assertSame( '100', $out['iso'], 'Iso value not the same' );
     165        $this->assertSame( '0.0025', $out['shutter_speed'], 'Shutter speed value not the same' );
     166        $this->assertSame( 'Photoshop Document Ttitle', $out['title'], 'Title value not the same' );
     167        $this->assertEquals( 1, $out['orientation'], 'Orientation value not equivalent' );
     168        $this->assertSame( array( 'beach', 'baywatch', 'LA', 'sunset' ), $out['keywords'], 'Keywords not the same' );
    170169    }
    171170
     
    240239        );
    241240    }
     241
     242    /**
     243     * @ticket 54385
     244     */
     245    public function test_exif_unexpected_data() {
     246        // Unexpected Exif data: FNumber is "0/0", aperture should be 0.
     247        $out = wp_read_image_metadata( DIR_TESTDATA . '/images/sugarloaf-mountain.jpg' );
     248
     249        $this->assertEquals( 0, $out['aperture'], 'Aperture value not equivalent' );
     250        $this->assertSame( '', $out['credit'], 'Credit value not the same' );
     251        $this->assertSame( 'X-T1', $out['camera'], 'Camera value not the same' );
     252        $this->assertSame( '', $out['caption'], 'Caption value not the same' );
     253        $this->assertEquals( 0, $out['created_timestamp'], 'Timestamp value not equivalent' );
     254        $this->assertSame( '', $out['copyright'], 'Copyright value not the same' );
     255        $this->assertEquals( 50, $out['focal_length'], 'Focal length value not equivalent' );
     256        $this->assertEquals( 200, $out['iso'], 'Iso value not equivalent' );
     257        $this->assertEquals( 2, $out['shutter_speed'], 'Shutter speed value not equivalent' );
     258        $this->assertSame( 'Sugarloaf Panorama', $out['title'], 'Title value not the same' );
     259    }
    242260}
Note: See TracChangeset for help on using the changeset viewer.