Issue #3293146 by phenaproxima, TravisCarden, kunal.sachdev: Don't run cron...

Issue #3293146 by phenaproxima, TravisCarden, kunal.sachdev: Don't run cron updates with PHP's built-in web server without an alternate port

automatic_updates.services.yml

@@ -153,6 +153,13 @@ services:
       - '@package_manager.path_locator'
     tags:
       - { name: event_subscriber }
+  automatic_updates.validator.cron_server:
+    class: Drupal\automatic_updates\Validator\CronServerValidator
+    arguments:
+      - '@request_stack'
+      - '@config.factory'
+    tags:
+      - { name: event_subscriber }
   logger.channel.automatic_updates:
     parent: logger.channel_base
     arguments: ['automatic_updates']

src/Validator/CronServerValidator.php

+<?php
+
+namespace Drupal\automatic_updates\Validator;
+
+use Drupal\automatic_updates\CronUpdater;
+use Drupal\automatic_updates\Event\ReadinessCheckEvent;
+use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Http\RequestStack;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\package_manager\Event\PreCreateEvent;
+use Drupal\package_manager\Event\PreOperationStageEvent;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Validates that the current server configuration can run cron updates.
+ *
+ * @internal
+ *   This is an internal part of Automatic Updates and may be changed or removed
+ *   at any time without warning. External code should not interact with this
+ *   class.
+ */
+final class CronServerValidator implements EventSubscriberInterface {
+
+  use StringTranslationTrait;
+
+  /**
+   * The current request.
+   *
+   * @var \Symfony\Component\HttpFoundation\Request
+   */
+  protected $request;
+
+  /**
+   * The config factory service.
+   *
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   */
+  protected $configFactory;
+
+  /**
+   * The type of interface between the web server and the PHP runtime.
+   *
+   * @var string
+   *
+   * @see php_sapi_name()
+   * @see https://www.php.net/manual/en/reserved.constants.php
+   */
+  protected static $serverApi = PHP_SAPI;
+
+  /**
+   * Constructs a CronServerValidator object.
+   *
+   * @param \Drupal\Core\Http\RequestStack $request_stack
+   *   The request stack service.
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config factory service.
+   */
+  public function __construct(RequestStack $request_stack, ConfigFactoryInterface $config_factory) {
+    $this->request = $request_stack->getCurrentRequest();
+    $this->configFactory = $config_factory;
+  }
+
+  /**
+   * Checks that the server is configured correctly to run cron updates.
+   *
+   * @param \Drupal\package_manager\Event\PreOperationStageEvent $event
+   *   The event object.
+   */
+  public function checkServer(PreOperationStageEvent $event): void {
+    if (!$event->getStage() instanceof CronUpdater) {
+      return;
+    }
+
+    $current_port = (int) $this->request->getPort();
+
+    $alternate_port = $this->configFactory->get('automatic_updates.settings')
+      ->get('cron_port');
+    // If no alternate port is configured, it's the same as the current port.
+    $alternate_port = intval($alternate_port) ?: $current_port;
+
+    if (static::$serverApi === 'cli-server' && $current_port === $alternate_port) {
+      // @todo Explain how to fix this problem on our help page, and link to it,
+      //   in https://drupal.org/i/3312669.
+      $event->addError([
+        $this->t('Your site appears to be running on the built-in PHP web server on port @port. Drupal cannot be automatically updated with this configuration unless the site can also be reached on an alternate port.', [
+          '@port' => $current_port,
+        ]),
+      ]);
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getSubscribedEvents() {
+    return [
+      ReadinessCheckEvent::class => 'checkServer',
+      PreCreateEvent::class => 'checkServer',
+    ];
+  }
+
+}

tests/src/Build/CoreUpdateTest.php

@@ -133,7 +133,9 @@ class CoreUpdateTest extends UpdateTestBase {
     $this->createTestProject($template);
 
     $this->visit('/admin/reports/status');
-    $this->getMink()->getSession()->getPage()->clickLink('Run cron');
+    $mink = $this->getMink();
+    $mink->assertSession()->pageTextContains('Your site is ready for automatic updates.');
+    $mink->getSession()->getPage()->clickLink('Run cron');
     $this->assertUpdateSuccessful('9.8.1');
   }
 

tests/src/Kernel/ReadinessValidation/CronServerValidatorTest.php

+<?php
+
+namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
+
+use Drupal\automatic_updates\CronUpdater;
+use Drupal\automatic_updates\Validator\CronServerValidator;
+use Drupal\Core\Logger\RfcLogLevel;
+use Drupal\package_manager\Exception\StageValidationException;
+use Drupal\package_manager\ValidationResult;
+use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase;
+use Psr\Log\Test\TestLogger;
+
+/**
+ * @covers \Drupal\automatic_updates\Validator\CronServerValidator
+ *
+ * @group automatic_updates
+ */
+class CronServerValidatorTest extends AutomaticUpdatesKernelTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['automatic_updates'];
+
+  /**
+   * Data provider for ::testCronServerValidation().
+   *
+   * @return array[]
+   *   Sets of arguments to pass to the test method.
+   */
+  public function providerCronServerValidation(): array {
+    $error = ValidationResult::createError([
+      'Your site appears to be running on the built-in PHP web server on port 80. Drupal cannot be automatically updated with this configuration unless the site can also be reached on an alternate port.',
+    ]);
+
+    return [
+      'PHP server with alternate port' => [
+        TRUE,
+        'cli-server',
+        [CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL],
+        [],
+      ],
+      'PHP server with same port, cron enabled' => [
+        FALSE,
+        'cli-server',
+        [CronUpdater::SECURITY, CronUpdater::ALL],
+        [$error],
+      ],
+      'PHP server with same port, cron disabled' => [
+        FALSE,
+        'cli-server',
+        [CronUpdater::DISABLED],
+        [],
+      ],
+      'other server with alternate port' => [
+        TRUE,
+        'nginx',
+        [CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL],
+        [],
+      ],
+      'other server with same port' => [
+        FALSE,
+        'nginx',
+        [CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL],
+        [],
+      ],
+    ];
+  }
+
+  /**
+   * Tests server configuration validation for unattended updates.
+   *
+   * @param bool $alternate_port
+   *   Whether or not an alternate port should be set.
+   * @param string $server_api
+   *   The value of the PHP_SAPI constant, as known to the validator.
+   * @param string[] $cron_modes
+   *   The cron modes to test with. Can contain any of
+   *   \Drupal\automatic_updates\CronUpdater::DISABLED,
+   *   \Drupal\automatic_updates\CronUpdater::SECURITY, and
+   *   \Drupal\automatic_updates\CronUpdater::ALL.
+   * @param \Drupal\package_manager\ValidationResult[] $expected_results
+   *   The expected validation results.
+   *
+   * @dataProvider providerCronServerValidation
+   */
+  public function testCronServerValidation(bool $alternate_port, string $server_api, array $cron_modes, array $expected_results): void {
+    $request = $this->container->get('request_stack')->getCurrentRequest();
+    $this->assertNotEmpty($request);
+    $this->assertSame(80, $request->getPort());
+
+    $property = new \ReflectionProperty(CronServerValidator::class, 'serverApi');
+    $property->setAccessible(TRUE);
+    $property->setValue(NULL, $server_api);
+
+    foreach ($cron_modes as $mode) {
+      $this->config('automatic_updates.settings')
+        ->set('cron', $mode)
+        ->set('cron_port', $alternate_port ? 2501 : 0)
+        ->save();
+
+      $this->assertCheckerResultsFromManager($expected_results, TRUE);
+
+      $logger = new TestLogger();
+      $this->container->get('logger.factory')
+        ->get('automatic_updates')
+        ->addLogger($logger);
+
+      // If errors were expected, cron should not have run.
+      $this->container->get('cron')->run();
+      if ($expected_results) {
+        $error = new StageValidationException($expected_results);
+        $this->assertTrue($logger->hasRecord($error->getMessage(), RfcLogLevel::ERROR));
+      }
+      else {
+        $this->assertFalse($logger->hasRecords(RfcLogLevel::ERROR));
+      }
+    }
+  }
+
+}