diff --git a/README.txt b/README.txt index 313ac89..1cb13da 100644 --- a/README.txt +++ b/README.txt @@ -11,7 +11,9 @@ To submit bug reports and feature suggestions, or to track changes: -- REQUIREMENTS -- -* Drupal 8.1.0 or higher +* PHP 5.6 or higher + +* Drupal 8.3.0 or higher * Either ImageMagick (http://www.imagemagick.org) or GraphicsMagick (http://www.graphicsmagick.org) need to be installed on your server and the @@ -20,7 +22,7 @@ To submit bug reports and feature suggestions, or to track changes: * The PHP configuration must allow invocation of proc_open() (which is security-wise identical to exec()). -* File Metadata Manager module 8.x-1.0-beta2 or higher +* File Metadata Manager module 8.x-1.1 or higher * Composer based installation process is needed to install the module dependencies, see https://www.drupal.org/node/2718229 @@ -34,7 +36,7 @@ these requirements. * Install the required module packages with Composer. From the Drupal installation root directory, type - $ composer require drupal/imagemagick:~2.0 + $ composer require drupal/imagemagick:^2.1 This will download both the ImageMagick module and any dependent module (namely, the File Metadata Manager module). diff --git a/composer.json b/composer.json index e80ecbd..4e0e46e 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "description": "Provides an image toolkit to integrate ImageMagick with the Image API.", "require": { "drupal/core": "^8.3", - "drupal/file_mdm": "^1", - "drupal/file_mdm_exif": "^1" + "drupal/file_mdm": "^1.1", + "drupal/file_mdm_exif": "^1.1" } } diff --git a/src/ImagemagickExecManager.php b/src/ImagemagickExecManager.php index 89515ca..3b90071 100644 --- a/src/ImagemagickExecManager.php +++ b/src/ImagemagickExecManager.php @@ -31,6 +31,13 @@ class ImagemagickExecManager implements ImagemagickExecManagerInterface { protected $appRoot; /** + * The execution timeout. + * + * @var int + */ + protected $timeout = 60; + + /** * The current user. * * @var \Drupal\Core\Session\AccountProxyInterface @@ -72,6 +79,23 @@ class ImagemagickExecManager implements ImagemagickExecManagerInterface { } /** + * Sets the execution timeout (max. runtime). + * + * To disable the timeout, set this value to null. + * + * @param int|null $timeout + * The timeout in seconds. + * + * @return $this + * + * @todo in next major, add this method to the interface. + */ + public function setTimeout($timeout) { + $this->timeout = $timeout; + return $this; + } + + /** * {@inheritdoc} */ public function getPackage($package = NULL) { @@ -269,13 +293,22 @@ class ImagemagickExecManager implements ImagemagickExecManagerInterface { */ public function runOsShell($command, $arguments, $id, &$output = NULL, &$error = NULL) { $command_line = $command . ' ' . $arguments; + $output = ''; + $error = ''; Timer::start('imagemagick:runOsShell'); $process = new Process($command_line, $this->appRoot); - $process->run(); - $output = utf8_encode($process->getOutput()); - $error = utf8_encode($process->getErrorOutput()); - $return_code = $process->getExitCode(); + $process->setTimeout($this->timeout); + try { + $process->run(); + $output = utf8_encode($process->getOutput()); + $error = utf8_encode($process->getErrorOutput()); + $return_code = $process->getExitCode(); + } + catch (\Exception $e) { + $error = $e->getMessage(); + $return_code = $process->getExitCode() ? $process->getExitCode() : 1; + } $execution_time = Timer::stop('imagemagick:runOsShell')['time']; // Process debugging information if required. diff --git a/tests/src/Functional/ToolkitImagemagickTest.php b/tests/src/Functional/ToolkitImagemagickTest.php index a2e3117..a0de9c7 100644 --- a/tests/src/Functional/ToolkitImagemagickTest.php +++ b/tests/src/Functional/ToolkitImagemagickTest.php @@ -81,51 +81,36 @@ class ToolkitImagemagickTest extends BrowserTestBase { } /** - * Provides data for testManipulations. - * - * @return array[] - * A simple array of simple arrays, each having the following elements: - * - binaries to use for testing. - */ - public function providerManipulationTest() { - return [ - ['imagemagick'], - ['graphicsmagick'], - ]; - } - - /** - * Test image toolkit operations. - * - * Since PHP can't visually check that our images have been manipulated - * properly, build a list of expected color values for each of the corners and - * the expected height and widths for the final images. + * Helper to setup the image toolkit. * * @param string $binaries * The graphics package binaries to use for testing. - * - * @dataProvider providerManipulationTest + * @param bool $check_path + * Whether the path to binaries should be tested. */ - public function testManipulations($binaries) { + protected function setUpToolkit($binaries, $test_path = TRUE) { // Change the toolkit. \Drupal::configFactory()->getEditable('system.image') ->set('toolkit', 'imagemagick') ->save(); // Execute tests with selected binaries. - // The test can only be executed if binaries are available on the shell - // path. \Drupal::configFactory()->getEditable('imagemagick.settings') ->set('debug', TRUE) ->set('binaries', $binaries) ->set('quality', 100) ->save(); - $status = \Drupal::service('image.toolkit.manager')->createInstance('imagemagick')->checkPath(''); - if (!empty($status['errors'])) { - // Bots running automated test on d.o. do not have binaries installed, - // so the test will be skipped; it can be run locally where binaries are - // installed. - $this->markTestSkipped("Tests for '{$binaries}' cannot run because the binaries are not available on the shell path."); + + if ($test_path) { + // The test can only be executed if binaries are available on the shell + // path. + $status = \Drupal::service('image.toolkit.manager')->createInstance('imagemagick')->checkPath(''); + if (!empty($status['errors'])) { + // Bots running automated test on d.o. do not have binaries installed, + // so the test will be skipped; it can be run locally where binaries + // are installed. + $this->markTestSkipped("Tests for '{$binaries}' cannot run because the binaries are not available on the shell path."); + } } // Set the toolkit on the image factory. @@ -137,6 +122,36 @@ class ToolkitImagemagickTest extends BrowserTestBase { // Prepare directory. file_unmanaged_delete_recursive($this->testDirectory); file_prepare_directory($this->testDirectory, FILE_CREATE_DIRECTORY); + } + + /** + * Provides data for testManipulations. + * + * @return array[] + * A simple array of simple arrays, each having the following elements: + * - binaries to use for testing. + */ + public function providerManipulationTest() { + return [ + ['imagemagick'], + ['graphicsmagick'], + ]; + } + + /** + * Test image toolkit operations. + * + * Since PHP can't visually check that our images have been manipulated + * properly, build a list of expected color values for each of the corners and + * the expected height and widths for the final images. + * + * @param string $binaries + * The graphics package binaries to use for testing. + * + * @dataProvider providerManipulationTest + */ + public function testManipulations($binaries) { + $this->setUpToolkit($binaries); // Typically the corner colors will be unchanged. These colors are in the // order of top-left, top-right, bottom-right, bottom-left. @@ -702,33 +717,7 @@ class ToolkitImagemagickTest extends BrowserTestBase { * in the command line passed to binaries. */ public function testInternalArguments() { - // Change the toolkit. - \Drupal::configFactory()->getEditable('system.image') - ->set('toolkit', 'imagemagick') - ->save(); - - // Execute tests with selected binaries. - // The test can only be executed if binaries are available on the shell - // path. - \Drupal::configFactory()->getEditable('imagemagick.settings') - ->set('debug', TRUE) - ->set('binaries', 'imagemagick') - ->set('quality', 100) - ->save(); - $status = \Drupal::service('image.toolkit.manager')->createInstance('imagemagick')->checkPath(''); - if (!empty($status['errors'])) { - // Bots running automated test on d.o. do not have binaries installed, - // so the test will be skipped; it can be run locally where binaries are - // installed. - $this->markTestSkipped("Tests for 'imagemagick' cannot run because the binaries are not available on the shell path."); - } - - // Set the toolkit on the image factory. - $this->imageFactory->setToolkitId('imagemagick'); - - // Prepare directory. - file_unmanaged_delete_recursive($this->testDirectory); - file_prepare_directory($this->testDirectory, FILE_CREATE_DIRECTORY); + $this->setUpToolkit('imagemagick'); // Prepare a copy of test files. $this->getTestFiles('image'); @@ -755,34 +744,13 @@ class ToolkitImagemagickTest extends BrowserTestBase { * Test module arguments alter hook. */ public function testArgumentsAlterHook() { - // Change the toolkit. - \Drupal::configFactory()->getEditable('system.image') - ->set('toolkit', 'imagemagick') - ->save(); + $this->setUpToolkit('imagemagick'); - // Execute tests with selected binaries. - // The test can only be executed if binaries are available on the shell - // path. + // Change the Advanced Colorspace setting, must be included in the + // command line. \Drupal::configFactory()->getEditable('imagemagick.settings') - ->set('debug', TRUE) - ->set('binaries', 'imagemagick') - ->set('quality', 100) ->set('advanced.colorspace', 'RGB') ->save(); - $status = \Drupal::service('image.toolkit.manager')->createInstance('imagemagick')->checkPath(''); - if (!empty($status['errors'])) { - // Bots running automated test on d.o. do not have binaries installed, - // so the test will be skipped; it can be run locally where binaries are - // installed. - $this->markTestSkipped("Tests for 'imagemagick' cannot run because the binaries are not available on the shell path."); - } - - // Set the toolkit on the image factory. - $this->imageFactory->setToolkitId('imagemagick'); - - // Prepare directory. - file_unmanaged_delete_recursive($this->testDirectory); - file_prepare_directory($this->testDirectory, FILE_CREATE_DIRECTORY); // Prepare a copy of test files. $this->getTestFiles('image'); @@ -805,4 +773,31 @@ class ToolkitImagemagickTest extends BrowserTestBase { $this->assertSame("-resize 100x75! -quality 75 -colorspace 'RGB'", $image->getToolkit()->getStringForBinary()); } + /** + * Test missing command on ExecManager. + */ + public function testExecManagerCommandNotFound() { + $exec_manager = \Drupal::service('imagemagick.exec_manager'); + $output = ''; + $error = ''; + // @todo differentiate for Windows? + $ret = $exec_manager->runOsShell('pinkpanther', '-inspector Clouseau', 'blake', $output, $error); + $this->assertEquals(127, $ret, $error); + } + + /** + * Test timeout on ExecManager. + */ + public function testExecManagerTimeout() { + $exec_manager = \Drupal::service('imagemagick.exec_manager'); + $output = ''; + $error = ''; + // Set a short timeout (1 sec.) and run a process that is expected to last + // longer (10 secs.). Should return a 'terminate' exit code. + $exec_manager->setTimeout(1); + // @todo differentiate for Windows? + $ret = $exec_manager->runOsShell('sleep', '10', 'sleep', $output, $error); + $this->assertEquals(143, $ret, $error); + } + }