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 3f05454..4e0e46e 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,8 @@ "type": "drupal-module", "description": "Provides an image toolkit to integrate ImageMagick with the Image API.", "require": { - "drupal/file_mdm": "^1", - "drupal/file_mdm_exif": "^1" + "drupal/core": "^8.3", + "drupal/file_mdm": "^1.1", + "drupal/file_mdm_exif": "^1.1" } } diff --git a/src/ImagemagickExecManager.php b/src/ImagemagickExecManager.php index d24268e..44d1f8b 100644 --- a/src/ImagemagickExecManager.php +++ b/src/ImagemagickExecManager.php @@ -7,6 +7,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Session\AccountProxyInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Psr\Log\LoggerInterface; +use Symfony\Component\Process\Process; /** * Manage execution of ImageMagick/GraphicsMagick commands. @@ -30,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 @@ -71,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) { @@ -267,45 +292,22 @@ class ImagemagickExecManager implements ImagemagickExecManagerInterface { * {@inheritdoc} */ public function runOsShell($command, $arguments, $id, &$output = NULL, &$error = NULL) { - if ($this->isWindows) { - // Use Window's start command with the /B flag to make the process run in - // the background and avoid a shell command line window from showing up. - // @see http://us3.php.net/manual/en/function.exec.php#56599 - // Use /D to run the command from PHP's current working directory so the - // file paths don't have to be absolute. - $command = 'start "' . $id . '" /D ' . $this->escapeShellArg($this->appRoot) . ' /B ' . $this->escapeShellArg($command); - } $command_line = $command . ' ' . $arguments; - - // Executes the command on the OS via proc_open(). - $descriptors = [ - // This is stdin. - 0 => ['pipe', 'r'], - // This is stdout. - 1 => ['pipe', 'w'], - // This is stderr. - 2 => ['pipe', 'w'], - ]; + $output = ''; + $error = ''; Timer::start('imagemagick:runOsShell'); - if ($h = proc_open($command_line, $descriptors, $pipes, $this->appRoot)) { - $output = ''; - while (!feof($pipes[1])) { - $output .= fgets($pipes[1]); - } - $output = utf8_encode($output); - $error = ''; - while (!feof($pipes[2])) { - $error .= fgets($pipes[2]); - } - $error = utf8_encode($error); - fclose($pipes[0]); - fclose($pipes[1]); - fclose($pipes[2]); - $return_code = proc_close($h); + $process = new Process($command_line, $this->appRoot); + $process->setTimeout($this->timeout); + try { + $process->run(); + $output = utf8_encode($process->getOutput()); + $error = utf8_encode($process->getErrorOutput()); + $return_code = $process->getExitCode(); } - else { - $return_code = FALSE; + catch (\Exception $e) { + $error = $e->getMessage(); + $return_code = $process->getExitCode() ? $process->getExitCode() : 1; } $execution_time = Timer::stop('imagemagick:runOsShell')['time']; diff --git a/tests/src/Functional/ToolkitImagemagickTest.php b/tests/src/Functional/ToolkitImagemagickTest.php index a2e3117..d67369e 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, $check_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 ($check_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. @@ -337,10 +352,10 @@ class ToolkitImagemagickTest extends BrowserTestBase { if ($package === 'graphicsmagick') { // @todo Issues with crop and convert on GIF files, investigate. if (in_array($file, [ - 'image-test.gif', 'image-test-no-transparency.gif' - ]) && in_array($op, [ - 'crop', 'scale_and_crop', 'convert_png' - ])) { + 'image-test.gif', 'image-test-no-transparency.gif', + ]) && in_array($op, [ + 'crop', 'scale_and_crop', 'convert_png', + ])) { continue; } } @@ -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); + } + }