Skip to content

Bug in ArgumentMetadataFactory messes up controller arguments with attributes #41478

Closed
@sgehrig

Description

@sgehrig

Symfony version(s) affected: 5.3.0

Description
When using attributes on controller arguments (such as \Symfony\Component\Security\Http\Attribute\CurrentUser for example) the \Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory messes up the arguments' metadata and applies the attribute to all arguments following the one argument actually having the attribute applied.

How to reproduce

namespace App\Controller;

use App\Security\ MyUserInterface;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Http\Attribute\CurrentUser;
use Symfony\Component\Security\Core\User\UserInterface

class TestController {
    public function test(
        #[CurrentUser] UserInterface|MyUserInterface $user,
        MessageBusInterface $commandBus
    ): Response {
        return new Response();
    }
}

In this example the $commandBus argument gets attributed with CurrentUser as well. This leads to errors like:

TypeError : App\Controller\TestController::test(): Argument #2 ($commandBus) must be of type Symfony\Component\Messenger\MessageBusInterface, App\Security\MyUser given, called in /web/app/vendor/symfony/http-kernel/HttpKernel.php on line 157

The problem comes from commit #d771e449ec92958d7cf7c88b3c8092d3e46c611e. The $attributes array in line 40 (

$attributes[] = $reflectionAttribute->newInstance();
) is never reset and therefore attributes discovered on controller arguments are simply accumulated for following arguments.

Possible Solution

The easiest solution would be to initialise the $attributes per parameter in the outer foreach loop. Such as:

// vendor/symfony/http-kernel/ControllerMetadata/ArgumentMetadataFactory.php:24
// \Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory::createArgumentMetadata
// ...
    foreach ($reflection->getParameters() as $param) {
            $attributes = []; // new
            if (\PHP_VERSION_ID >= 80000) {
                foreach ($param->getAttributes() as $reflectionAttribute) {
                    if (class_exists($reflectionAttribute->getName())) {
                        $attributes[] = $reflectionAttribute->newInstance();
                    }
                }
            }

            $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param, $reflection), $param->isVariadic(), $param->isDefaultValueAvailable(), $param->isDefaultValueAvailable() ? $param->getDefaultValue() : null, $param->allowsNull(), $attributes ?? []); // null-coalesce technically not needed any more.
        }
// ...

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions