Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DependencyInjection] Prevent a loop in aliases within thefindDefinition method#25364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[DependencyInjection] Prevent a loop in aliases within thefindDefinition method#25364
Uh oh!
There was an error while loading.Please reload this page.
Conversation
c716668 to22f3523Comparesroze commentedDec 6, 2017
AppVeyor's failure is not related... memory on |
| $id = (string)$this->aliasDefinitions[$id]; | ||
| if (isset($seen[$id])) { | ||
| thrownewServiceCircularReferenceException($id,array_keys($seen)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The message should be improved see the CheckCircularReferencePass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What do you mean?
nicolas-grekasDec 7, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry was on mobile :)
This is what I mean:
if (isset($seen[$id])) {$seen =array_slice($seen,array_search($id,$seen));$seen[] =$id;thrownewServiceCircularReferenceException($id,$seen);}$seen[$id] =$id;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I see the idea. Tho, we can't usearray_slice with string keys in an array, we'll have to use 2 of them.
nicolas-grekas commentedDec 6, 2017
Thanks for the PR. |
xabbuh left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM
ro0NL commentedDec 6, 2017
The same loop can happen in |
nicolas-grekas commentedDec 7, 2017
Yes, but is already handled there AFAIK. |
leotiger commentedDec 7, 2017
I've tested the fix and it throws now a circular reference exception as expected. Thx. |
fabpot commentedDec 7, 2017
Thank you@sroze. |
… `findDefinition` method (sroze)This PR was merged into the 3.3 branch.Discussion----------[DependencyInjection] Prevent a loop in aliases within the `findDefinition` method| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#25338| License | MIT| Doc PR | øThis prevents an infinite loop going when aliases reference themselves. This is based on 3.3 as the "normalized ID" changed to allow non-lowercase names. Fixing this in 2.7 would mean a merge conflict that IMO is not worth it.Commits-------22f3523 Prevent a loop in aliases within the `findDefinition` method
This PR was merged into the 3.3 branch.Discussion----------[DI] Fix circular-aliases message| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? || New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -The missing bits in#25364Commits-------f1a7b07 [DI] Fix circular-aliases message
This prevents an infinite loop going when aliases reference themselves. This is based on 3.3 as the "normalized ID" changed to allow non-lowercase names. Fixing this in 2.7 would mean a merge conflict that IMO is not worth it.