Issue #3094445 by heddn, bdragon: Code scan findings

automatic_updates.install

@@ -31,7 +31,7 @@ function automatic_updates_requirements($phase) {
     ];
   }
   if ($phase !== 'runtime') {
-    return;
+    return NULL;
   }
 
   $requirements = [];

automatic_updates.module

@@ -68,6 +68,13 @@ function automatic_updates_page_top(array &$page_top) {
  * Implements hook_cron().
  */
 function automatic_updates_cron() {
+  $state = \Drupal::state();
+  $request_time = \Drupal::time()->getRequestTime();
+  $last_check = $state->get('automatic_updates.last_check', 0);
+  // Only allow cron to run once every hour.
+  if (($request_time - $last_check) < 3600) {
+    return;
+  }
   // In-place updates won't function for dev releases of Drupal core.
   $dev_core = strpos(\Drupal::VERSION, '-dev') !== FALSE;
   /** @var \Drupal\Core\Config\ImmutableConfig $config */
@@ -104,6 +111,7 @@ function automatic_updates_cron() {
   foreach ($checker->getCategories() as $category) {
     $checker->run($category);
   }
+  $state->set('automatic_updates.last_check', \Drupal::time()->getCurrentTime());
 }
 
 /**

automatic_updates.services.yml

@@ -56,8 +56,8 @@ services:
   automatic_updates.readonly_checker:
     class: Drupal\automatic_updates\ReadinessChecker\ReadOnlyFilesystem
     arguments:
-      - '@logger.channel.automatic_updates'
       - '@app.root'
+      - '@logger.channel.automatic_updates'
       - '@file_system'
     tags:
       - { name: readiness_checker, priority: 100, category: error }

src/Controller/ReadinessCheckerController.php

@@ -51,8 +51,9 @@ class ReadinessCheckerController extends ControllerBase {
   public function run() {
     $messages = [];
     foreach ($this->checker->getCategories() as $category) {
-      $messages = array_merge($this->checker->run($category), $messages);
+      $messages[] = $this->checker->run($category);
     }
+    $messages = array_merge(...$messages);
     if (empty($messages)) {
       $this->messenger()->addStatus($this->t('No issues found. Your site is completely ready for <a href="@readiness_checks">automatic updates</a>.', ['@readiness_checks' => 'https://www.drupal.org/docs/8/update/automatic-updates#readiness-checks']));
     }

src/Form/SettingsForm.php

@@ -76,7 +76,7 @@ class SettingsForm extends ConfigFormBase {
 
     $form['psa'] = [
       '#type' => 'details',
-      '#title' => $this->t('Public serivice announcements'),
+      '#title' => $this->t('Public service announcements'),
       '#open' => TRUE,
     ];
     $form['psa']['description'] = [

src/IgnoredPathsTrait.php

@@ -21,6 +21,7 @@ trait IgnoredPathsTrait {
     if ($this->getPathMatcher()->matchPath($file_path, $paths)) {
       return TRUE;
     }
+    return FALSE;
   }
 
   /**

src/ProjectInfoTrait.php

@@ -70,7 +70,7 @@ trait ProjectInfoTrait {
       return $info['version'];
     }
     // Handle experimental modules from core.
-    if (strpos($info['install path'], "core") === 0) {
+    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]);
@@ -145,6 +145,7 @@ trait ProjectInfoTrait {
     catch (\Throwable $exception) {
       \Drupal::logger('automatic_updates')->error('Composer.json could not be located for @extension', ['@extension' => $extension_name]);
     }
+    return NULL;
   }
 
 }

src/ReadinessChecker/DiskSpace.php

@@ -2,6 +2,8 @@
 
 namespace Drupal\automatic_updates\ReadinessChecker;
 
+use Drupal\Component\FileSystem\FileSystem as FileSystemComponent;
+
 /**
  * Disk space checker.
  */
@@ -52,6 +54,13 @@ class DiskSpace extends Filesystem {
         '@space' => static::MINIMUM_DISK_SPACE / static::MEGABYTE_DIVISOR,
       ]);
     }
+    $temp = FileSystemComponent::getOsTemporaryDirectory();
+    if (disk_free_space($temp) < static::MINIMUM_DISK_SPACE) {
+      $messages[] = $this->t('Directory "@temp" has insufficient space. There must be at least @space megabytes free.', [
+        '@temp' => $temp,
+        '@space' => static::MINIMUM_DISK_SPACE / static::MEGABYTE_DIVISOR,
+      ]);
+    }
     return $messages;
   }
 

src/ReadinessChecker/ModifiedFiles.php

@@ -65,9 +65,9 @@ class ModifiedFiles implements ReadinessCheckerInterface {
    */
   public function __construct(ModifiedFilesInterface $modified_files, ExtensionList $modules, ExtensionList $profiles, ExtensionList $themes) {
     $this->modifiedFiles = $modified_files;
-    $this->modules = $modules;
-    $this->profiles = $profiles;
-    $this->themes = $themes;
+    $this->module = $modules;
+    $this->profile = $profiles;
+    $this->theme = $themes;
   }
 
   /**

src/ReadinessChecker/ReadOnlyFilesystem.php

@@ -29,16 +29,16 @@ class ReadOnlyFilesystem extends Filesystem {
   /**
    * ReadOnlyFilesystem constructor.
    *
-   * @param \Psr\Log\LoggerInterface $logger
-   *   The logger.
    * @param string $app_root
    *   The app root.
+   * @param \Psr\Log\LoggerInterface $logger
+   *   The logger.
    * @param \Drupal\Core\File\FileSystemInterface $file_system
    *   The file system service.
    */
-  public function __construct(LoggerInterface $logger, $app_root, FileSystemInterface $file_system) {
+  public function __construct($app_root, LoggerInterface $logger, FileSystemInterface $file_system) {
+    parent::__construct($app_root);
     $this->logger = $logger;
-    $this->rootPath = (string) $app_root;
     $this->fileSystem = $file_system;
   }
 

src/Services/AutomaticUpdatesPsa.php

@@ -135,8 +135,8 @@ class AutomaticUpdatesPsa implements AutomaticUpdatesPsaInterface {
     }
 
     try {
-      $json_payload = json_decode($response);
-      if (!is_null($json_payload)) {
+      $json_payload = json_decode($response, FALSE);
+      if ($json_payload !== NULL) {
         foreach ($json_payload as $json) {
           if ($json->is_psa && ($json->type === 'core' || $this->isValidExtension($json->type, $json->project))) {
             $messages[] = $this->message($json->title, $json->link);
@@ -192,15 +192,15 @@ class AutomaticUpdatesPsa implements AutomaticUpdatesPsaInterface {
    */
   protected function contribParser(array &$messages, $json) {
     $extension_version = $this->{$json->type}->getAllAvailableInfo()[$json->project]['version'];
-    $json->insecure = array_filter(array_map(function ($version) {
+    $json->insecure = array_filter(array_map(static function ($version) {
       $version_array = explode('-', $version, 2);
       if ($version_array && $version_array[0] === \Drupal::CORE_COMPATIBILITY) {
         return isset($version_array[1]) ? $version_array[1] : NULL;
       }
-      elseif (count($version_array) === 1) {
+      if (count($version_array) === 1) {
         return $version_array[0];
       }
-      elseif (count($version_array) === 2 && $version_array[1] === 'dev') {
+      if (count($version_array) === 2 && $version_array[1] === 'dev') {
         return $version;
       }
     }, $json->insecure));

src/Services/InPlaceUpdate.php

@@ -120,7 +120,7 @@ class InPlaceUpdate implements UpdateInterface {
     $this->fileSystem = $file_system;
     $this->httpClient = $http_client;
     $this->rootPath = (string) $app_root;
-    $this->vendorPath = $this->rootPath . DIRECTORY_SEPARATOR . 'vendor';
+    $this->vendorPath = $this->rootPath . DIRECTORY_SEPARATOR . 'vendor' . DIRECTORY_SEPARATOR;
     $project_root = drupal_get_path('module', 'automatic_updates');
     require_once $project_root . DIRECTORY_SEPARATOR . 'vendor' . DIRECTORY_SEPARATOR . 'autoload.php';
   }
@@ -173,7 +173,7 @@ class InPlaceUpdate implements UpdateInterface {
   protected function getArchive($project_name, $from_version, $to_version) {
     $quasi_patch = $this->getQuasiPatchFileName($project_name, $from_version, $to_version);
     $url = $this->buildUrl($project_name, $quasi_patch);
-    $temp_directory = $this->getTempDirectory();
+    $temp_directory = FileSystem::getOsTemporaryDirectory() . DIRECTORY_SEPARATOR;
     $destination = $this->fileSystem->getDestinationFilename($temp_directory . $quasi_patch, FileSystemInterface::EXISTS_REPLACE);
     $this->doGetResource($url, $destination);
     $csig_file = $quasi_patch . '.csig';
@@ -226,6 +226,7 @@ class InPlaceUpdate implements UpdateInterface {
       }
       $this->stripFileDirectoryPath($archive_file);
     }
+    unset($archive_file);
     if ($intersection = array_intersect($files, $archive_files)) {
       $this->logger->error('Can not update because %count files are modified: %paths', [
         '%count' => count($intersection),
@@ -524,7 +525,7 @@ class InPlaceUpdate implements UpdateInterface {
    *   The real path of a file.
    */
   protected function getProjectRealPath($file_path, $project_root) {
-    if (substr($file_path, 0, 6) === 'vendor/') {
+    if (strpos($file_path, 'vendor' . DIRECTORY_SEPARATOR) === 0) {
       return $this->vendorPath . substr($file_path, 7);
     }
     return rtrim($project_root, '/\\') . DIRECTORY_SEPARATOR . $file_path;
@@ -569,7 +570,7 @@ class InPlaceUpdate implements UpdateInterface {
   }
 
   /**
-   * Clear opcode cache on successful update.
+   * Clear Opcode cache on successful update.
    */
   protected function clearOpcodeCache() {
     if (function_exists('opcache_reset')) {

src/Services/Notify.php

@@ -150,6 +150,9 @@ class Notify implements NotifyInterface {
    *   The email address where the message will be sent.
    * @param array $params
    *   Parameters to build the email.
+   *
+   * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
+   * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException
    */
   protected function doSend($to, array $params) {
     $users = $this->entityTypeManager->getStorage('user')

tests/modules/test_automatic_updates/src/Controller/ModifiedFilesController.php

@@ -56,14 +56,14 @@ class ModifiedFilesController extends ControllerBase {
     // Special edge case for core.
     if ($project_type === 'core') {
       $infos = $this->getInfos('module');
-      $extensions = array_filter($infos, function (array $info) {
+      $extensions = array_filter($infos, static function (array $info) {
         return $info['project'] === 'drupal';
       });
     }
     // Filter for the main project.
     else {
       $infos = $this->getInfos($project_type);
-      $extensions = array_filter($infos, function (array $info) use ($extension, $project_type) {
+      $extensions = array_filter($infos, static function (array $info) use ($extension, $project_type) {
         return $info['install path'] === "{$project_type}s/contrib/$extension";
       });
     }

tests/src/Build/InPlaceUpdateTest.php

@@ -258,7 +258,7 @@ class InPlaceUpdateTest extends QuickStartTestBase {
     $this->deletions = [];
     $http_client = new Client();
     $filesystem = new SymfonyFilesystem();
-    $this->deletionsDestination = DrupalFileSystem::getOsTemporaryDirectory() . DIRECTORY_SEPARATOR . "$project-" . rand(10000, 99999) . microtime(TRUE);
+    $this->deletionsDestination = DrupalFileSystem::getOsTemporaryDirectory() . DIRECTORY_SEPARATOR . "$project-" . mt_rand(10000, 99999) . microtime(TRUE);
     $filesystem->mkdir($this->deletionsDestination);
     $file_name = "$project-$from_version-to-$to_version.zip";
     $zip_file = $this->deletionsDestination . DIRECTORY_SEPARATOR . $file_name;

tests/src/Kernel/ReadinessChecker/CronFrequencyTest.php

@@ -39,7 +39,7 @@ class CronFrequencyTest extends KernelTestBase {
       ->set('interval', 21600)
       ->save();
     $messages = $this->container->get('automatic_updates.cron_frequency')->run();
-    $this->assertEquals('Cron is not set to run frequently enough. <a href="/admin/config/system/cron">Configure it</a> to run at least every 3 hours or disable automated cron and run it via an external scheduling system.', $messages[0]);
+    self::assertEquals('Cron is not set to run frequently enough. <a href="/admin/config/system/cron">Configure it</a> to run at least every 3 hours or disable automated cron and run it via an external scheduling system.', $messages[0]);
   }
 
 }

tests/src/Kernel/ReadinessChecker/DiskSpaceTest.php

@@ -33,12 +33,12 @@ class DiskSpaceTest extends KernelTestBase {
     // Out of space.
     $disk_space = new TestDiskSpace($this->container->get('app.root'));
     $messages = $disk_space->run();
-    $this->assertCount(1, $messages);
+    $this->assertCount(2, $messages);
 
     // Out of space not the same logical disk.
     $disk_space = new TestDiskSpaceNonSameDisk($this->container->get('app.root'));
     $messages = $disk_space->run();
-    $this->assertCount(2, $messages);
+    $this->assertCount(3, $messages);
   }
 
 }

tests/src/Kernel/ReadinessChecker/OpcodeCacheTest.php

@@ -28,7 +28,7 @@ class OpcodeCacheTest extends KernelTestBase {
     $messages = $this->container->get('automatic_updates.opcode_cache')->run();
     if ($failure) {
       $this->assertNotEmpty($messages);
-      $this->assertEquals((string) $messages[0], 'Automatic updates cannot run via CLI  when opcode file cache is enabled.');
+      self::assertEquals((string) $messages[0], 'Automatic updates cannot run via CLI  when opcode file cache is enabled.');
     }
     else {
       $this->assertEmpty($messages);

tests/src/Kernel/ReadinessChecker/PendingDbUpdatesTest.php

@@ -27,7 +27,7 @@ class PendingDbUpdatesTest extends KernelTestBase {
     $this->assertEmpty($messages);
 
     $messages = (new TestPendingDbUpdates())->run();
-    $this->assertEquals('There are pending database updates, therefore updates cannot be applied. Please run update.php.', $messages[0]);
+    self::assertEquals('There are pending database updates, therefore updates cannot be applied. Please run update.php.', $messages[0]);
   }
 
 }

tests/src/Kernel/ReadinessChecker/PhpSapiTest.php

@@ -29,7 +29,7 @@ class PhpSapiTest extends KernelTestBase {
 
     $this->container->get('state')->set('automatic_updates.php_sapi', 'foo');
     $messages = $this->container->get('automatic_updates.php_sapi')->run();
-    $this->assertEquals('PHP changed from running as "foo" to "cli". This can lead to inconsistent and misleading results.', $messages[0]);
+    self::assertEquals('PHP changed from running as "foo" to "cli". This can lead to inconsistent and misleading results.', $messages[0]);
   }
 
 }