Issue #3316668 by tedbow, phenaproxima, yash.rode, Wim Leers, TravisCarden:...

Issue #3316668 by tedbow, phenaproxima, yash.rode, Wim Leers, TravisCarden: ComposerSettingsValidator should run `composer config` to determine if HTTPS is enabled

package_manager/package_manager.services.yml

@@ -55,6 +55,10 @@ services:
       - '@update.manager'
     tags:
       - { name: event_subscriber }
+  package_manager.composer_inspector:
+    class: Drupal\package_manager\ComposerInspector
+    autowire: true
+    public: false
 
   # Validators.
   package_manager.validator.environment_support:
@@ -105,6 +109,8 @@ services:
     class: Drupal\package_manager\Validator\ComposerSettingsValidator
     arguments:
       - '@string_translation'
+      - '@package_manager.composer_inspector'
+      - '@package_manager.path_locator'
     tags:
       - { name: event_subscriber }
   package_manager.validator.multisite:

package_manager/src/ComposerInspector.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\package_manager;
+
+use PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ComposerRunnerInterface;
+
+/**
+ * Defines a class to get information from Composer.
+ *
+ * @internal
+ *   This is an internal part of Package Manager and may be changed or removed
+ *   at any time without warning. External code should not interact with this
+ *   class.
+ */
+final class ComposerInspector {
+
+  /**
+   * The Composer runner service from Composer Stager.
+   *
+   * @var \PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ComposerRunnerInterface
+   */
+  protected ComposerRunnerInterface $runner;
+
+  /**
+   * The JSON process output callback.
+   *
+   * @var \Drupal\package_manager\JsonProcessOutputCallback
+   */
+  private JsonProcessOutputCallback $jsonCallback;
+
+  /**
+   * Constructs a ComposerInspector object.
+   *
+   * @param \PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ComposerRunnerInterface $runner
+   *   The Composer runner service from Composer Stager.
+   */
+  public function __construct(ComposerRunnerInterface $runner) {
+    $this->runner = $runner;
+    $this->jsonCallback = new JsonProcessOutputCallback();
+  }
+
+  /**
+   * Returns a config value from Composer.
+   *
+   * @param string $key
+   *   The config key to get.
+   * @param string $working_dir
+   *   The working directory in which to run Composer.
+   *
+   * @return mixed|null
+   *   The output data.
+   */
+  public function getConfig(string $key, string $working_dir) {
+    $this->runner->run(['config', $key, "--working-dir=$working_dir", '--json'], $this->jsonCallback);
+    return $this->jsonCallback->getOutputData();
+  }
+
+  /**
+   * Returns the current Composer version.
+   *
+   * @param string $working_dir
+   *   The working directory in which to run Composer.
+   *
+   * @return string
+   *   The Composer version.
+   *
+   * @throws \UnexpectedValueException
+   *   Thrown if the expect data format is not found.
+   */
+  public function getVersion(string $working_dir): string {
+    $this->runner->run(['--format=json', "--working-dir=$working_dir"], $this->jsonCallback);
+    $data = $this->jsonCallback->getOutputData();
+    if (isset($data['application']['name'])
+      && isset($data['application']['version'])
+      && $data['application']['name'] === 'Composer'
+      && is_string($data['application']['version'])) {
+      return $data['application']['version'];
+    }
+    throw new \UnexpectedValueException('Unable to determine Composer version');
+  }
+
+}

package_manager/src/JsonProcessOutputCallback.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\package_manager;
+
+use Drupal\Component\Serialization\Json;
+use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
+
+/**
+ * A process callback for handling output in the JSON format.
+ *
+ * @internal
+ *   This is an internal part of Package Manager and may be changed or removed
+ *   at any time without warning. External code should not interact with this
+ *   class.
+ */
+final class JsonProcessOutputCallback implements ProcessOutputCallbackInterface {
+
+  /**
+   * The output buffer.
+   *
+   * @var string
+   */
+  private string $outBuffer = '';
+
+  /**
+   * The error buffer.
+   *
+   * @var string
+   */
+  private string $errorBuffer = '';
+
+  /**
+   * {@inheritdoc}
+   */
+  public function __invoke(string $type, string $buffer): void {
+    // @todo Support self::ERR output in
+    //   https://www.drupal.org/project/automatic_updates/issues/3337504.
+    if ($type === self::OUT) {
+      $this->outBuffer .= $buffer;
+      return;
+    }
+    elseif ($type === self::ERR) {
+      $this->errorBuffer .= $buffer;
+      return;
+    }
+    throw new \InvalidArgumentException("Unsupported output type: '$type'");
+  }
+
+  /**
+   * Gets the output data.
+   *
+   * @return mixed|null
+   *   The output data or NULL if there was an exception.
+   *
+   * @throws \Exception
+   *   Thrown if error buffer was not empty.
+   */
+  public function getOutputData() {
+    $error = $this->errorBuffer;
+    $out = $this->outBuffer;
+    $this->errorBuffer = '';
+    $this->outBuffer = '';
+    if ($error !== '') {
+      // @todo Handle deprecations messages in the error output in
+      //   https://drupal.org/i/3337667.
+      throw new \Exception($error);
+    }
+    return Json::decode($out);
+  }
+
+}

package_manager/src/Validator/ComposerSettingsValidator.php

@@ -6,10 +6,12 @@ namespace Drupal\package_manager\Validator;
 
 use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Drupal\Core\StringTranslation\TranslationInterface;
+use Drupal\package_manager\ComposerInspector;
 use Drupal\package_manager\Event\PreApplyEvent;
 use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Event\PreOperationStageEvent;
 use Drupal\package_manager\Event\StatusCheckEvent;
+use Drupal\package_manager\PathLocator;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 /**
@@ -24,26 +26,50 @@ final class ComposerSettingsValidator implements EventSubscriberInterface {
 
   use StringTranslationTrait;
 
+  /**
+   * The Composer inspector service.
+   *
+   * @var \Drupal\package_manager\ComposerInspector
+   */
+  protected ComposerInspector $inspector;
+
+  /**
+   * The path locator service.
+   *
+   * @var \Drupal\package_manager\PathLocator
+   */
+  protected PathLocator $pathLocator;
+
   /**
    * Constructs a ComposerSettingsValidator object.
    *
    * @param \Drupal\Core\StringTranslation\TranslationInterface $translation
    *   The string translation service.
+   * @param \Drupal\package_manager\ComposerInspector $inspector
+   *   The Composer inspector service.
+   * @param \Drupal\package_manager\PathLocator $path_locator
+   *   The path locator service.
    */
-  public function __construct(TranslationInterface $translation) {
+  public function __construct(TranslationInterface $translation, ComposerInspector $inspector, PathLocator $path_locator) {
     $this->setStringTranslation($translation);
+    $this->inspector = $inspector;
+    $this->pathLocator = $path_locator;
   }
 
   /**
    * {@inheritdoc}
    */
   public function validateStagePreOperation(PreOperationStageEvent $event): void {
-    $config = $event->getStage()
-      ->getActiveComposer()
-      ->getComposer()
-      ->getConfig();
+    $dir = $this->pathLocator->getProjectRoot();
 
-    if ($config->get('secure-http') !== TRUE) {
+    try {
+      $setting = (int) $this->inspector->getConfig('secure-http', $dir);
+    }
+    catch (\Exception $exception) {
+      $event->addErrorFromThrowable($exception, $this->t('Unable to determine Composer <code>secure-http</code> setting.'));
+      return;
+    }
+    if ($setting !== 1) {
       $event->addError([
         $this->t('HTTPS must be enabled for Composer downloads. See <a href=":url">the Composer documentation</a> for more information.', [
           ':url' => 'https://getcomposer.org/doc/06-config.md#secure-http',

package_manager/tests/fixtures/fake_site/composer.json

@@ -4,5 +4,17 @@
   },
   "require-dev": {
     "drupal/core-dev": "^9"
+  },
+  "extra": {
+    "boo": "boo boo",
+    "foo": {
+        "dev": "2.x-dev"
+    },
+    "foo-bar": true,
+    "boo-far": {
+        "foo": 1.23,
+        "bar": 134,
+        "foo-bar": null
+    }
   }
 }

package_manager/tests/src/Kernel/ComposerInspectorTest.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Tests\package_manager\Kernel;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\KernelTests\KernelTestBase;
+use PhpTuf\ComposerStager\Domain\Exception\RuntimeException;
+
+/**
+ * @coversDefaultClass \Drupal\package_manager\ComposerInspector
+ *
+ * @group package_manager
+ */
+class ComposerInspectorTest extends KernelTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['update', 'package_manager'];
+
+  /**
+   * @covers ::getConfig
+   */
+  public function testConfig(): void {
+    $dir = __DIR__ . '/../../fixtures/fake_site';
+    $inspector = $this->container->get('package_manager.composer_inspector');
+    $this->assertSame(1, $inspector->getConfig('secure-http', $dir));
+
+    $this->assertSame([
+      'boo' => 'boo boo',
+      "foo" => ["dev" => "2.x-dev"],
+      "foo-bar" => TRUE,
+      "boo-far" => [
+        "foo" => 1.23,
+        "bar" => 134,
+        "foo-bar" => NULL,
+      ],
+    ], $inspector->getConfig('extra', $dir));
+
+    $this->expectException(RuntimeException::class);
+    $inspector->getConfig('non-existent-config', $dir);
+  }
+
+  /**
+   * @covers ::getVersion
+   */
+  public function testGetVersion() {
+    $dir = __DIR__ . '/../../fixtures/fake_site';
+    $inspector = $this->container->get('package_manager.composer_inspector');
+    $version = $inspector->getVersion($dir);
+    // We can assert an exact version of Composer, but we can assert that the
+    // number is in the expected 'MAJOR.MINOR.PATCH' format.
+    $parts = explode('.', $version);
+    $this->assertCount(3, $parts);
+    $this->assertCount(3, array_filter($parts, 'is_numeric'));
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function register(ContainerBuilder $container) {
+    parent::register($container);
+    $container->getDefinition('package_manager.composer_inspector')->setPublic(TRUE);
+  }
+
+}

package_manager/tests/src/Kernel/LockFileValidatorTest.php

@@ -75,7 +75,9 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
     // priority of 0, this listener changes lock file before the validator
     // runs.
     $this->addEventTestListener(function () {
-      file_put_contents($this->activeDir . '/composer.lock', 'changed');
+      $lock = json_decode(file_get_contents($this->activeDir . '/composer.lock'), TRUE);
+      $lock['extra']['key'] = 'value';
+      file_put_contents($this->activeDir . '/composer.lock', json_encode($lock, JSON_THROW_ON_ERROR));
     }, $event_class);
     $result = ValidationResult::createError([
       t('Unexpected changes were detected in composer.lock, which indicates that other Composer operations were performed since this Package Manager operation started. This can put the code base into an unreliable state and therefore is not allowed.'),

package_manager/tests/src/Unit/JsonProcessOutputCallbackTest.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Tests\package_manager\Unit;
+
+use Drupal\package_manager\JsonProcessOutputCallback;
+use Drupal\Tests\UnitTestCase;
+use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface;
+
+/**
+ * @coversDefaultClass \Drupal\package_manager\JsonProcessOutputCallback
+ *
+ * @group package_manager
+ */
+class JsonProcessOutputCallbackTest extends UnitTestCase {
+
+  /**
+   * @covers ::__invoke
+   * @covers ::getOutputData
+   */
+  public function testGetOutputData(): void {
+    // Create a data array that has 1 '*' character to allow easily splitting
+    // up the JSON encoded string over multiple lines.
+    $data = [
+      'value' => 'I have value!*',
+      'another value' => 'I have another value!',
+      'one' => 1,
+    ];
+    $json_string = json_encode($data, JSON_THROW_ON_ERROR);
+    $lines = explode('*', $json_string);
+    $lines[0] .= '*';
+    // Confirm that 2 string concatenated together will recreate the original
+    // data.
+    $this->assertSame($data, json_decode($lines[0] . $lines[1], TRUE));
+    $callback = new JsonProcessOutputCallback();
+    $callback(ProcessOutputCallbackInterface::OUT, $lines[0]);
+    $callback(ProcessOutputCallbackInterface::OUT, $lines[1]);
+    $this->assertSame($data, $callback->getOutputData());
+
+    $callback = new JsonProcessOutputCallback();
+    $callback(ProcessOutputCallbackInterface::OUT, '1');
+    $this->assertSame(1, $callback->getOutputData());
+  }
+
+  /**
+   * @covers ::getOutputData
+   */
+  public function testNoInvokeCall(): void {
+    $callback = new JsonProcessOutputCallback();
+    $this->assertSame(NULL, $callback->getOutputData());
+  }
+
+  /**
+   * @covers ::getOutputData
+   */
+  public function testError(): void {
+    $callback = new JsonProcessOutputCallback();
+    $callback(ProcessOutputCallbackInterface::OUT, '1');
+    $callback(ProcessOutputCallbackInterface::ERR, 'Oh no, what happened!!!!!');
+    $callback(ProcessOutputCallbackInterface::OUT, '2');
+    $callback(ProcessOutputCallbackInterface::ERR, 'Really what happened????');
+    $this->expectException(\Exception::class);
+    $this->expectExceptionMessage('Oh no, what happened!!!!!Really what happened????');
+    $callback->getOutputData();
+  }
+
+}