fix: #3592887 AttributeRouteDiscovery does not cleanly handle invalid classes (5f815c3b) · Commits · project / drupal

core/lib/Drupal/Core/Routing/AttributeRouteDiscovery.php

+15 −5

Original line number	Diff line number	Diff line
		@@ -46,6 +46,11 @@ protected function collectRoutes(): iterable {
		);
		foreach ($iterator as $fileinfo) {
		if ($fileinfo->getExtension() == 'php') {
		// Skip files that do not contain a Route attribute.
		$contents = file_get_contents($fileinfo->getPathname());
		if (!str_contains($contents, '#[Route') && !str_contains($contents, 'Routing\\Attribute\\Route')) {
		continue;
		}
		$subPath = $iterator->getSubIterator()->getSubPath();
		$subPath = $subPath ? str_replace(DIRECTORY_SEPARATOR, '\\', $subPath) . '\\' : '';
		$class = $namespace . '\\' . $subPath . $fileinfo->getBasename('.php');
		@@ -68,11 +73,16 @@ protected function collectRoutes(): iterable {
		private function createRouteCollection(string $className): RouteCollection {
		$collection = new RouteCollection();

		try {
		if (!class_exists($className)) {
		// In Symfony code this triggers an exception. It is removed here because
		// Drupal already has traits, interfaces and other things in this folder.
		// Alternatively, we could remove this if clause and then check what the
		// resulting reflection object is.
		// In Symfony code this triggers an exception. It is removed here
		// because Drupal already has traits, interfaces and other things in
		// this folder. Alternatively, we could remove this if clause and then
		// check what the resulting reflection object is.
		return $collection;
		}
		}
		catch (\Error) {
		return $collection;
		}
		$class = new \ReflectionClass($className);

0 → 100644

+20 −0

Original line number	Diff line number	Diff line
		<?php

		declare(strict_types=1);

		namespace Drupal\router_test\Controller;

		/**
		* A structurally invalid controller that cannot be autoloaded.
		*/
		class TestInvalidController {

		/**
		* An abstract method, which is not allowed on a non-abstract class.
		*
		* @return array
		* A render array.
		*/
		abstract public function build(): array;

		}

0 → 100644

+30 −0

Original line number	Diff line number	Diff line
		<?php

		declare(strict_types=1);

		namespace Drupal\router_test\Controller;

		use Drupal\a_module_that_does_not_exist\SomeController;
		use Symfony\Component\Routing\Attribute\Route;

		/**
		* A controller that declares a route but extends a class from a missing module.
		*
		* A missing parent class (rather than a missing trait) is used deliberately: it
		* is reported as a catchable error on all supported PHP versions, whereas a
		* missing trait only became catchable in PHP 8.5.
		*/
		class TestMissingDependency extends SomeController {

		/**
		* Builds something.
		*
		* @return array
		* A render array.
		*/
		#[Route('/test_missing_dependency', name: 'router_test.missing_dependency')]
		public function build(): array {
		return [];
		}

		}

+2 −0

Original line number	Diff line number	Diff line
		@@ -51,6 +51,8 @@ parameters:
		- modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/fruit/ExtendingNonInstalledClass.php
		- modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/UsingNonInstalledTraitClass.php
		- modules/system/tests/modules/plugin_test/src/Plugin/plugin_test/custom_annotation/ExtendingNonInstalledClass.php
		- modules/system/tests/modules/router_test_directory/src/Controller/TestInvalidController.php
		- modules/system/tests/modules/router_test_directory/src/Controller/TestMissingDependency.php

		ignoreErrors:
		# new static() is a best practice in Drupal, so we cannot fix that.

+9 −0

Original line number	Diff line number	Diff line
		@@ -105,6 +105,15 @@ public function testAllRouteProperties(): void {
		$this->assertSame($route, $this->routeCollection->get(TestAttributes::class . '::allProperties'));
		}

		/**
		* Tests that invalid controller classes do not break route discovery.
		*/
		public function testInvalidClasses(): void {
		$this->assertNotNull($this->routeCollection->get('router_test.method_attribute'));
		$this->assertNotNull($this->routeCollection->get('router_test.class_invoke'));
		$this->assertNull($this->routeCollection->get('router_test.missing_dependency'));
		}

		/**
		* Tests that a method inherits class-level globals.
		*/