[DependencyInjection] Ensure Definition::getTags() contains a list of attributes for each tag
#62683
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carsonbot
changed the title
Member
nicolas-grekas
left a comment
nicolas-grekas
left a comment
There was a problem hiding this comment.
For 6.4? I think this was mostly undefined, but indeed it's better to define the behavior.
src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php
Outdated
Show resolved
Hide resolved
Member
Author
|
We can also fix the bug directly where it was introduced: diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
index 00fdaaf2b47..751f8a535c8 100644
--- a/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php
@@ -81,6 +81,7 @@ trait PriorityTaggedServiceTrait
$phpAttributes = [];
}
+ $attributes = array_values($attributes);
for ($i = 0; $i < \count($attributes); ++$i) {
if (!($attribute = $attributes[$i]) && $phpAttributes) {
array_splice($attributes, $i--, 1, $phpAttributes);
|
… attributes for each tag
OskarStark
changed the title
Definition::getTags() contains a list of attributes for each tag
GromNaN
commented
Dec 7, 2025
| ; | ||
| $container->register(ControllerArguments::class, ControllerArguments::class) | ||
| ->setTags(['controller.service_arguments']) | ||
| ->setTags(['controller.service_arguments' => []]) |
Member
Author
There was a problem hiding this comment.
It seems to me to be a breaking change if people inject random values via
Definition::setTags() and expect to retrieve these values with Definition::getTags().
Member
Author
|
Replaced by #62685 We can improve the type of |
nicolas-grekas
added a commit
that referenced
this pull request
Dec 8, 2025
…n tag attributes are not a list (GromNaN) This PR was merged into the 7.3 branch. Discussion ---------- [DependencyInjection] Fix `PriorityTaggedServiceTrait` when tag attributes are not a list | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Alternative to #62683 Ensure the attributes of the tag are in a list before iterating on them using sequential indexes. > In symfony/monolog-bundle#485, the tag attributes are filtered, which generates gaps in the keys. The PR mentioned above causes MonologBundle to fail. This can be fixed by symfony/monolog-bundle#563. However I believe the issue should also be addressed in Symfony. Commits ------- 5a4a036 [DependencyInjection] Fix PriorityTaggedServiceTrait when tag attributes are not a list
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #62329, we assume that the response of
Definition::getTags()has the typearray<string, list<array>>. However, when usingDefinition::setTags(), we can set anyarrayvalue without validation.In symfony/monolog-bundle#485, the tag attributes are filtered, which generates gaps in the keys. The PR mentioned above causes MonologBundle to fail. This can be fixed by symfony/monolog-bundle#563. However I believe the issue should also be addressed in Symfony.
This PR targets 7.3 because the bug is in this version, but the patch can be applied in 6.4.Switched to 6.4