Skip to content

Double-autowiring can lead to bad outcomes #48019

Closed
@Seldaek

Description

@Seldaek

Symfony version(s) affected

6.1.x (probably 5.4 and 4.4 perhaps as well)

Description

If a class gets autowired twice the effects can be unpredictable and consequences can be pretty bad.

How to reproduce

The case which triggered this find was that I made a monolog processor using the new AsMonologProcessor (symfony/monolog-bundle#428) attribute, which looks like this:

use Monolog\Attribute\AsMonologProcessor;
use Monolog\Level;
use Monolog\LogRecord;

#[AsMonologProcessor(channel: 'my_channel')]
class ErrorSuppressingProcessor implements \Monolog\Processor\ProcessorInterface
{
    public function __invoke(LogRecord $record): LogRecord
    {
        return $record->with(level: Level::Debug);
    }
}

Now as you can see I specified this should only apply to the my_channel channel in the AsMonologProcessor attribute. Given what the processor does, having it applied to all channels would be pretty bad as it turns all records into debug records.

This in fact works well, the MonologExtension registers it correctly in https://github.com/symfony/monolog-bundle/blob/a41bbcdc1105603b6d73a7d9a43a3788f8e0fb7d/DependencyInjection/MonologExtension.php#L155-L166

The problem is that because I also implemented ProcessorInterface, and the MonologExtension defines this interface as autowired (https://github.com/symfony/monolog-bundle/blob/a41bbcdc1105603b6d73a7d9a43a3788f8e0fb7d/DependencyInjection/MonologExtension.php#L134-L135), the DIC also adds a plain monolog.processor tag to my processor class.

Then in AddProcessorPass you end up with the ErrorSuppressingProcessor service with two tags, one containing the correct channel mapping and the other one being a blank monolog.processor tag without options, which means the processor suddenly gets registered to all Loggers/channels. Not at all what I wanted not expected :)

Possible Solution

I am not entirely sure what can be done here, but I believe a solution should be in symfony/di and not in the bundle itself as that was just a symptom of the problem.

My instinct would be to say that whatever code handles registerForAutoconfiguration should probably not add a tag if the tag is already present. Not sure what the consequences of doing that would be though.

Additional Context

No response

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