Merge branch 'feature/230' into develop

Close #230

Author: weierophinney

CHANGELOG.md

@@ -29,7 +29,8 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Fixed
 
-- Nothing.
+- [#230](https://github.com/zendframework/zend-servicemanager/pull/230) fixes a
+  problem in detecting cyclic aliases, ensuring they are detected correctly.
 
 ## 3.3.1 - 2017-11-27
 

src/ServiceManager.php

@@ -868,6 +868,12 @@ private function mapAliasToTarget($alias, $target)
      *
      * This function maps $this->aliases in place.
      *
+     * This algorithm is an adaptated version of Tarjans Strongly
+     * Connected Components. Instead of returning the strongly
+     * connected components (i.e. cycles in our case), we throw.
+     * If nodes are not strongly connected (i.e. resolvable in
+     * our case), they get resolved.
+     *
      * This algorithm is fast for mass updates through configure().
      * It is not appropriate if just a single alias is added.
      *
@@ -881,16 +887,35 @@ private function mapAliasesToTargets()
             if (isset($tagged[$alias])) {
                 continue;
             }
+
             $tCursor = $this->aliases[$alias];
             $aCursor = $alias;
+            if ($aCursor === $tCursor) {
+                throw CyclicAliasException::fromCyclicAlias($alias, $this->aliases);
+            }
+            if (! isset($this->aliases[$tCursor])) {
+                continue;
+            }
+
+            $stack = [];
+
             while (isset($this->aliases[$tCursor])) {
-                $tagged[$aCursor] = true;
-                $this->aliases[$aCursor] = $this->aliases[$tCursor];
+                $stack[] = $aCursor;
+                if ($aCursor === $this->aliases[$tCursor]) {
+                    throw CyclicAliasException::fromCyclicAlias($alias, $this->aliases);
+                }
                 $aCursor = $tCursor;
                 $tCursor = $this->aliases[$tCursor];
-                if ($aCursor === $tCursor) {
+            }
+
+            $tagged[$aCursor] = true;
+
+            foreach ($stack as $alias) {
+                if ($alias === $tCursor) {
                     throw CyclicAliasException::fromCyclicAlias($alias, $this->aliases);
                 }
+                $this->aliases[$alias] = $tCursor;
+                $tagged[$alias] = true;
             }
         }
     }

test/CommonServiceLocatorBehaviorsTrait.php

@@ -855,4 +855,28 @@ public function testCrashesOnCyclicAliases()
             ],
         ]);
     }
+
+    public function testMinimalCyclicAliasDefinitionShouldThrow()
+    {
+        $sm = $this->createContainer([]);
+
+        $this->expectException(CyclicAliasException::class);
+        $sm->setAlias('alias', 'alias');
+    }
+
+    public function testCoverageDepthFirstTaggingOnRecursiveAliasDefinitions()
+    {
+        $sm = $this->createContainer([
+            'factories' => [
+                stdClass::class => InvokableFactory::class,
+            ],
+            'aliases' => [
+                'alias1' => 'alias2',
+                'alias2' => 'alias3',
+                'alias3' => stdClass::class,
+            ],
+        ]);
+        $this->assertSame($sm->get('alias1'), $sm->get('alias2'));
+        $this->assertSame($sm->get(stdClass::class), $sm->get('alias1'));
+    }
 }