From 8a736da669508672e069d43e3272b40f602ca3cf Mon Sep 17 00:00:00 2001 From: Steve Breker Date: Thu, 18 Jun 2026 17:06:27 -0700 Subject: [PATCH 1/3] 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..8e61951809 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 extension_loaded('imagick'); + } - // 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 5a7e19fc2a86ede44d54194271897ae90ef706c2 Mon Sep 17 00:00:00 2001 From: Steve Breker Date: Fri, 26 Jun 2026 17:43:49 -0700 Subject: [PATCH 2/3] 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. --- .../actions/editThemeAction.class.php | 8 +-- ...RepositoryThemeCropValidatedFile.class.php | 57 ++++++++++++++++--- ...arRepositoryThemeCropValidatedFileTest.php | 54 ++++++++++++++++-- 3 files changed, 103 insertions(+), 16 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 8e61951809..4d16baf76b 100644 --- a/lib/form/arRepositoryThemeCropValidatedFile.class.php +++ b/lib/form/arRepositoryThemeCropValidatedFile.class.php @@ -38,16 +38,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 +66,64 @@ 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 extension_loaded('imagick'); } - protected function cropImage($width, $height) + protected function resizeAndCropImage($width, $height) { try { $image = new Imagick($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() From 3869a0aefc006aaffe6473e69817117fa4dbfe65 Mon Sep 17 00:00:00 2001 From: Steve Breker Date: Fri, 26 Jun 2026 17:50:51 -0700 Subject: [PATCH 3/3] Add Imagick resource limit theme img processing Set Imagick memory, map, and pixel-area limits before reading repository logo and banner uploads. --- lib/form/arRepositoryThemeCropValidatedFile.class.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/form/arRepositoryThemeCropValidatedFile.class.php b/lib/form/arRepositoryThemeCropValidatedFile.class.php index 4d16baf76b..1a357d3fb5 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) { @@ -99,7 +102,11 @@ protected function shouldCropImages() 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(),