From 3efd96d822c35bb53ccf4de9a7ae3bc07883256e Mon Sep 17 00:00:00 2001
From: rahul_ <rahul_gupta@3278723.no-reply.drupal.org>
Date: Thu, 29 Sep 2022 20:03:18 +0000
Subject: [PATCH] Issue #3310901 by rahul_, phenaproxima, TravisCarden:
Stage::require() should validate the incoming package names
---
package_manager/src/Stage.php | 33 ++++++++++
...OverwriteExistingPackagesValidatorTest.php | 2 +-
.../tests/src/Kernel/StageTest.php | 60 ++++++++++++++++++-
3 files changed, 91 insertions(+), 4 deletions(-)
diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php
index e34a400252..ef386f951c 100644
--- a/package_manager/src/Stage.php
+++ b/package_manager/src/Stage.php
@@ -356,10 +356,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);
$command = array_merge(['require', '--no-update'], $runtime);
$this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout);
}
if ($dev) {
+ $this->validatePackageNames($dev);
$command = array_merge(['require', '--dev', '--no-update'], $dev);
$this->stager->stage($command, $active_dir, $stage_dir, NULL, $timeout);
}
@@ -705,4 +707,35 @@ class Stage implements LoggerAwareInterface {
return $this->t('Staged changes failed to apply, and the site is in an indeterminate state. It is strongly recommended to restore the code and database from a backup.');
}
+ /**
+ * 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.
+ *
+ * @param string[] $package_versions
+ * A set of package names (with or without version constraints), as passed
+ * to ::require().
+ *
+ * @throws \InvalidArgumentException
+ * Thrown if any of the given package names are invalid.
+ *
+ * @see https://getcomposer.org/doc/articles/composer-platform-dependencies.md
+ */
+ protected function validatePackageNames(array $package_versions): void {
+ foreach ($package_versions as $package_name) {
+ $package_name = trim($package_name);
+
+ // Don't mistake the legitimate `php` and `composer` platform requirements
+ // for Drupal projects.
+ if ($package_name === 'php' || $package_name === 'composer') {
+ continue;
+ }
+ elseif (preg_match('/^[a-z0-9_]+$/i', $package_name)) {
+ throw new \InvalidArgumentException("Invalid package name '$package_name'.");
+ }
+ }
+ }
+
}
diff --git a/package_manager/tests/src/Kernel/OverwriteExistingPackagesValidatorTest.php b/package_manager/tests/src/Kernel/OverwriteExistingPackagesValidatorTest.php
index 7eae32bb71..b8ba5daa15 100644
--- a/package_manager/tests/src/Kernel/OverwriteExistingPackagesValidatorTest.php
+++ b/package_manager/tests/src/Kernel/OverwriteExistingPackagesValidatorTest.php
@@ -51,7 +51,7 @@ class OverwriteExistingPackagesValidatorTest extends PackageManagerKernelTestBas
$stage = $this->createStage();
$stage->create();
- $stage->require(['drupal' => '9.8.1']);
+ $stage->require(['drupal/core:9.8.1']);
$expected_results = [
ValidationResult::createError([
diff --git a/package_manager/tests/src/Kernel/StageTest.php b/package_manager/tests/src/Kernel/StageTest.php
index 2dd2dd82fe..7edc016dc9 100644
--- a/package_manager/tests/src/Kernel/StageTest.php
+++ b/package_manager/tests/src/Kernel/StageTest.php
@@ -300,7 +300,7 @@ class StageTest extends PackageManagerKernelTestBase {
public function testCommitException(string $thrown_class, string $expected_class): void {
$stage = $this->createStage();
$stage->create();
- $stage->require(['drupal/core' => '9.8.1']);
+ $stage->require(['drupal/core:9.8.1']);
$thrown_message = 'A very bad thing happened';
// PreconditionException requires a preconditions object.
@@ -421,7 +421,7 @@ class StageTest extends PackageManagerKernelTestBase {
// Run apply and post-apply in the same request (i.e., the same request
// time), and ensure the warning is logged.
$stage->create();
- $stage->require(['drupal/core' => '9.8.1']);
+ $stage->require(['drupal/core:9.8.1']);
$stage->apply();
$stage->postApply();
$this->assertTrue($logger->hasRecord($warning_message, LogLevel::WARNING));
@@ -429,7 +429,7 @@ class StageTest extends PackageManagerKernelTestBase {
$logger->reset();
$stage->create();
- $stage->require(['drupal/core' => '9.8.2']);
+ $stage->require(['drupal/core:9.8.2']);
$stage->apply();
// Simulate post-apply taking place in another request by simulating a
// request time 30 seconds after apply started.
@@ -438,6 +438,60 @@ 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 gien 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');
+ $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 [
+ // White space is trimmed out of package names during validation.
+ 'empty string' => ['', FALSE],
+ 'white space' => [' ', FALSE],
+ // The `composer` and `php` requirements are special, since they could
+ // otherwise be mistaken for Drupal project names.
+ 'Composer runtime, unconstrained' => ['composer', FALSE],
+ 'Composer runtime, constrained' => ['composer:^2.4', FALSE],
+ 'Composer runtime API, unconstrained' => ['composer-runtime-api', FALSE],
+ 'Composer runtime API, constrained' => ['composer-runtime-api:~2.2.0', FALSE],
+ 'PHP runtime, unconstrained' => ['php', FALSE],
+ 'PHP runtime, constrained' => ['php:>=7.4', FALSE],
+ 'PHP runtime variant, unconstrained' => ['php-zts', FALSE],
+ 'PHP runtime variant, constrained' => ['php-zts:8.1', FALSE],
+ 'PHP extension, unconstrained' => ['ext-json', FALSE],
+ 'PHP extension, constrained' => ['ext-json:7.4.1', FALSE],
+ 'PHP library, unconstrained' => ['lib-curl', FALSE],
+ 'PHP library, constrained' => ['lib-curl:^7', FALSE],
+ 'Drupal package, unconstrained' => ['drupal/semver_test', FALSE],
+ 'Drupal package, constrained' => ['drupal/semver_test:^1.10', FALSE],
+ 'Drupal project' => ['semver_test', TRUE],
+ ];
+ }
+
}
/**
--
GitLab