media: Log and fail gracefully on invalid EXIF coordinates
authorThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Tue, 26 Nov 2019 08:54:05 +0000 (09:54 +0100)
committerReedy <reedy@wikimedia.org>
Tue, 17 Dec 2019 18:49:23 +0000 (18:49 +0000)
The $coord value is a value extracted from the EXIF section of an
image file. We expect it to be a float, but there is no guarantee this
is the case. It could, for example, be an empty string.

I suggest this trivial fix. It does have the following effects:
* Instead of logging a PHP notice when floor() hits something that is
  not a number, I try to log something that's more useful for later,
  more in-depth debugging. Note this log call isn't necessarily meant
  to stay, but to find an even better fix for this issue.
* I return the string as it is. If it's "foo", the user will see "foo"
  instead of "0° 0′ 0″ N", which wasn't helpful.

Also note how wrong and misleading the PHPDoc block for this function
was.

Bug: T226751
Change-Id: I1ca98728de4113ee1ae4362bd3e62b425d589388
(cherry picked from commit f6787ede2db29fcc2c1923e23eaa2e9bf86522a1)

includes/media/FormatMetadata.php

index 6cb33ac..2a5a56f 100644 (file)
@@ -1400,11 +1400,16 @@ class FormatMetadata extends ContextSource {
         * Format a coordinate value, convert numbers from floating point
         * into degree minute second representation.
         *
-        * @param int $coord Degrees, minutes and seconds
-        * @param string $type Latitude or longitude (for if its a NWS or E)
-        * @return mixed A floating point number or whatever we were fed
+        * @param float|string $coord Expected to be a number or numeric string in degrees
+        * @param string $type "latitude" or "longitude"
+        * @return string
         */
        private function formatCoords( $coord, $type ) {
+               if ( !is_numeric( $coord ) ) {
+                       wfDebugLog( 'exif', __METHOD__ . ": \"$coord\" is not a number" );
+                       return (string)$coord;
+               }
+
                $ref = '';
                if ( $coord < 0 ) {
                        $nCoord = -$coord;
@@ -1414,7 +1419,7 @@ class FormatMetadata extends ContextSource {
                                $ref = 'W';
                        }
                } else {
-                       $nCoord = $coord;
+                       $nCoord = (float)$coord;
                        if ( $type === 'latitude' ) {
                                $ref = 'N';
                        } elseif ( $type === 'longitude' ) {
@@ -1423,13 +1428,14 @@ class FormatMetadata extends ContextSource {
                }
 
                $deg = floor( $nCoord );
-               $min = floor( ( $nCoord - $deg ) * 60.0 );
-               $sec = round( ( ( $nCoord - $deg ) - $min / 60 ) * 3600, 2 );
+               $min = floor( ( $nCoord - $deg ) * 60 );
+               $sec = round( ( ( $nCoord - $deg ) * 60 - $min ) * 60, 2 );
 
                $deg = $this->formatNum( $deg );
                $min = $this->formatNum( $min );
                $sec = $this->formatNum( $sec );
 
+               // Note the default message "$1° $2′ $3″ $4" ignores the 5th parameter
                return $this->msg( 'exif-coordinate-format', $deg, $min, $sec, $ref, $coord )->text();
        }