From f10b4d35a37e2ed5643e029b308dba84a2979e76 Mon Sep 17 00:00:00 2001 From: lucashedding <lucashedding@1463982.no-reply.drupal.org> Date: Wed, 6 Nov 2019 14:49:54 -0600 Subject: [PATCH] Issue #3092777 by heddn: Fail in place update if modified files hash is not available --- .../views.view.automatic_updates_log.yml | 18 ++++---- src/Controller/InPlaceUpdateController.php | 4 +- src/ProjectInfoTrait.php | 6 +-- src/Services/InPlaceUpdate.php | 30 ++++++++----- src/Services/ModifiedFiles.php | 44 ++++++++++++++----- src/Services/ModifiedFilesInterface.php | 4 +- 6 files changed, 69 insertions(+), 37 deletions(-) diff --git a/config/optional/views.view.automatic_updates_log.yml b/config/optional/views.view.automatic_updates_log.yml index 397ff7e64d..bec769f418 100644 --- a/config/optional/views.view.automatic_updates_log.yml +++ b/config/optional/views.view.automatic_updates_log.yml @@ -239,21 +239,21 @@ display: value: 6: '6' group: 1 - exposed: false + exposed: true expose: - operator_id: '' - label: '' + operator_id: severity_op + label: 'Severity level' description: '' use_operator: false - operator: '' - operator_limit_selection: false - operator_list: { } - identifier: '' + operator: severity_op + identifier: severity required: false remember: false - multiple: false + multiple: true remember_roles: authenticated: authenticated + anonymous: '0' + administrator: '0' reduce: false is_grouped: false group_info: @@ -305,6 +305,7 @@ display: max-age: -1 contexts: - 'languages:language_interface' + - url - url.query_args - user.permissions tags: { } @@ -329,6 +330,7 @@ display: max-age: -1 contexts: - 'languages:language_interface' + - url - url.query_args - user.permissions tags: { } diff --git a/src/Controller/InPlaceUpdateController.php b/src/Controller/InPlaceUpdateController.php index e22d4fea0d..151e525cd4 100644 --- a/src/Controller/InPlaceUpdateController.php +++ b/src/Controller/InPlaceUpdateController.php @@ -8,7 +8,7 @@ use Drupal\Core\Messenger\MessengerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** - * Returns responses for Test Automatic Updates routes. + * Returns responses for Automatic Updates routes. */ class InPlaceUpdateController extends ControllerBase { @@ -47,7 +47,7 @@ class InPlaceUpdateController extends ControllerBase { $message = $this->t('Update successful'); if (!$updated) { $message_type = MessengerInterface::TYPE_ERROR; - $message = $this->t('Update Failed'); + $message = $this->t('Update failed. Please review logs to determine the cause.'); } $this->messenger()->addMessage($message, $message_type); return $this->redirect('automatic_updates.settings'); diff --git a/src/ProjectInfoTrait.php b/src/ProjectInfoTrait.php index 219ca5f953..638c936994 100644 --- a/src/ProjectInfoTrait.php +++ b/src/ProjectInfoTrait.php @@ -46,7 +46,7 @@ trait ProjectInfoTrait { $info['version'] = $this->getExtensionVersion($info); }); $system = isset($infos['system']) ? $infos['system'] : NULL; - $infos = array_filter($infos, function (array $info, $project_name) { + $infos = array_filter($infos, static function (array $info, $project_name) { return $info && $info['project'] === $project_name; }, ARRAY_FILTER_USE_BOTH); if ($system) { @@ -70,7 +70,7 @@ trait ProjectInfoTrait { return $info['version']; } // Handle experimental modules from core. - if (substr($info['install path'], 0, 4) === "core") { + if (strpos($info['install path'], "core") === 0) { return $this->getExtensionList('module')->get('system')->info['version']; } \Drupal::logger('automatic_updates')->error('Version cannot be located for @extension', ['@extension' => $extension_name]); @@ -98,7 +98,7 @@ trait ProjectInfoTrait { $project_name = $this->getSuffix($composer_json['name'], '/', $extension_name); } } - if (substr($info['install path'], 0, 4) === "core") { + if (strpos($info['install path'], 'core') === 0) { $project_name = 'drupal'; } return $project_name; diff --git a/src/Services/InPlaceUpdate.php b/src/Services/InPlaceUpdate.php index 4b865fab59..a9f1b4c076 100644 --- a/src/Services/InPlaceUpdate.php +++ b/src/Services/InPlaceUpdate.php @@ -196,15 +196,20 @@ class InPlaceUpdate implements UpdateInterface { * Return TRUE if modified files exist, FALSE otherwise. */ protected function checkModifiedFiles($project_name, $project_type, ArchiverInterface $archive) { - $extensions = []; - foreach (['module', 'profile', 'theme'] as $extension_type) { - $extensions[] = $this->getInfos($extension_type); + if ($project_type === 'core') { + $project_type = 'module'; } - $extensions = array_merge(...$extensions); - + $extensions = $this->getInfos($project_type); /** @var \Drupal\automatic_updates\Services\ModifiedFilesInterface $modified_files */ $modified_files = \Drupal::service('automatic_updates.modified_files'); - $files = iterator_to_array($modified_files->getModifiedFiles([$extensions[$project_name]])); + try { + $files = iterator_to_array($modified_files->getModifiedFiles([$extensions[$project_name]], TRUE)); + } + catch (RequestException $exception) { + // While not strictly true that there are modified files, we can't be sure + // there aren't any. So assume the worst. + return TRUE; + } $files = array_unique($files); $archive_files = $archive->listContents(); foreach ($archive_files as $index => &$archive_file) { @@ -247,8 +252,9 @@ class InPlaceUpdate implements UpdateInterface { ]); } catch (RequestException $exception) { - if ($exception->getResponse()->getStatusCode() === 429 && ($retry = $exception->getResponse()->getHeader('Retry-After'))) { - $this->doGetArchive($url, $destination, $retry[0] * 1000); + $response = $exception->getResponse(); + if (!$response || ($response->getStatusCode() === 429 && ($retry = $response->getHeader('Retry-After')))) { + $this->doGetArchive($url, $destination, $retry[0] ?? 10 * 1000); } else { $this->logger->error('Retrieval of "@url" failed with: @message', [ @@ -371,7 +377,7 @@ class InPlaceUpdate implements UpdateInterface { * TRUE if path was removed, else FALSE. */ protected function stripFileDirectoryPath(&$file) { - if (substr($file, 0, 6) === self::ARCHIVE_DIRECTORY) { + if (strpos($file, self::ARCHIVE_DIRECTORY) === 0) { $file = substr($file, 6); return TRUE; } @@ -440,11 +446,11 @@ class InPlaceUpdate implements UpdateInterface { * The iterator of SplFileInfos. */ protected function getFilesList($directory) { - $filter = function ($file, $file_name, $iterator) { + $filter = static function ($file, $file_name, $iterator) { /** @var \SplFileInfo $file */ /** @var string $file_name */ /** @var \RecursiveDirectoryIterator $iterator */ - if ($iterator->hasChildren() && !in_array($file->getFilename(), ['.git'], TRUE)) { + if ($iterator->hasChildren() && $file->getFilename() !== '.git') { return TRUE; } $skipped_files = [ @@ -471,7 +477,7 @@ class InPlaceUpdate implements UpdateInterface { */ protected function buildUrl($project_name, $file_name) { $uri = $this->configFactory->get('automatic_updates.settings')->get('download_uri'); - return Url::fromUri($uri . "/$project_name/" . $file_name)->toString(); + return Url::fromUri("$uri/$project_name/$file_name")->toString(); } /** diff --git a/src/Services/ModifiedFiles.php b/src/Services/ModifiedFiles.php index 801894ffbc..0b4473d680 100644 --- a/src/Services/ModifiedFiles.php +++ b/src/Services/ModifiedFiles.php @@ -10,6 +10,7 @@ use Drupal\Signify\ChecksumList; use Drupal\Signify\FailedCheckumFilter; use Drupal\Signify\Verifier; use GuzzleHttp\ClientInterface; +use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Promise\EachPromise; use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; @@ -43,7 +44,7 @@ class ModifiedFiles implements ModifiedFilesInterface { protected $configFactory; /** - * ModifiedCode constructor. + * ModifiedFiles constructor. * * @param \Psr\Log\LoggerInterface $logger * The logger. @@ -63,16 +64,19 @@ class ModifiedFiles implements ModifiedFilesInterface { /** * {@inheritdoc} */ - public function getModifiedFiles(array $extensions = []) { + public function getModifiedFiles(array $extensions = [], $exception_on_failure = FALSE) { $modified_files = new \ArrayIterator(); /** @var \GuzzleHttp\Promise\PromiseInterface[] $promises */ $promises = $this->getHashRequests($extensions); // Wait until all the requests are finished. (new EachPromise($promises, [ 'concurrency' => 4, - 'fulfilled' => function ($resource) use ($modified_files) { + 'fulfilled' => function (array $resource) use ($modified_files) { $this->processHashes($resource, $modified_files); }, + 'rejected' => function (RequestException $exception) use ($exception_on_failure) { + $this->processFailures($exception, $exception_on_failure); + }, ]))->promise()->wait(); return $modified_files; } @@ -80,14 +84,16 @@ class ModifiedFiles implements ModifiedFilesInterface { /** * Process checking hashes of files from external URL. * - * @param array $resource + * @param array $hash * An array of http response and project info. * @param \ArrayIterator $modified_files * The list of modified files. + * + * @throws \SodiumException */ - protected function processHashes(array $resource, \ArrayIterator $modified_files) { - $contents = $resource['contents']; - $info = $resource['info']; + protected function processHashes(array $hash, \ArrayIterator $modified_files) { + $contents = $hash['contents']; + $info = $hash['info']; $directory_root = $info['install path']; if ($info['project'] === 'drupal') { $directory_root = ''; @@ -113,6 +119,21 @@ class ModifiedFiles implements ModifiedFilesInterface { } } + /** + * Handle HTTP failures. + * + * @param \GuzzleHttp\Exception\RequestException $exception + * The request exception. + * @param bool $exception_on_failure + * Throw exception on HTTP failures, defaults to FALSE. + */ + protected function processFailures(RequestException $exception, $exception_on_failure) { + if ($exception_on_failure) { + watchdog_exception('automatic_updates', $exception); + throw $exception; + } + } + /** * Get an iterator of promises that return a resource stream. * @@ -151,13 +172,14 @@ class ModifiedFiles implements ModifiedFilesInterface { return $this->httpClient->requestAsync('GET', $url, [ 'stream' => TRUE, 'read_timeout' => 30, - ]) - ->then(function (ResponseInterface $response) use ($info) { + ])->then( + static function (ResponseInterface $response) use ($info) { return [ 'contents' => $response->getBody()->getContents(), 'info' => $info, ]; - }); + } + ); } /** @@ -174,7 +196,7 @@ class ModifiedFiles implements ModifiedFilesInterface { $project_name = $info['project']; $hash_name = $this->getHashName($info); $uri = ltrim($this->configFactory->get('automatic_updates.settings')->get('hashes_uri'), '/'); - return Url::fromUri($uri . "/$project_name/$version/$hash_name")->toString(); + return Url::fromUri("$uri/$project_name/$version/$hash_name")->toString(); } /** diff --git a/src/Services/ModifiedFilesInterface.php b/src/Services/ModifiedFilesInterface.php index ba46f254ee..44566d3b88 100644 --- a/src/Services/ModifiedFilesInterface.php +++ b/src/Services/ModifiedFilesInterface.php @@ -13,10 +13,12 @@ interface ModifiedFilesInterface { * @param array $extensions * The list of extensions, keyed by extension name with values an info * array. + * @param bool $exception_on_failure + * (optional) Throw exception on HTTP failures, defaults to FALSE. * * @return \Iterator * The modified files. */ - public function getModifiedFiles(array $extensions = []); + public function getModifiedFiles(array $extensions = [], $exception_on_failure = FALSE); } -- GitLab