Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [
Expand Down
27 changes: 16 additions & 11 deletions system/Images/Handlers/ImageMagickHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ class ImageMagickHandler extends BaseHandler
protected $resource;

/**
* Constructor.
*
* @param Images $config
*
* @throws ImageException
Expand All @@ -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);
}
}

/**
Expand Down Expand Up @@ -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;

Expand Down
28 changes: 28 additions & 0 deletions tests/system/Images/ImageMagickHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -77,6 +78,33 @@
$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();
Expand Down Expand Up @@ -422,7 +450,7 @@
$this->assertSame(exif_imagetype($this->root . 'ci-logo.png'), IMAGETYPE_PNG);
}

public function testImageReorientLandscape(): void

Check warning on line 453 in tests/system/Images/ImageMagickHandlerTest.php

View workflow job for this annotation

GitHub Actions / Others (8.1) / Sanity Tests

Took 0.58s from 0.50s limit to run CodeIgniter\\Images\\ImageMagickHandlerTest::testImageReorientLandscape
{
for ($i = 0; $i <= 8; $i++) {
$source = $this->origin . 'EXIFsamples/landscape_' . $i . '.jpg';
Expand All @@ -441,7 +469,7 @@
}
}

public function testImageReorientPortrait(): void

Check warning on line 472 in tests/system/Images/ImageMagickHandlerTest.php

View workflow job for this annotation

GitHub Actions / Others (8.1) / Sanity Tests

Took 0.55s from 0.50s limit to run CodeIgniter\\Images\\ImageMagickHandlerTest::testImageReorientPortrait
{
for ($i = 0; $i <= 8; $i++) {
$source = $this->origin . 'EXIFsamples/portrait_' . $i . '.jpg';
Expand Down
Loading