From: Brian Wolff Date: Thu, 5 Jan 2012 23:25:39 +0000 (+0000) Subject: Make sure that if we fail to read the App13 (iptc) block of a JPG file, that that... X-Git-Tag: 1.31.0-rc.0~25497 X-Git-Url: http://git.cyclocoop.org/%22%2C%20generer_url_ecrire%28?a=commitdiff_plain;h=f9173cb902cac38f75bf6525e0f8d861d50de8cc;p=lhc%2Fweb%2Fwiklou.git Make sure that if we fail to read the App13 (iptc) block of a JPG file, that that doesn't block other metadata from being read. Also makes sure if more then one app13 block is in the file, they are all read, not just the last one that appears in the file (This required some changes to tests since before the intermediate value was just one value, now its an array of all such blocks) --- diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19 index 2cb77f7d99..e073ce5f15 100644 --- a/RELEASE-NOTES-1.19 +++ b/RELEASE-NOTES-1.19 @@ -223,6 +223,8 @@ production. * (bug 33321) Adding a line to MediaWiki:Sidebar that contains a pipe, but doesn't have any pipes after being transformed by MessageCache, causes exception on all pages. +* Files with IPTC blocks we can't read no longer prevent extraction of exif + or other metadata. === API changes in 1.19 === * (bug 19838) siprop=interwikimap can now use the interwiki cache. diff --git a/includes/media/BitmapMetadataHandler.php b/includes/media/BitmapMetadataHandler.php index d1caa67a38..8aab9bd5f2 100644 --- a/includes/media/BitmapMetadataHandler.php +++ b/includes/media/BitmapMetadataHandler.php @@ -32,7 +32,15 @@ class BitmapMetadataHandler { * @param String $app13 String containing app13 block from jpeg file */ private function doApp13 ( $app13 ) { - $this->iptcType = JpegMetadataExtractor::doPSIR( $app13 ); + try { + $this->iptcType = JpegMetadataExtractor::doPSIR( $app13 ); + } catch ( MWException $e ) { + // Error reading the iptc hash information. + // This probably means the App13 segment is something other than what we expect. + // However, still try to read it, and treat it as if the hash didn't exist. + wfDebug( "Error parsing iptc data of file: " . $e->getMessage() . "\n" ); + $this->iptcType = 'iptc-no-hash'; + } $iptc = IPTC::parse( $app13 ); $this->addMetadata( $iptc, $this->iptcType ); @@ -122,8 +130,10 @@ class BitmapMetadataHandler { if ( isset( $seg['COM'] ) && isset( $seg['COM'][0] ) ) { $meta->addMetadata( Array( 'JPEGFileComment' => $seg['COM'] ), 'native' ); } - if ( isset( $seg['PSIR'] ) ) { - $meta->doApp13( $seg['PSIR'] ); + if ( isset( $seg['PSIR'] ) && count( $seg['PSIR'] ) > 0 ) { + foreach( $seg['PSIR'] as $curPSIRValue ) { + $meta->doApp13( $curPSIRValue ); + } } if ( isset( $seg['XMP'] ) && $showXMP ) { $xmp = new XMPReader(); diff --git a/includes/media/JpegMetadataExtractor.php b/includes/media/JpegMetadataExtractor.php index 4769bf8e56..224b4a2bed 100644 --- a/includes/media/JpegMetadataExtractor.php +++ b/includes/media/JpegMetadataExtractor.php @@ -31,6 +31,7 @@ class JpegMetadataExtractor { $segments = array( 'XMP_ext' => array(), 'COM' => array(), + 'PSIR' => array(), ); if ( !$filename ) { @@ -122,7 +123,7 @@ class JpegMetadataExtractor { // APP13 - PSIR. IPTC and some photoshop stuff $temp = self::jpegExtractMarker( $fh ); if ( substr( $temp, 0, 14 ) === "Photoshop 3.0\x00" ) { - $segments["PSIR"] = $temp; + $segments["PSIR"][] = $temp; } } elseif ( $buffer === "\xD9" || $buffer === "\xDA" ) { // EOI - end of image or SOS - start of scan. either way we're past any interesting segments @@ -162,11 +163,12 @@ class JpegMetadataExtractor { * This should generally be called by BitmapMetadataHandler::doApp13() * * @param String $app13 photoshop psir app13 block from jpg. + * @throws MWException (It gets caught next level up though) * @return String if the iptc hash is good or not. */ public static function doPSIR ( $app13 ) { if ( !$app13 ) { - return; + throw new MWException( "No App13 segment given" ); } // First compare hash with real thing // 0x404 contains IPTC, 0x425 has hash @@ -218,8 +220,8 @@ class JpegMetadataExtractor { // this should not happen, but check. if ( $lenData['len'] + $offset > $appLen ) { - wfDebug( __METHOD__ . " PSIR data too long.\n" ); - return 'iptc-no-hash'; + throw new MWException( "PSIR data too long. (item length=" . $lenData['len'] + . "; offset=$offset; total length=$appLen)" ); } if ( $valid ) { diff --git a/tests/phpunit/data/media/iptc-invalid-psir.jpg b/tests/phpunit/data/media/iptc-invalid-psir.jpg new file mode 100644 index 0000000000..01b9acf39a Binary files /dev/null and b/tests/phpunit/data/media/iptc-invalid-psir.jpg differ diff --git a/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php b/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php index bb0f0bb8c6..f4f52dd875 100644 --- a/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php +++ b/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php @@ -56,6 +56,16 @@ class BitmapMetadataHandlerTest extends MediaWikiTestCase { $meta['JPEGFileComment'][0] ); } + /** + * Make sure a bad iptc block doesn't stop the other metadata + * from being extracted. + */ + public function testBadIPTC() { + $meta = BitmapMetadataHandler::Jpeg( $this->filePath . + 'iptc-invalid-psir.jpg' ); + $this->assertEquals( 'Created with GIMP', $meta['JPEGFileComment'][0] ); + } + public function testIPTCDates() { $meta = BitmapMetadataHandler::Jpeg( $this->filePath . 'iptc-timetest.jpg' ); diff --git a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php index 5bf9bfe7c7..f48382a431 100644 --- a/tests/phpunit/includes/media/JpegMetadataExtractorTest.php +++ b/tests/phpunit/includes/media/JpegMetadataExtractorTest.php @@ -59,7 +59,7 @@ class JpegMetadataExtractorTest extends MediaWikiTestCase { public function testPSIRExtraction() { $res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-xmp-psir.jpg' ); $expected = '50686f746f73686f7020332e30003842494d04040000000000181c02190004746573741c02190003666f6f1c020000020004'; - $this->assertEquals( $expected, bin2hex( $res['PSIR'] ) ); + $this->assertEquals( $expected, bin2hex( $res['PSIR'][0] ) ); } public function testXMPExtractionAltAppId() { $res = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-xmp-alt.jpg' ); @@ -70,19 +70,19 @@ class JpegMetadataExtractorTest extends MediaWikiTestCase { public function testIPTCHashComparisionNoHash() { $segments = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-xmp-psir.jpg' ); - $res = JpegMetadataExtractor::doPSIR( $segments['PSIR'] ); + $res = JpegMetadataExtractor::doPSIR( $segments['PSIR'][0] ); $this->assertEquals( 'iptc-no-hash', $res ); } public function testIPTCHashComparisionBadHash() { $segments = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-iptc-bad-hash.jpg' ); - $res = JpegMetadataExtractor::doPSIR( $segments['PSIR'] ); + $res = JpegMetadataExtractor::doPSIR( $segments['PSIR'][0] ); $this->assertEquals( 'iptc-bad-hash', $res ); } public function testIPTCHashComparisionGoodHash() { $segments = JpegMetadataExtractor::segmentSplitter( $this->filePath . 'jpeg-iptc-good-hash.jpg' ); - $res = JpegMetadataExtractor::doPSIR( $segments['PSIR'] ); + $res = JpegMetadataExtractor::doPSIR( $segments['PSIR'][0] ); $this->assertEquals( 'iptc-good-hash', $res ); }