[ServiceManager] Implemented circular alias reference detection #5100

Closed
wants to merge 5 commits into
from

3 participants

@tux-rampage

ServiceManagers that allow overwriting the service definition may
contain circular references which will cause an infinite loop.

I extracted the alias resolving code to a protected method that will detect
circular references and throw an exception if one is encountered.

tux-rampage added some commits Sep 11, 2013
@tux-rampage tux-rampage Implemented curcular alias reference detection
ServiceManagers that allow overwriting the service definition may
contain circular references which will cause an infinite loop. 
Extracted alias resolving code to a protected method that will detect
circular references.
8252a78
@tux-rampage tux-rampage cs fix 5fe8223
@tux-rampage tux-rampage Removed unneccessary import for exception class 8430572
@Ocramius
Zend Framework member

@tux-rampage wouldn't it be easier to handle this kind of logic in setAlias?

@samsonasik samsonasik commented on an outdated diff Sep 11, 2013
tests/ZendTest/ServiceManager/ServiceManagerTest.php
@@ -810,4 +810,25 @@ public function testUsesMultipleDelegates()
$this->assertInstanceOf('stdClass', array_shift($fooDelegator->instances));
$this->assertSame($fooDelegator, array_shift($barDelegator->instances));
}
+
+ /**
+ * @covers Zend\ServiceMnager\ServiceManager::resolveAlias
@samsonasik
samsonasik added a line comment Sep 11, 2013

typo ServiceMnager instead of ServiceManager, miss 'a' after 'M'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tux-rampage

@Ocramius Might need to think about it. It's not as easy as putting it in the resolve logics. We should also keep in mind that $aliases is a protected and not a private member. So derived classes may manipulate it aside from setAlias().

@tux-rampage

@Ocramius It might be doable in the setAlias() method a s well, but I'm a bit concerned about performance when users start to add a lot of aliases. Any toughts?

@tux-rampage

@Ocramius I added a cycle check to setAlias() as well and modified the unit test accordingly

@weierophinney weierophinney added a commit that referenced this pull request Oct 22, 2013
@weierophinney weierophinney [#5100] CS fixes
- one argument per line (sprintf call)
2a93df1
@weierophinney weierophinney added a commit that closed this pull request Oct 22, 2013
@weierophinney weierophinney Merge branch 'hotfix/5100'
Close #5100
5bbe1c8
@weierophinney weierophinney added a commit that referenced this pull request Oct 22, 2013
@weierophinney weierophinney Merge branch 'hotfix/5100' into develop
Forward port #5100
955d8d3
@weierophinney weierophinney added a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#5100 from tux-rampage/…
…sm-cyclic-alias-ref

[ServiceManager] Implemented circular alias reference detection
dc2205d
@weierophinney weierophinney added a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#5100] CS fixes
- one argument per line (sprintf call)
b655053
@weierophinney weierophinney added a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/5100' e7edbc6
@weierophinney weierophinney added a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/5100' into develop 88622d8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment