Verified Commit b0d5f3a7 authored by Dave Long's avatar Dave Long
Browse files

fix: #3592887 AttributeRouteDiscovery does not cleanly handle invalid classes

By: cmlara
By: longwave
By: godotislate
By: catch
parent f4150321
Loading
Loading
Loading
Loading
Loading
+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);
+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;

}
+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
@@ -47,6 +47,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
    # Needs to be skipped until PHPStan recognises the existence of Pdo\Sqlite
    - modules/sqlite/src/Driver/Database/sqlite/SqliteConnection.php

+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.
   */