Merge pull request #8680 from paulbalandan/imagemagick-library-path

fix: [ImageMagickHandler] early terminate processing of invalid library path

Author: paulbalandan

phpstan-baseline.php

@@ -7723,7 +7723,7 @@
 ];
 $ignoreErrors[] = [
 	'message' => '#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#',
-	'count' => 9,
+	'count' => 8,
 	'path' => __DIR__ . '/system/Images/Handlers/ImageMagickHandler.php',
 ];
 $ignoreErrors[] = [

system/Images/Handlers/ImageMagickHandler.php

@@ -32,8 +32,6 @@ class ImageMagickHandler extends BaseHandler
     protected $resource;
 
     /**
-     * Constructor.
-     *
      * @param Images $config
      *
      * @throws ImageException
@@ -45,6 +43,22 @@ public function __construct($config = null)
         if (! (extension_loaded('imagick') || class_exists(Imagick::class))) {
             throw ImageException::forMissingExtension('IMAGICK'); // @codeCoverageIgnore
         }
+
+        $cmd = $this->config->libraryPath;
+
+        if ($cmd === '') {
+            throw ImageException::forInvalidImageLibraryPath($cmd);
+        }
+
+        if (preg_match('/convert$/i', $cmd) !== 1) {
+            $cmd = rtrim($cmd, '\/') . '/convert';
+
+            $this->config->libraryPath = $cmd;
+        }
+
+        if (! is_file($cmd)) {
+            throw ImageException::forInvalidImageLibraryPath($cmd);
+        }
     }
 
     /**
@@ -184,19 +198,10 @@ public function getVersion(): string
      */
     protected function process(string $action, int $quality = 100): array
     {
-        // Do we have a vaild library path?
-        if (empty($this->config->libraryPath)) {
-            throw ImageException::forInvalidImageLibraryPath($this->config->libraryPath);
-        }
-
         if ($action !== '-version') {
             $this->supportedFormatCheck();
         }
 
-        if (! preg_match('/convert$/i', $this->config->libraryPath)) {
-            $this->config->libraryPath = rtrim($this->config->libraryPath, '/') . '/convert';
-        }
-
         $cmd = $this->config->libraryPath;
         $cmd .= $action === '-version' ? ' ' . $action : ' -quality ' . $quality . ' ' . $action;
 

tests/system/Images/ImageMagickHandlerTest.php

@@ -14,6 +14,7 @@
 use CodeIgniter\Config\Services;
 use CodeIgniter\Images\Exceptions\ImageException;
 use CodeIgniter\Images\Handlers\BaseHandler;
+use CodeIgniter\Images\Handlers\ImageMagickHandler;
 use CodeIgniter\Test\CIUnitTestCase;
 use Config\Images;
 use Imagick;
@@ -77,6 +78,33 @@ protected function setUp(): void
         $this->handler = Services::image('imagick', $config, false);
     }
 
+    /**
+     * @dataProvider provideNonexistentLibraryPathTerminatesProcessing
+     */
+    public function testNonexistentLibraryPathTerminatesProcessing(string $path, string $invalidPath): void
+    {
+        $this->expectException(ImageException::class);
+        $this->expectExceptionMessage(lang('Images.libPathInvalid', [$invalidPath]));
+
+        $config = new Images();
+
+        $config->libraryPath = $path;
+
+        new ImageMagickHandler($config);
+    }
+
+    /**
+     * @return iterable<string, list<string>>
+     */
+    public static function provideNonexistentLibraryPathTerminatesProcessing(): iterable
+    {
+        yield 'empty string' => ['', ''];
+
+        yield 'invalid file' => ['/var/log/convert', '/var/log/convert'];
+
+        yield 'nonexistent file' => ['/var/www/file', '/var/www/file/convert'];
+    }
+
     public function testGetVersion(): void
     {
         $version = $this->handler->getVersion();