Issue #3347031 by phenaproxima, Wim Leers: Stage::validatePackageNames()...

Issue #3347031 by phenaproxima, Wim Leers: Stage::validatePackageNames() should not use the Composer API

dictionary.txt

@@ -9,3 +9,4 @@ testlogger
 unwritable
 filedate
 unshallow
+hhvm

package_manager/src/Stage.php

@@ -4,6 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\package_manager;
 
+use Composer\Semver\VersionParser;
 use Drupal\Component\Datetime\TimeInterface;
 use Drupal\Component\Utility\Crypt;
 use Drupal\Core\File\Exception\FileException;
@@ -121,6 +122,29 @@ class Stage implements LoggerAwareInterface {
    */
   private const TEMPSTORE_DESTROYED_STAGES_INFO_PREFIX = 'TEMPSTORE_DESTROYED_STAGES_INFO';
 
+  /**
+   * The regular expression to check if a package name is a platform package.
+   *
+   * @var string
+   *
+   * @see \Composer\Repository\PlatformRepository::PLATFORM_PACKAGE_REGEX
+   * @see ::validateRequirements()
+   */
+  private const COMPOSER_PLATFORM_PACKAGE_REGEX = '{^(?:php(?:-64bit|-ipv6|-zts|-debug)?|hhvm|(?:ext|lib)-[a-z0-9](?:[_.-]?[a-z0-9]+)*|composer(?:-(?:plugin|runtime)-api)?)$}iD';
+
+  /**
+   * The regular expression to check if a package name is a regular package.
+   *
+   * If you try to require an invalid package name, this is the regular
+   * expression that Composer will, at the command line, tell you to match.
+   *
+   * @var string
+   *
+   * @see \Composer\Package\Loader\ValidatingArrayLoader::hasPackageNamingError()
+   * @see ::validateRequirements()
+   */
+  private const COMPOSER_PACKAGE_REGEX = '/^[a-z0-9]([_.-]?[a-z0-9]+)*\/[a-z0-9](([_.]?|-{0,2})[a-z0-9]+)*$/';
+
   /**
    * The lock info for the stage.
    *
@@ -327,12 +351,12 @@ class Stage implements LoggerAwareInterface {
     // Change the runtime and dev requirements as needed, but don't update
     // the installed packages yet.
     if ($runtime) {
-      $this->validatePackageNames($runtime);
+      self::validateRequirements($runtime);
       $command = array_merge(['require', '--no-update'], $runtime);
       $this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout);
     }
     if ($dev) {
-      $this->validatePackageNames($dev);
+      self::validateRequirements($dev);
       $command = array_merge(['require', '--dev', '--no-update'], $dev);
       $this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout);
     }
@@ -726,10 +750,10 @@ class Stage implements LoggerAwareInterface {
    * Validates a set of package names.
    *
    * Package names are considered invalid if they look like Drupal project
-   * names. The only exceptions to this are `php` and `composer`, which Composer
-   * treats as legitimate requirements.
+   * names. The only exceptions to this are platform requirements, like `php`,
+   * `composer`, or `ext-json`, which are legitimate to Composer.
    *
-   * @param string[] $package_versions
+   * @param string[] $requirements
    *   A set of package names (with or without version constraints), as passed
    *   to ::require().
    *
@@ -738,10 +762,18 @@ class Stage implements LoggerAwareInterface {
    *
    * @see https://getcomposer.org/doc/articles/composer-platform-dependencies.md
    */
-  protected function validatePackageNames(array $package_versions): void {
-    foreach ($package_versions as $package_name) {
-      if (!ComposerUtility::isValidRequirement($package_name)) {
-        throw new \InvalidArgumentException("Invalid package name '$package_name'.");
+  protected static function validateRequirements(array $requirements): void {
+    $version_parser = new VersionParser();
+
+    foreach ($requirements as $requirement) {
+      $parts = explode(':', $requirement, 2);
+      $name = $parts[0];
+
+      if (!preg_match(self::COMPOSER_PLATFORM_PACKAGE_REGEX, $name) && !preg_match(self::COMPOSER_PACKAGE_REGEX, $name)) {
+        throw new \InvalidArgumentException("Invalid package name '$name'.");
+      }
+      if (count($parts) > 1) {
+        $version_parser->parseConstraints($parts[1]);
       }
     }
   }

package_manager/tests/src/Kernel/StageTest.php

@@ -544,42 +544,6 @@ class StageTest extends PackageManagerKernelTestBase {
     $this->assertFalse($logger->hasRecord($warning_message, LogLevel::WARNING));
   }
 
-  /**
-   * @covers ::validatePackageNames
-   *
-   * @param string $package_name
-   *   The package name.
-   * @param bool $is_invalid
-   *   TRUE if the given package name is invalid and will cause an exception,
-   *   FALSE otherwise.
-   *
-   * @dataProvider providerValidatePackageNames
-   */
-  public function testValidatePackageNames(string $package_name, bool $is_invalid): void {
-    $stage = $this->createStage();
-    $stage->create();
-    if ($is_invalid) {
-      $this->expectException(\InvalidArgumentException::class);
-      $this->expectExceptionMessage("Invalid package name '$package_name'.");
-    }
-    $stage->require([$package_name]);
-    // If we got here, the package name is valid and we just need to assert something so PHPUnit doesn't complain.
-    $this->assertTrue(TRUE);
-  }
-
-  /**
-   * Data provider for testValidatePackageNames.
-   *
-   * @return array[]
-   *   The test cases.
-   */
-  public function providerValidatePackageNames(): array {
-    return [
-      'Full package name' => ['drupal/semver_test', FALSE],
-      'Bare Drupal project name' => ['semver_test', TRUE],
-    ];
-  }
-
   /**
    * Tests that ignored paths are collected before create and apply.
    */

package_manager/tests/src/Unit/StageTest.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Tests\package_manager\Unit;
+
+use Drupal\package_manager\Stage;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\package_manager\Stage
+ * @group package_manager
+ * @internal
+ */
+class StageTest extends UnitTestCase {
+
+  /**
+   * @covers ::validateRequirements
+   *
+   * @param string|null $expected_exception
+   *   The exception class that should be thrown, or NULL if there should not be
+   *   any exception.
+   * @param string $requirement
+   *   The requirement (package name and optional constraint) to validate.
+   *
+   * @dataProvider providerValidateRequirements
+   */
+  public function testValidateRequirements(?string $expected_exception, string $requirement): void {
+    $reflector = new \ReflectionClass(Stage::class);
+    $method = $reflector->getMethod('validateRequirements');
+    $method->setAccessible(TRUE);
+
+    if ($expected_exception) {
+      $this->expectException($expected_exception);
+    }
+    else {
+      $this->assertNull($expected_exception);
+    }
+
+    $method->invoke(NULL, [$requirement]);
+  }
+
+  /**
+   * Data provider for testValidateRequirements.
+   *
+   * @return array[]
+   *   The test cases.
+   */
+  public function providerValidateRequirements(): array {
+    return [
+      // Valid requirements.
+      [NULL, 'vendor/package'],
+      [NULL, 'vendor/snake_case'],
+      [NULL, 'vendor/kebab-case'],
+      [NULL, 'vendor/with.dots'],
+      [NULL, '1vendor2/3package4'],
+      [NULL, 'vendor/package:1'],
+      [NULL, 'vendor/package:1.2'],
+      [NULL, 'vendor/package:1.2.3'],
+      [NULL, 'vendor/package:1.x'],
+      [NULL, 'vendor/package:^1'],
+      [NULL, 'vendor/package:~1'],
+      [NULL, 'vendor/package:>1'],
+      [NULL, 'vendor/package:<1'],
+      [NULL, 'vendor/package:>=1'],
+      [NULL, 'vendor/package:>1 <2'],
+      [NULL, 'vendor/package:1 || 2'],
+      [NULL, 'vendor/package:>=1,<1.1.0'],
+      [NULL, 'vendor/package:1a'],
+      [NULL, 'vendor/package:*'],
+      [NULL, 'vendor/package:dev-master'],
+      [NULL, 'vendor/package:*@dev'],
+      [NULL, 'vendor/package:@dev'],
+      [NULL, 'vendor/package:master@dev'],
+      [NULL, 'vendor/package:master@beta'],
+      [NULL, 'php'],
+      [NULL, 'php:8'],
+      [NULL, 'php:8.0'],
+      [NULL, 'php:^8.1'],
+      [NULL, 'php:~8.1'],
+      [NULL, 'php-64bit'],
+      [NULL, 'composer'],
+      [NULL, 'composer-plugin-api'],
+      [NULL, 'composer-plugin-api:1'],
+      [NULL, 'ext-json'],
+      [NULL, 'ext-json:1'],
+      [NULL, 'ext-pdo_mysql'],
+      [NULL, 'ext-pdo_mysql:1'],
+      [NULL, 'lib-curl'],
+      [NULL, 'lib-curl:1'],
+      [NULL, 'lib-curl-zlib'],
+      [NULL, 'lib-curl-zlib:1'],
+
+      // Invalid requirements.
+      [\InvalidArgumentException::class, ''],
+      [\InvalidArgumentException::class, ' '],
+      [\InvalidArgumentException::class, '/'],
+      [\InvalidArgumentException::class, 'php8'],
+      [\InvalidArgumentException::class, 'package'],
+      [\InvalidArgumentException::class, 'vendor\package'],
+      [\InvalidArgumentException::class, 'vendor//package'],
+      [\InvalidArgumentException::class, 'vendor/package1 vendor/package2'],
+      [\InvalidArgumentException::class, 'vendor/package/extra'],
+      [\UnexpectedValueException::class, 'vendor/package:a'],
+      [\UnexpectedValueException::class, 'vendor/package:'],
+      [\UnexpectedValueException::class, 'vendor/package::'],
+      [\UnexpectedValueException::class, 'vendor/package::1'],
+      [\UnexpectedValueException::class, 'vendor/package:1:2'],
+      [\UnexpectedValueException::class, 'vendor/package:develop@dev@dev'],
+      [\UnexpectedValueException::class, 'vendor/package:develop@'],
+      [\InvalidArgumentException::class, 'vEnDor/pAcKaGe'],
+      [\InvalidArgumentException::class, '_vendor/package'],
+      [\InvalidArgumentException::class, '_vendor/_package'],
+      [\InvalidArgumentException::class, 'vendor_/package'],
+      [\InvalidArgumentException::class, '_vendor/package_'],
+      [\InvalidArgumentException::class, 'vendor/package-'],
+      [\InvalidArgumentException::class, 'php-'],
+      [\InvalidArgumentException::class, 'ext'],
+      [\InvalidArgumentException::class, 'lib'],
+    ];
+  }
+
+}