[FrameworkBundle] Enable controller service with #[Route] attribute
#60081
Conversation
There was a problem hiding this comment.
Just saying: It might be interesting to check the perf overhead of scanning all methods instead of just all classes.
This attribute is not the only one that triggers this scan so my comment is not a blocker, but still something I have in mind when reviewing :)
|
@nicolas-grekas is there caching of attribute usage? Then it will only affect cache warm up. |
|
Yes, I'm thinking about optimizing the compile time indeed. At runtime, it doesn't matter. |
|
Thank you @GromNaN. |
|
I just realized that this PR is missing some integration with https://github.com/symfony/symfony/pull/52471/files |
|
I wasn't sure for security reasons. Requiring the |
|
Which security problem could this lead to? If a method is registered as a controller, it's safe to be used as a fragment to me. |
|
Here is the patch that should do it. I'm not submitting it as a PR because it might be not needed in the end: the error message is self explanatory and explains one should use diff --git a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php
index 12bcd4c2dd..b8e5623476 100644
--- a/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php
+++ b/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php
@@ -15,6 +15,7 @@ use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Attribute\AsController;
+use Symfony\Component\Routing\Attribute\Route;
/**
* This implementation uses the '_controller' request attribute to determine
@@ -26,7 +27,10 @@ use Symfony\Component\HttpKernel\Attribute\AsController;
class ControllerResolver implements ControllerResolverInterface
{
private array $allowedControllerTypes = [];
- private array $allowedControllerAttributes = [AsController::class => AsController::class];
+ private array $allowedControllerAttributes = [
+ AsController::class => AsController::class,
+ Route::class => Route::class,
+ ];
public function __construct(
private ?LoggerInterface $logger = null,
@@ -228,12 +232,14 @@ class ControllerResolver implements ControllerResolverInterface
}
$r = null;
+ $method = null;
if (\is_array($controller)) {
- [$class, $name] = $controller;
- $name = (\is_string($class) ? $class : $class::class).'::'.$name;
+ [$class, $method] = $controller;
+ $name = (\is_string($class) ? $class : $class::class).'::'.$method;
} elseif (\is_object($controller) && !$controller instanceof \Closure) {
$class = $controller;
+ $method = '__invoke';
$name = $class::class.'::__invoke';
} else {
$r = new \ReflectionFunction($controller);
@@ -255,14 +261,16 @@ class ControllerResolver implements ControllerResolverInterface
}
}
- $r ??= new \ReflectionClass($class);
+ do {
+ $r ??= new \ReflectionClass($class);
- foreach ($r->getAttributes() as $attribute) {
- if (isset($this->allowedControllerAttributes[$attribute->getName()])) {
- return $controller;
+ foreach ($r->getAttributes() as $attribute) {
+ if (isset($this->allowedControllerAttributes[$attribute->getName()])) {
+ return $controller;
+ }
}
- }
-
+ } while ($method && $r instanceof \ReflectionClass && method_exists($class, $method) && $r = new \ReflectionMethod($class, $method));
+
if (str_contains($name, '@anonymous')) {
$name = preg_replace_callback('/[a-zA-Z_\x7f-\xff][\\\\a-zA-Z0-9_\x7f-\xff]*+@anonymous\x00.*?\.php(?:0x?|:[0-9]++\$)?[0-9a-fA-F]++/', fn ($m) => class_exists($m[0], false) ? (get_parent_class($m[0]) ?: key(class_implements($m[0])) ?: 'class').'@anonymous' : $m[0], $name);
} |
|
NVM my latest comments, such controllers will be added to the set of allowed controllers thanks to the tag and to #53985 |
The
#[AsController]attribute has 2 purposes:controller.service_argumentthat configures the service and service argument injection (RegisterControllerArgumentLocatorsPass)In this PR, I propose to add the tag
argument_resolver.serviceon services when the class has the#[Route]attribute. Removing the need for#[AsController]on classes that use the#[Route]attribute.I assume that if there is a route, it is a controller.
Diff (from the docs):