From eac3d58b442b4e2ffd5625bc30c86a6c87596faf Mon Sep 17 00:00:00 2001 From: Bryan Tong Minh Date: Wed, 2 Nov 2011 20:48:50 +0000 Subject: [PATCH] Per comments on r99911 move $wgMaxImageArea check back to normaliseParams(). Added hook BitmapHandlerCheckImageArea to override the area check. I'm not very happy with this overly specific hook, but I don't see a clear way to obtain the functionallity required otherwise. Remove the width and height params from BitmapHandler::getImageArea(). There is really no reason for them to be there. --- RELEASE-NOTES-1.19 | 1 + docs/hooks.txt | 5 ++ includes/media/Bitmap.php | 82 ++++++++----------- includes/media/GIF.php | 11 ++- .../includes/media/BitmapScalingTest.php | 6 ++ 5 files changed, 49 insertions(+), 56 deletions(-) diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19 index 18be8797ed..f6dceb47eb 100644 --- a/RELEASE-NOTES-1.19 +++ b/RELEASE-NOTES-1.19 @@ -76,6 +76,7 @@ production. syntax. * The default user signature now contains a talk link in addition to the user link. * (bug 25306) Add link of old page title to MediaWiki:Delete_and_move_reason +* Added hook BitmapHandlerCheckImageArea === Bug fixes in 1.19 === * $wgUploadNavigationUrl should be used for file redlinks if diff --git a/docs/hooks.txt b/docs/hooks.txt index 506145a675..5bc9f9378a 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -624,6 +624,11 @@ $image: File &$scalerParams: Array with scaler parameters &$mto: null, set to a MediaTransformOutput +'BitmapHandlerCheckImageArea': by BitmapHandler::normaliseParams, after all normalizations have been performed, except for the $wgMaxImageArea check +$image: File +&$params: Array of parameters +&$checkImageAreaHookResult: null, set to true or false to override the $wgMaxImageArea check result + 'PerformRetroactiveAutoblock': called before a retroactive autoblock is applied to a user $block: Block object (which is set to be autoblocking) &$blockIds: Array of block IDs of the autoblock diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php index 95b916dd2a..021df42258 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -12,7 +12,6 @@ * @ingroup Media */ class BitmapHandler extends ImageHandler { - /** * @param $image File * @param $params array Transform parameters. Entries with the keys 'width' @@ -21,7 +20,6 @@ class BitmapHandler extends ImageHandler { * @return bool */ function normaliseParams( $image, &$params ) { - if ( !parent::normaliseParams( $image, $params ) ) { return false; } @@ -41,41 +39,26 @@ class BitmapHandler extends ImageHandler { return true; } } - - return true; - } - - /** - * Check if the file is smaller than the maximum image area for - * thumbnailing. Check will always pass if the scaler is 'hookaborted' or - * if the scaler is 'im' and the mime type is 'image/jpeg' - * - * @param File $image - * @param string $scaler - */ - function checkImageArea( $image, $scaler ) { - global $wgMaxImageArea; - # Don't thumbnail an image so big that it will fill hard drives and send servers into swap - # JPEG has the handy property of allowing thumbnailing without full decompression, so we make - # an exception for it. - - - if ( $image->getMimeType() == 'image/jpeg' && $scaler == 'im' ) - { - # ImageMagick can efficiently downsize jpg images without loading - # the entire file in memory - return true; - } - - if ( $scaler == 'hookaborted' ) - { - # If a hook wants to transform the image, it is responsible for - # checking the image size, so abort here - return true; + + + # Check if the file is smaller than the maximum image area for thumbnailing + $checkImageAreaHookResult = null; + wfRunHooks( 'BitmapHandlerCheckImageArea', array( $image, &$params, &$checkImageAreaHookResult ) ); + if ( is_null( $checkImageAreaHookResult ) ) { + global $wgMaxImageArea; + + if ( $this->getImageArea( $image ) > $wgMaxImageArea && + !( $image->getMimeType() == 'image/jpeg' && + self::getScalerType( false, false ) == 'im' ) ) { + # Only ImageMagick can efficiently downsize jpg images without loading + # the entire file in memory + return false; + } + } else { + return $checkImageAreaHookResult; } - - # Do the actual check - return $this->getImageArea( $image, $image->getWidth(), $image->getHeight() ) <= $wgMaxImageArea; + + return true; } @@ -104,10 +87,15 @@ class BitmapHandler extends ImageHandler { } - // Function that returns the number of pixels to be thumbnailed. - // Intended for animated GIFs to multiply by the number of frames. - function getImageArea( $image, $width, $height ) { - return $width * $height; + /** + * Function that returns the number of pixels to be thumbnailed. + * Intended for animated GIFs to multiply by the number of frames. + * + * @param File $image + * @return int + */ + function getImageArea( $image ) { + return $image->getWidth() * $image->getHeight(); } /** @@ -143,7 +131,10 @@ class BitmapHandler extends ImageHandler { 'dstUrl' => $dstUrl, ); - wfDebug( __METHOD__ . ": creating {$scalerParams['physicalDimensions']} thumbnail at $dstPath\n" ); + # Determine scaler type + $scaler = self::getScalerType( $dstPath ); + + wfDebug( __METHOD__ . ": creating {$scalerParams['physicalDimensions']} thumbnail at $dstPath using scaler $scaler\n" ); if ( !$image->mustRender() && $scalerParams['physicalWidth'] == $scalerParams['srcWidth'] @@ -154,9 +145,6 @@ class BitmapHandler extends ImageHandler { return $this->getClientScalingThumbnailImage( $image, $scalerParams ); } - # Determine scaler type - $scaler = self::getScalerType( $dstPath ); - wfDebug( __METHOD__ . ": scaler $scaler\n" ); if ( $scaler == 'client' ) { # Client-side image scaling, use the source URL @@ -184,12 +172,6 @@ class BitmapHandler extends ImageHandler { $scaler = 'hookaborted'; } - # Check max image area - if ( !$this->checkImageArea( $image, $scaler ) ) - { - return new TransformParameterError( $params ); - } - switch ( $scaler ) { case 'hookaborted': # Handled by the hook above diff --git a/includes/media/GIF.php b/includes/media/GIF.php index 325ceb9a4f..32618e9447 100644 --- a/includes/media/GIF.php +++ b/includes/media/GIF.php @@ -50,17 +50,16 @@ class GIFHandler extends BitmapHandler { /** * @param $image File - * @param $width - * @param $height - * @return + * @todo unittests + * @return bool */ - function getImageArea( $image, $width, $height ) { + function getImageArea( $image ) { $ser = $image->getMetadata(); if ( $ser ) { $metadata = unserialize( $ser ); - return $width * $height * $metadata['frameCount']; + return $image->getWidth() * $image->getHeight() * $metadata['frameCount']; } else { - return $width * $height; + return $image->getWidth() * $image->getHeight(); } } diff --git a/tests/phpunit/includes/media/BitmapScalingTest.php b/tests/phpunit/includes/media/BitmapScalingTest.php index 39fdb81f8f..11d9dc47c3 100644 --- a/tests/phpunit/includes/media/BitmapScalingTest.php +++ b/tests/phpunit/includes/media/BitmapScalingTest.php @@ -119,6 +119,12 @@ class BitmapScalingTest extends MediaWikiTestCase { $this->assertEquals( 'TransformParameterError', get_class( $handler->doTransform( $file, 'dummy path', '', $params ) ) ); } + + function testImageArea() { + $file = new FakeDimensionFile( array( 7, 9 ) ); + $handler = new BitmapHandler; + $this->assertEquals( 63, $handler->getImageArea( $file ) ); + } } class FakeDimensionFile extends File { -- 2.20.1