' . implode('
', $form_state['imagemagick_errors']) . '
' . implode('
', $status['errors']) . '
' . check_plain(trim($output)) . '', + '#markup' => '
' . check_plain(trim($status['output'])) . '', '#description' => t('ImageMagick was found and returns this version information.'), ); } @@ -118,40 +123,56 @@ function _imagemagick_build_version($element, &$form_state) { } /** - * Verify file path of ImageMagick convert binary. + * Verifies file path of ImageMagick convert binary by checking its version. * * @param $file * The user-submitted file path to the convert binary. * * @return - * A list of errors indicating whether ImageMagick could not be found on this - * machine, which is empty if it was found. + * An associative array containing: + * - output: The shell output of 'convert -version', if any. + * - errors: A list of error messages indicating whether ImageMagick could not + * be found or executed. */ function _imagemagick_check_path($file) { - $errors = array(); - - // If only the name of the executable is given, check whether it is in the - // path and can be invoked. - if ($file == 'convert' && dirname($file) === '.') { - $status = _imagemagick_convert_exec('-version', $output, $errors, $file); - if ($status === 0) { - return array($errors); - } - } + $status = array( + 'output' => '', + 'errors' => array(), + ); - if (!is_file($file)) { - $errors[] = t('The specified ImageMagick file path %file does not exist.', array('%file' => $file)); - } - if (!$errors && !is_executable($file)) { - $errors[] = t('The specified ImageMagick file path %file is not executable.', array('%file' => $file)); + // If only the name of the executable is given, we only check whether it is in + // the path and can be invoked. + if ($file != 'convert') { + // Check whether the given file exists. + if (!is_file($file)) { + $status['errors'][] = t('The specified ImageMagick file path %file does not exist.', array('%file' => $file)); + } + // If it exists, check whether we can execute it. + elseif (!is_executable($file)) { + $status['errors'][] = t('The specified ImageMagick file path %file is not executable.', array('%file' => $file)); + } } - if ($errors && ($open_basedir = ini_get('open_basedir'))) { - $errors[] = t('The PHP open_basedir security restriction is set to %open-basedir, which may prevent to locate ImageMagick.', array( + // In case of errors, check for open_basedir restrictions. + if ($status['errors'] && ($open_basedir = ini_get('open_basedir'))) { + $status['errors'][] = t('The PHP open_basedir security restriction is set to %open-basedir, which may prevent to locate ImageMagick.', array( '%open-basedir' => $open_basedir, '@php-url' => 'http://php.net/manual/en/ini.core.php#ini.open-basedir', )); } - return $errors; + + // Unless we had errors so far, try to invoke convert. + if (!$status['errors']) { + $result = _imagemagick_convert_exec('-version', $status['output'], $error, $file); + // _imagemagick_convert_exec() triggers a user error upon failure, but + // during form validation all errors need to be reported. + if ($error !== '') { + // $error normally needs check_plain(), but file system errors on Windows + // use a unknown encoding. check_plain() would eliminate the entire string. + $status['errors'][] = $error; + } + } + + return $status; } /** @@ -356,8 +377,7 @@ function _imagemagick_convert($source, $dest, $args) { // @see http://www.imagemagick.org/Usage/basics/#cmdline $command = escapeshellarg($source) . ' ' . implode(' ', $args) . ' ' . escapeshellarg($dest); - $status = _imagemagick_convert_exec($command, $output, $errors); - if ($status !== 0) { + if (_imagemagick_convert_exec($command, $output, $error) !== TRUE) { return FALSE; } return file_exists($dest); @@ -369,20 +389,25 @@ function _imagemagick_convert($source, $dest, $args) { * @param $command_args * A string containing arguments to pass to the convert command, which must * have been passed through escapeshellarg() already. - * @param $output + * @param &$output * (optional) A variable to assign the shell stdout to, passed by reference. - * @param $errors + * @param &$error * (optional) A variable to assign the shell stderr to, passed by reference. * @param $convert_path * (optional) A custom file path to the convert binary. Internal use only. * - * @return - * 0 (zero) if the command succeeded, Boolean FALSE if the shell process could - * not be executed, or the error status code returned by the executable. + * @return mixed + * The return value depends on the shell command result: + * - Boolean TRUE if the command succeeded. + * - Boolean FALSE if the shell process could not be executed. + * - Error exit status code integer returned by the executable. */ -function _imagemagick_convert_exec($command_args, &$output = NULL, &$errors = NULL, $convert_path = NULL) { +function _imagemagick_convert_exec($command_args, &$output = NULL, &$error = NULL, $convert_path = NULL) { + // $convert_path is only passed from the system-wide image toolkit form, on + // which the path to convert is configured. + // @see _imagemagick_check_path() if (!isset($convert_path)) { - // By using a default to NULL, we force users to setup the toolkit through + // By using a default of NULL, we force users to setup the toolkit through // the image toolkit administration UI. Sites enforcing a path via // settings.php should know what they are doing. $convert_path = variable_get('imagemagick_convert', NULL); @@ -416,10 +441,9 @@ function _imagemagick_convert_exec($command_args, &$output = NULL, &$errors = NU while (!feof($pipes[1])) { $output .= fgets($pipes[1]); } - - $errors = ''; + $error = ''; while (!feof($pipes[2])) { - $errors .= fgets($pipes[2]); + $error .= fgets($pipes[2]); } fclose($pipes[0]); @@ -433,31 +457,30 @@ function _imagemagick_convert_exec($command_args, &$output = NULL, &$errors = NU if ($output !== '') { debug($output, t('ImageMagick output'), TRUE); } - if ($errors !== '') { - debug($errors, t('ImageMagick errors'), TRUE); + if ($error !== '') { + debug($error, t('ImageMagick error'), TRUE); } } + // If ImageMagick returned a non-zero code, trigger a PHP error that will + // be caught by Drupal's error handler, logged to the watchdog and + // eventually displayed to the user if configured to do so. if ($return_code != 0) { - // If ImageMagick returned a non-zero code, trigger a PHP error that will - // be caught by Drupal's error handler, logged to the watchdog and - // eventually displayed to the user if configured to do so. - - // If $errors is empty, only report the error code. - if ($errors === '') { - trigger_error(t('ImageMagick error @code', array('@code' => $return_code)), E_USER_ERROR); - } - // Otherwise report the error code, and the error message. - else { - trigger_error(t('ImageMagick error @code: @error', array( - '@code' => $return_code, - '@error' => $errors, - )), E_USER_ERROR); + // If there is no error message, clarify this. + if ($error === '') { + $error = t('(No message)'); } + trigger_error(t('ImageMagick error @code: @error', array( + '@code' => $return_code, + '@error' => $error, + )), E_USER_ERROR); + // ImageMagick exited with an error code, return it. + return $return_code; } - - return $return_code; + // The shell command was executed successfully. + return TRUE; } + // The shell command could not be executed. return FALSE; }