From 61e250cff15e703a1c001e8f24ba080a89b5fd65 Mon Sep 17 00:00:00 2001 From: Bernd Oliver Suenderhauf <bos@suenderhauf.de> Date: Tue, 21 May 2019 14:35:15 +0200 Subject: [PATCH] Issue #3046419 by Pancho, mondrake, wizonesolutions: Set locale to support filenames with non-ASCII characters in pdftk --- config/install/fillpdf.settings.yml | 2 + config/schema/fillpdf.schema.yml | 3 + fillpdf.install | 11 +- fillpdf.services.yml | 5 + src/Form/FillPdfSettingsForm.php | 56 +++++++++- .../FillPdfBackend/PdftkFillPdfBackend.php | 39 +++++-- src/ShellManager.php | 100 ++++++++++++++++++ src/ShellManagerInterface.php | 58 ++++++++++ tests/src/Functional/PdfParseTest.php | 4 +- tests/src/Functional/PdfPopulationTest.php | 2 +- 10 files changed, 262 insertions(+), 18 deletions(-) create mode 100644 src/ShellManager.php create mode 100644 src/ShellManagerInterface.php diff --git a/config/install/fillpdf.settings.yml b/config/install/fillpdf.settings.yml index b8819ab..e45fd62 100644 --- a/config/install/fillpdf.settings.yml +++ b/config/install/fillpdf.settings.yml @@ -2,6 +2,8 @@ allowed_schemes: - private template_scheme: '' +shell_locale: en_US.utf8 + remote_protocol: https # Should not contain a protocol. That's what the above is for. diff --git a/config/schema/fillpdf.schema.yml b/config/schema/fillpdf.schema.yml index 375a721..46d83f4 100644 --- a/config/schema/fillpdf.schema.yml +++ b/config/schema/fillpdf.schema.yml @@ -22,3 +22,6 @@ fillpdf.settings: local_service_endpoint: type: string label: 'Local FillPDF PDF API endpoint' + shell_locale: + type: string + label: 'Locale for escaping shell commands' diff --git a/fillpdf.install b/fillpdf.install index 1a4fa78..cf6781a 100644 --- a/fillpdf.install +++ b/fillpdf.install @@ -8,7 +8,6 @@ use Drupal\Core\Field\BaseFieldDefinition; use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\Component\Render\FormattableMarkup; -use Drupal\Core\StringTranslation\TranslatableMarkup; use Drupal\fillpdf\Entity\FillPdfForm; use Drupal\fillpdf\Service\FillPdfAdminFormHelper; use Drupal\Core\Link; @@ -184,3 +183,13 @@ function fillpdf_update_8108() { '%list' => new FormattableMarkup(implode(', ', $updated_ids), []), ]); } + +/** + * Adds the 'shell_locale' config setting. + */ +function fillpdf_update_8109() { + $settings = \Drupal::configFactory()->getEditable('fillpdf.settings'); + if ($settings->get('shell_locale') === NULL) { + $settings->set('shell_locale', 'en_US.utf8')->save(); + } +} diff --git a/fillpdf.services.yml b/fillpdf.services.yml index b01d36b..5370938 100644 --- a/fillpdf.services.yml +++ b/fillpdf.services.yml @@ -56,3 +56,8 @@ services: plugin.manager.fillpdf_backend_service: class: Drupal\fillpdf\Plugin\BackendServiceManager parent: default_plugin_manager + + fillpdf.shell_manager: + class: Drupal\fillpdf\ShellManager + arguments: ['@config.factory'] + diff --git a/src/Form/FillPdfSettingsForm.php b/src/Form/FillPdfSettingsForm.php index 38ae1b7..e5d45a7 100644 --- a/src/Form/FillPdfSettingsForm.php +++ b/src/Form/FillPdfSettingsForm.php @@ -10,11 +10,11 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Url; use Drupal\fillpdf\Component\Utility\FillPdf; use Drupal\fillpdf\Service\FillPdfAdminFormHelper; +use Drupal\fillpdf\ShellManager; use GuzzleHttp\Client; use Symfony\Component\DependencyInjection\ContainerInterface; use Drupal\Core\Link; use Drupal\fillpdf\Entity\FillPdfForm; -use Drupal\Core\Config\Config; /** * Configure FillPDF settings form. @@ -51,6 +51,13 @@ class FillPdfSettingsForm extends ConfigFormBase { */ protected $httpClient; + /** + * The FillPDF shell manager. + * + * @var \Drupal\fillpdf\ShellManager + */ + protected $shellManager; + /** * Constructs a FillPdfSettingsForm object. * @@ -62,18 +69,22 @@ class FillPdfSettingsForm extends ConfigFormBase { * The FillPDF admin form helper service. * @param \GuzzleHttp\Client $http_client * The Guzzle HTTP client service. + * @param \Drupal\fillpdf\ShellManager $shell_manager + * The FillPDF shell manager. */ public function __construct( ConfigFactoryInterface $config_factory, FileSystemInterface $file_system, FillPdfAdminFormHelper $admin_form_helper, - Client $http_client + Client $http_client, + ShellManager $shell_manager ) { parent::__construct($config_factory); $this->fileSystem = $file_system; $this->adminFormHelper = $admin_form_helper; $this->httpClient = $http_client; + $this->shellManager = $shell_manager; $backend_manager = \Drupal::service('plugin.manager.fillpdf_backend'); $this->definitions = $backend_manager->getDefinitions(); @@ -87,7 +98,8 @@ class FillPdfSettingsForm extends ConfigFormBase { $container->get('config.factory'), $container->get('file_system'), $container->get('fillpdf.admin_form_helper'), - $container->get('http_client') + $container->get('http_client'), + $container->get('fillpdf.shell_manager') ); } @@ -275,6 +287,41 @@ class FillPdfSettingsForm extends ConfigFormBase { '#group' => 'pdftk', ]; + $form['shell_locale'] = [ + '#title' => $this->t('Server locale'), + '#group' => 'pdftk', + ]; + if ($this->shellManager->isWindows()) { + $form['shell_locale'] += [ + '#type' => 'textfield', + '#description' => $this->t("The locale to be used to prepare the command passed to executables. The default, <kbd>'@default'</kbd>, should work in most cases. If that is not available on the server, @op.", [ + '@default' => '', + '@op' => $this->t('enter another locale'), + ]), + '#default_value' => $config->get('shell_locale') ?: '', + ]; + } + else { + $locales = $this->shellManager->getInstalledLocales(); + // Locale names are unfortunately not standardized. 'locale -a' will give + // 'en_US.UTF-8' on Mac OS systems, 'en_US.utf8' on most/all Unix systems. + $default = isset($locales['en_US.UTF-8']) ? 'en_US.UTF-8' : 'en_US.utf8'; + $form['shell_locale'] += [ + '#type' => 'select', + '#description' => $this->t("The locale to be used to prepare the command passed to executables. The default, <kbd>'@default'</kbd>, should work in most cases. If that is not available on the server, @op.", [ + '@default' => $default, + '@op' => $this->t('choose another locale'), + ]), + '#options' => $locales, + '#default_value' => $config->get('shell_locale') ?: 'en_US.utf8', + ]; + // @todo: We're working around Core issue #2190333, resp. #2854166. + // Remove once one of these landed. See: + // https://www.drupal.org/project/drupal/issues/2854166. + $form['shell_locale']['#process'][] = ['Drupal\Core\Render\Element\Select', 'processGroup']; + $form['shell_locale']['#pre_render'][] = ['Drupal\Core\Render\Element\Select', 'preRenderGroup']; + } + return $form; } @@ -359,7 +406,8 @@ class FillPdfSettingsForm extends ConfigFormBase { break; case 'pdftk': - $config->set('pdftk_path', $form_state->getValue('pdftk_path')); + $config->set('pdftk_path', $form_state->getValue('pdftk_path')) + ->set('shell_locale', $values['shell_locale']); break; } diff --git a/src/Plugin/FillPdfBackend/PdftkFillPdfBackend.php b/src/Plugin/FillPdfBackend/PdftkFillPdfBackend.php index 71ad8b3..a19ae42 100644 --- a/src/Plugin/FillPdfBackend/PdftkFillPdfBackend.php +++ b/src/Plugin/FillPdfBackend/PdftkFillPdfBackend.php @@ -10,6 +10,7 @@ use Drupal\fillpdf\Component\Utility\FillPdf; use Drupal\fillpdf\FillPdfAdminFormHelperInterface; use Drupal\fillpdf\FillPdfBackendPluginInterface; use Drupal\fillpdf\FillPdfFormInterface; +use Drupal\fillpdf\ShellManager; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -43,6 +44,13 @@ class PdftkFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFac */ protected $fileSystem; + /** + * The FillPDF shell manager. + * + * @var \Drupal\fillpdf\ShellManager + */ + protected $shellManager; + /** * The FillPDF admin form helper. * @@ -55,6 +63,8 @@ class PdftkFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFac * * @param \Drupal\Core\File\FileSystem $file_system * The file system. + * @param \Drupal\fillpdf\ShellManager $shell_manager + * The FillPDF shell manager. * @param \Drupal\fillpdf\FillPdfAdminFormHelperInterface $admin_form_helper * The FillPDF admin form helper. * @param array $configuration @@ -64,8 +74,9 @@ class PdftkFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFac * @param array $plugin_definition * The plugin implementation definition. */ - public function __construct(FileSystem $file_system, FillPdfAdminFormHelperInterface $admin_form_helper, array $configuration, $plugin_id, $plugin_definition) { + public function __construct(FileSystem $file_system, ShellManager $shell_manager, FillPdfAdminFormHelperInterface $admin_form_helper, array $configuration, $plugin_id, $plugin_definition) { $this->fileSystem = $file_system; + $this->shellManager = $shell_manager; $this->adminFormHelper = $admin_form_helper; $this->configuration = $configuration; } @@ -76,6 +87,7 @@ class PdftkFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFac public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { return new static( $container->get('file_system'), + $container->get('fillpdf.shell_manager'), $container->get('fillpdf.admin_form_helper'), $configuration, $plugin_id, @@ -91,18 +103,21 @@ class PdftkFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFac $file = File::load($fillpdf_form->file->target_id); $filename = $file->getFileUri(); - $path_to_pdftk = $this->getPdftkPath(); - $status = FillPdf::checkPdftkPath($path_to_pdftk); + $pdftk_path = $this->getPdftkPath(); + $status = FillPdf::checkPdftkPath($pdftk_path); if ($status === FALSE) { \Drupal::messenger()->addError($this->t('pdftk not properly installed.')); return []; } + // Escape the template's realpath. + $template_path = $this->shellManager->escapeShellArg($this->fileSystem->realpath($filename)); + // Use exec() to call pdftk (because it will be easier to go line-by-line // parsing the output) and pass $content via stdin. Retrieve the fields with // dump_data_fields(). $output = []; - exec($path_to_pdftk . ' ' . escapeshellarg($this->fileSystem->realpath($filename)) . ' dump_data_fields', $output, $status); + exec("{$pdftk_path} {$template_path} dump_data_fields", $output, $status); if (count($output) === 0) { \Drupal::messenger()->addWarning($this->t('PDF does not contain fillable fields.')); return []; @@ -154,20 +169,24 @@ class PdftkFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFac $fields = $field_mapping['fields']; module_load_include('inc', 'fillpdf', 'xfdf'); - $xfdfname = $filename . '.xfdf'; - $xfdf = create_xfdf(basename($xfdfname), $fields); + $xfdf_name = $filename . '.xfdf'; + $xfdf = create_xfdf(basename($xfdf_name), $fields); // Generate the file. - $xfdffile = file_save_data($xfdf, $xfdfname, FILE_EXISTS_RENAME); + $xfdf_file = file_save_data($xfdf, $xfdf_name, FILE_EXISTS_RENAME); + + // Escape the template's and the XFDF file's realpath. + $template_path = $this->shellManager->escapeShellArg($this->fileSystem->realpath($filename)); + $xfdf_path = $this->shellManager->escapeShellArg($this->fileSystem->realpath($xfdf_file->getFileUri())); // Now feed this to pdftk and save the result to a variable. - $path_to_pdftk = $this->getPdftkPath(); + $pdftk_path = $this->getPdftkPath(); ob_start(); - passthru($path_to_pdftk . ' ' . escapeshellarg($this->fileSystem->realpath($filename)) . ' fill_form ' . escapeshellarg($this->fileSystem->realpath($xfdffile->getFileUri())) . ' output - ' . ($context['flatten'] ? 'flatten ' : '') . 'drop_xfa'); + passthru("{$pdftk_path} {$template_path} fill_form {$xfdf_path} output - " . ($context['flatten'] ? 'flatten ' : '') . 'drop_xfa'); $data = ob_get_clean(); if ($data === FALSE) { \Drupal::messenger()->addError($this->t('pdftk not properly installed. No PDF generated.')); } - $xfdffile->delete(); + $xfdf_file->delete(); if ($data) { return $data; diff --git a/src/ShellManager.php b/src/ShellManager.php new file mode 100644 index 0000000..e18c0d4 --- /dev/null +++ b/src/ShellManager.php @@ -0,0 +1,100 @@ +<?php + +namespace Drupal\fillpdf; + +use Drupal\Core\Config\ConfigFactoryInterface; + +/** + * Manage execution of shell commands. + * + * @internal + */ +class ShellManager implements ShellManagerInterface { + + /** + * The configuration factory. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; + + /** + * Whether we are running on Windows OS. + * + * @var bool + */ + protected $isWindows; + + /** + * Constructs a ShellManager object. + * + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * The config factory. + */ + public function __construct(ConfigFactoryInterface $config_factory) { + $this->configFactory = $config_factory; + $this->isWindows = substr(PHP_OS, 0, 3) === 'WIN'; + } + + /** + * {@inheritdoc} + */ + public function isWindows() { + return $this->isWindows; + } + + /** + * {@inheritdoc} + */ + public function getInstalledLocales() { + if ($this->isWindows()) { + return []; + } + + $output = []; + $status = NULL; + exec("locale -a", $output, $status); + return array_combine($output, $output); + } + + /** + * {@inheritdoc} + */ + public function escapeShellArg(string $arg) { + // Put the configured locale in a static to avoid multiple config get calls + // in the same request. + static $config_locale; + + if (!isset($config_locale)) { + $config_locale = $this->configFactory->get('fillpdf.settings')->get('shell_locale'); + } + + $current_locale = setlocale(LC_CTYPE, 0); + + if ($this->isWindows()) { + // Temporarily replace % characters. + $arg = str_replace('%', static::PERCENTAGE_REPLACE, $arg); + } + + if ($current_locale !== $config_locale) { + // Temporarily swap the current locale with the configured one, if + // available. Otherwise fall back. + setlocale(LC_CTYPE, [$config_locale, 'C.UTF-8', $current_locale]); + } + + $arg_escaped = escapeshellarg($arg); + + if ($current_locale !== $config_locale) { + // Restore the current locale. + setlocale(LC_CTYPE, $current_locale); + } + + // Get our % characters back. + if ($this->isWindows()) { + $arg_escaped = str_replace(static::PERCENTAGE_REPLACE, '%', $arg_escaped); + } + + return $arg_escaped; + } + +} diff --git a/src/ShellManagerInterface.php b/src/ShellManagerInterface.php new file mode 100644 index 0000000..3b6489b --- /dev/null +++ b/src/ShellManagerInterface.php @@ -0,0 +1,58 @@ +<?php + +namespace Drupal\fillpdf; + +/** + * Provides an interface for FillPDF execution manager. + * + * @internal + */ +interface ShellManagerInterface { + + /** + * Replacement for percentage while escaping. + */ + const PERCENTAGE_REPLACE = 'PERCENTSIGN'; + + /** + * Whether we are running on Windows OS. + * + * @return bool + * TRUE if we're running on Windows, otherwise FALSE. + */ + public function isWindows(); + + /** + * Gets the list of locales installed on the server. + * + * @return string[] + * Associative array of installed locales as returned by 'locale -a' on *nix + * systems, keyed by itself. Will return an empty array on Windows servers. + */ + public function getInstalledLocales(); + + /** + * Escapes a string. + * + * PHP escapeshellarg() drops non-ascii characters, this is a replacement. + * + * Stop-gap replacement until core issue #1561214 has been solved. Solution + * proposed in #1502924-8. + * + * PHP escapeshellarg() on Windows also drops % (percentage sign) characters. + * We prevent this by replacing it with a pattern that should be highly + * unlikely to appear in the string itself and does not contain any + * "dangerous" character at all (very wide definition of dangerous). After + * escaping we replace that pattern back with a % character. + * + * @param string $arg + * The string to escape. + * + * @return string + * Escaped string. + * + * @see https://www.drupal.org/project/drupal/issues/1561214 + */ + public function escapeShellArg(string $arg); + +} diff --git a/tests/src/Functional/PdfParseTest.php b/tests/src/Functional/PdfParseTest.php index 04c66df..dfca806 100644 --- a/tests/src/Functional/PdfParseTest.php +++ b/tests/src/Functional/PdfParseTest.php @@ -69,12 +69,12 @@ class PdfParseTest extends BrowserTestBase { * Tests a backend. */ protected function backendTest() { - $this->uploadTestPdf('fillpdf_test_v3.pdf'); + $this->uploadTestPdf('fillpdf_Ŧäßð_v3â.pdf'); $this->assertSession()->pageTextNotContains('No fields detected in PDF.'); $fillpdf_form = FillPdfForm::load($this->getLatestFillPdfForm()); $fields = $fillpdf_form->getFormFields(); - $this->assertCount(11, $fields); + $this->assertCount(12, $fields); } } diff --git a/tests/src/Functional/PdfPopulationTest.php b/tests/src/Functional/PdfPopulationTest.php index 7ac636d..75314aa 100644 --- a/tests/src/Functional/PdfPopulationTest.php +++ b/tests/src/Functional/PdfPopulationTest.php @@ -287,7 +287,7 @@ class PdfPopulationTest extends FillPdfTestBase { protected function backendTest() { // If we can upload a PDF, parsing is working. // Test with a node. - $this->uploadTestPdf('fillpdf_test_v3.pdf'); + $this->uploadTestPdf('fillpdf_Ŧäßð_v3â.pdf'); $fillpdf_form = FillPdfForm::load($this->getLatestFillPdfForm()); // Get the field definitions for the form that was created and configure -- GitLab