Page MenuHomePhabricator

GIFMetadataExtractor.php throws "Unexpected non-MediaWiki exception encountered, of type 'Exception'", halts if metadata in user-provided image not accepted
Closed, ResolvedPublic

Description

Author: carlb613

Description:
The error handling in GIFMetadataExtractor.php appears to create more problems than it solves. If the script finds the smallest bit of the metadata objectionable in a user-supplied image, it does not allow any part of the page on which the image appears to display.

Errors which could be safely ignored (for instance, the http://jpe.gs/wiki/File:Sucrose_b.gif file triggering an error at GIFMetadataExtractor.php:133) instead now prevent the display of the entire page, for every page in every wiki on which the image appears.

This issue appeared on 'upgrade' from MediaWiki 1.14 to the August 15, 2009 SVN version 1.16alpha - header in the metadata extractor file claims to be avoiding use of the MWException class for portability reasons, but even no displayed metadata at all would be better than entire wiki pages turned into:

Unexpected non-MediaWiki exception encountered, of type "Exception"
exception 'Exception' with message 'At position: 208, Unknown byte 11' in ...mw116a/includes/media/GIFMetadataExtractor.php:133
Stack trace:
#0 ...mw116a/includes/media/GIF.php(16): GIFMetadataExtractor::getMetadata('/var/im...')
#1 ...mw116a/includes/filerepo/UnregisteredLocalFile.php(89): GIFHandler->getMetadata(Object(UnregisteredLocalFile), '/var/im...')
#2 ...mw116a/includes/filerepo/File.php(899): UnregisteredLocalFile->getMetadata()
#3 ...mw116a/includes/ImagePage.php(90): File->formatMetadata()
#4 ...mw116a/includes/Wiki.php(463): ImagePage->view()
#5 ...mw116a/includes/Wiki.php(67): MediaWiki->performAction(Object(OutputPage), Object(ImagePage), Object(Title), Object(User), Object(WebRequest))
#6 ...mw116a/index.php(116): MediaWiki->performRequestForTitle(Object(Title), Object(ImagePage), Object(OutputPage), Object(User), Object(WebRequest))
#7 {main}

#0 ...mw116a/includes/media/GIF.php(16): GIFMetadataExtractor::getMetadata('/var/im...')
#1 ...mw116a/includes/filerepo/UnregisteredLocalFile.php(89): GIFHandler->getMetadata(Object(UnregisteredLocalFile), '/var/im...')
#2 ...mw116a/includes/filerepo/File.php(899): UnregisteredLocalFile->getMetadata()
#3 ...mw116a/includes/ImagePage.php(90): File->formatMetadata()
#4 ...mw116a/includes/Wiki.php(463): ImagePage->view()
#5 ...mw116a/includes/Wiki.php(67): MediaWiki->performAction(Object(OutputPage), Object(ImagePage), Object(Title), Object(User), Object(WebRequest))
#6 ...mw116a/index.php(116): MediaWiki->performRequestForTitle(Object(Title), Object(ImagePage), Object(OutputPage), Object(User), Object(WebRequest))
#7 {main}

Disabling the one "throw new Exception" line of code throwing the error (includes/media/GIFMetadataExtractor.php:133) allows the image to display normally:

} else {
        $byte = unpack( 'C', $buf );
        $byte = $byte[1];
        #throw new Exception( "At position: ".ftell($fh). ", Unknown byte ".$byte );
}

Oddly, the image itself looks fine in the browser. Whatever supposed 'error' exists in this animated .gif file is not valid enough that is should be used to justify MediaWiki 1.16a refusing to display the entire page, for every page where the image appears.

Any chance the code could be changed to recover just a wee bit more gracefully?


Version: 1.16.x
Severity: normal
OS: Linux
Platform: PC
URL: http://jpe.gs/wiki/File:Sucrose_b.gif

Details

Reference
bz20364

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:57 PM
bzimport set Reference to bz20364.
demon added a comment.Aug 23 2009, 2:52 PM

Assigning to Andrew.

Assigning to myself since I was poking at it. :)

Fixed in r55541.

There was a missing parameter in GIFMetadataExtractor's skipBlock() call for 'netscape 2.0' data blocks, which threw a monkey in the works. Now also checking for exceptions thrown by the metadata load and stubbing out null metadata for files which can't be read, rather than letting the exception bubble up and kill MediaWiki. :)

Gilles moved this task from Untriaged to Done on the Multimedia board.Dec 4 2014, 10:49 AM