From 2c247f44d71c0812dbdee81e6500205e8321ea6b Mon Sep 17 00:00:00 2001 From: Steve Breker Date: Thu, 18 Jun 2026 17:06:27 -0700 Subject: [PATCH 1/2] Use Imagick methods for resizing repo theme images Refactored class arRepositoryThemeCropValidatedFile to use Imagick methods instead of mogrify() command. --- ...RepositoryThemeCropValidatedFile.class.php | 67 ++++++++++++------- ...arRepositoryThemeCropValidatedFileTest.php | 55 +++++++++++++++ 2 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php diff --git a/lib/form/arRepositoryThemeCropValidatedFile.class.php b/lib/form/arRepositoryThemeCropValidatedFile.class.php index acc6782ea3..93ec6483e4 100644 --- a/lib/form/arRepositoryThemeCropValidatedFile.class.php +++ b/lib/form/arRepositoryThemeCropValidatedFile.class.php @@ -34,43 +34,58 @@ public function save($file = null, $fileMode = 0666, $create = true, $dirMode = { $file = parent::save($file, $fileMode, $create, $dirMode); - // Check if mogrify is available in the system - exec('which mogrify', $output, $status); - if (0 < $status) { + if (!$this->shouldCropImages()) { return $file; } - // Figure out necessary dimensions from the filename - $pathInfo = pathinfo($this->savedName); + if (null === $dimensions = self::getCropDimensionsFromPath($this->savedName)) { + return $file; + } + + $this->cropImage($dimensions['width'], $dimensions['height']); + + return $file; + } + + public static function getCropDimensionsFromPath($path) + { + $pathInfo = pathinfo($path); switch ($pathInfo['filename']) { case 'logo': - $width = self::LOGO_MAX_WIDTH; - $height = self::LOGO_MAX_HEIGHT; - - break; + return [ + 'width' => self::LOGO_MAX_WIDTH, + 'height' => self::LOGO_MAX_HEIGHT, + ]; case 'banner': - $width = self::BANNER_MAX_WIDTH; - $height = self::BANNER_MAX_HEIGHT; - - break; + return [ + 'width' => self::BANNER_MAX_WIDTH, + 'height' => self::BANNER_MAX_HEIGHT, + ]; } + } - // Stop execution if dimensions were not set - if (!isset($width, $height)) { - return $file; - } + protected function shouldCropImages() + { + return QubitDigitalObject::imagickExtensionLoaded(); + } - // mogrify overwrites the original image file - $command = sprintf( - 'mogrify -crop %sx%s+0+0 %s', - $width, - $height, - $this->savedName - ); - exec($command, $output, $status); + protected function cropImage($width, $height) + { + try { + $image = new Imagick($this->savedName); - return $file; + $cropWidth = min($width, $image->getImageWidth()); + $cropHeight = min($height, $image->getImageHeight()); + + $image->cropImage($cropWidth, $cropHeight, 0, 0); + $image->setImagePage(0, 0, 0, 0); + $image->writeImage($this->savedName); + $image->clear(); + $image->destroy(); + } catch (Exception $e) { + // Leave the uploaded file untouched if Imagick cannot process it. + } } } diff --git a/test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php b/test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php new file mode 100644 index 0000000000..f14d91509b --- /dev/null +++ b/test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php @@ -0,0 +1,55 @@ +assertSame( + [ + 'width' => arRepositoryThemeCropValidatedFile::LOGO_MAX_WIDTH, + 'height' => arRepositoryThemeCropValidatedFile::LOGO_MAX_HEIGHT, + ], + arRepositoryThemeCropValidatedFile::getCropDimensionsFromPath('/tmp/logo.png') + ); + } + + public function testBannerUploadsUseExpectedCropDimensions() + { + $this->assertSame( + [ + 'width' => arRepositoryThemeCropValidatedFile::BANNER_MAX_WIDTH, + 'height' => arRepositoryThemeCropValidatedFile::BANNER_MAX_HEIGHT, + ], + arRepositoryThemeCropValidatedFile::getCropDimensionsFromPath('/tmp/banner.png') + ); + } + + public function testUnknownUploadNamesAreNotCropped() + { + $this->assertNull(arRepositoryThemeCropValidatedFile::getCropDimensionsFromPath('/tmp/other.png')); + } + + public function testCroppingRequiresImagickExtension() + { + $file = new TestRepositoryThemeCropValidatedFile('logo.png', 'image/png', '/tmp/logo.png', 0); + + $this->assertSame(extension_loaded('imagick'), $file->shouldCropImagesForTest()); + } +} + +class TestRepositoryThemeCropValidatedFile extends arRepositoryThemeCropValidatedFile +{ + public function shouldCropImagesForTest() + { + return $this->shouldCropImages(); + } +} From 8b099944024b753b35b5fa67b929a822ada0df11 Mon Sep 17 00:00:00 2001 From: Steve Breker Date: Fri, 26 Jun 2026 17:43:49 -0700 Subject: [PATCH 2/2] Update repository logo and banner crop logic Replace the previous crop-only behavior with resize-and-crop processing for repository logo and banner uploads. Images are now scaled to cover the target dimensions while preserving aspect ratio, then center-cropped to the final logo or banner size. This keeps uploaded assets small after processing and avoids squashing non-square source images. Also increased the allowed upload size from 256K to 500K to give users more room to upload larger source images before resize and crop. Set Imagick memory, map, and pixel-area limits before reading repository logo and banner uploads. --- .../actions/editThemeAction.class.php | 8 +-- ...RepositoryThemeCropValidatedFile.class.php | 66 ++++++++++++++++--- ...arRepositoryThemeCropValidatedFileTest.php | 54 +++++++++++++-- 3 files changed, 111 insertions(+), 17 deletions(-) diff --git a/apps/qubit/modules/repository/actions/editThemeAction.class.php b/apps/qubit/modules/repository/actions/editThemeAction.class.php index dbff364f96..6c4b9e038f 100644 --- a/apps/qubit/modules/repository/actions/editThemeAction.class.php +++ b/apps/qubit/modules/repository/actions/editThemeAction.class.php @@ -118,7 +118,7 @@ protected function addField($name) sfContext::getInstance()->getConfiguration()->loadHelpers('Url'); $this->form->setValidator($name, new sfValidatorFile([ - 'max_size' => '262144', // 256K + 'max_size' => '512000', // 500K 'mime_types' => ['image/png'], // Crop image, it is synchronous but it should be fast 'validated_file_class' => 'arRepositoryThemeCropValidatedFile', @@ -129,7 +129,7 @@ protected function addField($name) $this->form->setWidget($name, new arB5WidgetFormInputFileEditable([ 'label' => $this->context->i18n->__('Banner'), 'help' => $this->context->i18n->__( - 'Requirements: PNG format, 256K max. size.
Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.', + 'Requirements: PNG format, 500K max. size.
Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.', [ '%1%' => arRepositoryThemeCropValidatedFile::BANNER_MAX_WIDTH, '%2%' => arRepositoryThemeCropValidatedFile::BANNER_MAX_HEIGHT, @@ -153,7 +153,7 @@ protected function addField($name) sfContext::getInstance()->getConfiguration()->loadHelpers('Url'); $this->form->setValidator($name, new sfValidatorFile([ - 'max_size' => '262144', // 256K + 'max_size' => '512000', // 500K 'mime_types' => ['image/png'], // Crop image, it is synchronous but it should be fast 'validated_file_class' => 'arRepositoryThemeCropValidatedFile', @@ -164,7 +164,7 @@ protected function addField($name) $this->form->setWidget($name, new arB5WidgetFormInputFileEditable([ 'label' => $this->context->i18n->__('Logo'), 'help' => $this->context->i18n->__( - 'Requirements: PNG format, 256K max. size.
Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.', + 'Requirements: PNG format, 500K max. size.
Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.', [ '%1%' => arRepositoryThemeCropValidatedFile::LOGO_MAX_WIDTH, '%2%' => arRepositoryThemeCropValidatedFile::LOGO_MAX_HEIGHT, diff --git a/lib/form/arRepositoryThemeCropValidatedFile.class.php b/lib/form/arRepositoryThemeCropValidatedFile.class.php index 93ec6483e4..4e61ac5981 100644 --- a/lib/form/arRepositoryThemeCropValidatedFile.class.php +++ b/lib/form/arRepositoryThemeCropValidatedFile.class.php @@ -29,6 +29,9 @@ class arRepositoryThemeCropValidatedFile extends sfValidatedFile public const LOGO_MAX_HEIGHT = 270; public const BANNER_MAX_WIDTH = 800; public const BANNER_MAX_HEIGHT = 300; + public const IMAGICK_MEMORY_LIMIT_MB = 64; + public const IMAGICK_MAP_LIMIT_MB = 64; + public const IMAGICK_AREA_LIMIT_PIXELS = 16000000; public function save($file = null, $fileMode = 0666, $create = true, $dirMode = 0777) { @@ -38,16 +41,16 @@ public function save($file = null, $fileMode = 0666, $create = true, $dirMode = return $file; } - if (null === $dimensions = self::getCropDimensionsFromPath($this->savedName)) { + if (null === $dimensions = self::getTargetDimensionsFromPath($this->savedName)) { return $file; } - $this->cropImage($dimensions['width'], $dimensions['height']); + $this->resizeAndCropImage($dimensions['width'], $dimensions['height']); return $file; } - public static function getCropDimensionsFromPath($path) + public static function getTargetDimensionsFromPath($path) { $pathInfo = pathinfo($path); @@ -66,21 +69,68 @@ public static function getCropDimensionsFromPath($path) } } + public static function getResizeGeometry($sourceWidth, $sourceHeight, $targetWidth, $targetHeight) + { + if ( + 0 >= $sourceWidth + || 0 >= $sourceHeight + || 0 >= $targetWidth + || 0 >= $targetHeight + ) { + return null; + } + + // Scale to fully cover the target box while preserving aspect ratio. + $scale = max($targetWidth / $sourceWidth, $targetHeight / $sourceHeight); + $resizeWidth = (int) ceil($sourceWidth * $scale); + $resizeHeight = (int) ceil($sourceHeight * $scale); + + return [ + // Crop back to the exact target size from the centered overflow. + 'resizeWidth' => $resizeWidth, + 'resizeHeight' => $resizeHeight, + 'cropX' => max(0, (int) floor(($resizeWidth - $targetWidth) / 2)), + 'cropY' => max(0, (int) floor(($resizeHeight - $targetHeight) / 2)), + ]; + } + protected function shouldCropImages() { return QubitDigitalObject::imagickExtensionLoaded(); } - protected function cropImage($width, $height) + protected function resizeAndCropImage($width, $height) { try { - $image = new Imagick($this->savedName); + $image = new Imagick(); + $image->setResourceLimit(Imagick::RESOURCETYPE_MEMORY, self::IMAGICK_MEMORY_LIMIT_MB); + $image->setResourceLimit(Imagick::RESOURCETYPE_MAP, self::IMAGICK_MAP_LIMIT_MB); + $image->setResourceLimit(Imagick::RESOURCETYPE_AREA, self::IMAGICK_AREA_LIMIT_PIXELS); + $image->readImage($this->savedName); + $geometry = self::getResizeGeometry( + $image->getImageWidth(), + $image->getImageHeight(), + $width, + $height + ); + + if (null === $geometry) { + $image->clear(); + $image->destroy(); - $cropWidth = min($width, $image->getImageWidth()); - $cropHeight = min($height, $image->getImageHeight()); + return; + } - $image->cropImage($cropWidth, $cropHeight, 0, 0); + $image->resizeImage( + $geometry['resizeWidth'], + $geometry['resizeHeight'], + Imagick::FILTER_LANCZOS, + 1 + ); + $image->cropImage($width, $height, $geometry['cropX'], $geometry['cropY']); $image->setImagePage(0, 0, 0, 0); + $image->stripImage(); + $image->setImageCompressionQuality(90); $image->writeImage($this->savedName); $image->clear(); $image->destroy(); diff --git a/test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php b/test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php index f14d91509b..55ffe45968 100644 --- a/test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php +++ b/test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php @@ -11,31 +11,75 @@ */ class arRepositoryThemeCropValidatedFileTest extends TestCase { - public function testLogoUploadsUseExpectedCropDimensions() + public function testLogoUploadsUseExpectedTargetDimensions() { $this->assertSame( [ 'width' => arRepositoryThemeCropValidatedFile::LOGO_MAX_WIDTH, 'height' => arRepositoryThemeCropValidatedFile::LOGO_MAX_HEIGHT, ], - arRepositoryThemeCropValidatedFile::getCropDimensionsFromPath('/tmp/logo.png') + arRepositoryThemeCropValidatedFile::getTargetDimensionsFromPath('/tmp/logo.png') ); } - public function testBannerUploadsUseExpectedCropDimensions() + public function testBannerUploadsUseExpectedTargetDimensions() { $this->assertSame( [ 'width' => arRepositoryThemeCropValidatedFile::BANNER_MAX_WIDTH, 'height' => arRepositoryThemeCropValidatedFile::BANNER_MAX_HEIGHT, ], - arRepositoryThemeCropValidatedFile::getCropDimensionsFromPath('/tmp/banner.png') + arRepositoryThemeCropValidatedFile::getTargetDimensionsFromPath('/tmp/banner.png') ); } public function testUnknownUploadNamesAreNotCropped() { - $this->assertNull(arRepositoryThemeCropValidatedFile::getCropDimensionsFromPath('/tmp/other.png')); + $this->assertNull(arRepositoryThemeCropValidatedFile::getTargetDimensionsFromPath('/tmp/other.png')); + } + + public function testResizeGeometryForLandscapeLogoCentersHorizontalCrop() + { + $this->assertSame( + [ + 'resizeWidth' => 540, + 'resizeHeight' => 270, + 'cropX' => 135, + 'cropY' => 0, + ], + arRepositoryThemeCropValidatedFile::getResizeGeometry(1200, 600, 270, 270) + ); + } + + public function testResizeGeometryForPortraitLogoCentersVerticalCrop() + { + $this->assertSame( + [ + 'resizeWidth' => 270, + 'resizeHeight' => 540, + 'cropX' => 0, + 'cropY' => 135, + ], + arRepositoryThemeCropValidatedFile::getResizeGeometry(600, 1200, 270, 270) + ); + } + + public function testResizeGeometryForBannerCentersVerticalCrop() + { + $this->assertSame( + [ + 'resizeWidth' => 800, + 'resizeHeight' => 600, + 'cropX' => 0, + 'cropY' => 150, + ], + arRepositoryThemeCropValidatedFile::getResizeGeometry(1600, 1200, 800, 300) + ); + } + + public function testResizeGeometryRejectsInvalidDimensions() + { + $this->assertNull(arRepositoryThemeCropValidatedFile::getResizeGeometry(0, 1200, 270, 270)); } public function testCroppingRequiresImagickExtension()