Merge branch 'hotfix/89' into develop

Forward port #89

Author: weierophinney

CHANGELOG.md

@@ -6,8 +6,7 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Added
 
-- [#64](https://github.com/zendframework/zend-servicemanager/pull/64) performance optimizations
-  when dealing with alias resolution during service manager instantiation
+- Nothing.
 
 ### Deprecated
 
@@ -25,7 +24,10 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Added
 
-- Nothing.
+- [#89](https://github.com/zendframework/zend-servicemanager/pull/89) adds
+  cyclic alias detection to the `ServiceManager`; it now raises a
+  `Zend\ServiceManager\Exception\CyclicAliasException` when one is detected,
+  detailing the cycle detected.
 
 ### Deprecated
 

src/Exception/CyclicAliasException.php

@@ -0,0 +1,139 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link      http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace Zend\ServiceManager\Exception;
+
+class CyclicAliasException extends InvalidArgumentException
+{
+    /**
+     * @param string[] $aliases map of referenced services, indexed by alias name (string)
+     *
+     * @return self
+     */
+    public static function fromAliasesMap(array $aliases)
+    {
+        $detectedCycles = array_filter(array_map(
+            function ($alias) use ($aliases) {
+                return self::getCycleFor($aliases, $alias);
+            },
+            array_keys($aliases)
+        ));
+
+        if (! $detectedCycles) {
+            return new self(sprintf(
+                "A cycle was detected within the following aliases map:\n\n%s",
+                self::printReferencesMap($aliases)
+            ));
+        }
+
+        return new self(sprintf(
+            "Cycles were detected within the provided aliases:\n\n%s\n\n"
+            . "The cycle was detected in the following alias map:\n\n%s",
+            self::printCycles(self::deDuplicateDetectedCycles($detectedCycles)),
+            self::printReferencesMap($aliases)
+        ));
+    }
+
+    /**
+     * Retrieves the cycle detected for the given $alias, or `null` if no cycle was detected
+     *
+     * @param string[] $aliases
+     * @param string   $alias
+     *
+     * @return array|null
+     */
+    private static function getCycleFor(array $aliases, $alias)
+    {
+        $cycleCandidate = [];
+        $targetName     = $alias;
+
+        while (isset($aliases[$targetName])) {
+            if (isset($cycleCandidate[$targetName])) {
+                return $cycleCandidate;
+            }
+
+            $cycleCandidate[$targetName] = true;
+
+            $targetName = $aliases[$targetName];
+        }
+
+        return null;
+    }
+
+    /**
+     * @param string[] $aliases
+     *
+     * @return string
+     */
+    private static function printReferencesMap(array $aliases)
+    {
+        $map = [];
+
+        foreach ($aliases as $alias => $reference) {
+            $map[] = '"' . $alias . '" => "' . $reference . '"';
+        }
+
+        return "[\n" . implode("\n", $map) . "\n]";
+    }
+
+    /**
+     * @param string[][] $detectedCycles
+     *
+     * @return string
+     */
+    private static function printCycles(array $detectedCycles)
+    {
+        return "[\n" . implode("\n", array_map([__CLASS__, 'printCycle'], $detectedCycles)) . "\n]";
+    }
+
+    /**
+     * @param string[] $detectedCycle
+     *
+     * @return string
+     */
+    private static function printCycle(array $detectedCycle)
+    {
+        $fullCycle   = array_keys($detectedCycle);
+        $fullCycle[] = reset($fullCycle);
+
+        return implode(
+            ' => ',
+            array_map(
+                function ($cycle) {
+                    return '"' . $cycle . '"';
+                },
+                $fullCycle
+            )
+        );
+    }
+
+    /**
+     * @param bool[][] $detectedCycles
+     *
+     * @return bool[][] de-duplicated
+     */
+    private static function deDuplicateDetectedCycles(array $detectedCycles)
+    {
+        $detectedCyclesByHash = [];
+
+        foreach ($detectedCycles as $detectedCycle) {
+            $cycleAliases = array_keys($detectedCycle);
+
+            sort($cycleAliases);
+
+            $hash = serialize(array_values($cycleAliases));
+
+            $detectedCyclesByHash[$hash] = isset($detectedCyclesByHash[$hash])
+                ? $detectedCyclesByHash[$hash]
+                : $detectedCycle;
+        }
+
+        return array_values($detectedCyclesByHash);
+    }
+}

src/ServiceManager.php

@@ -16,11 +16,12 @@
 use ProxyManager\Factory\LazyLoadingValueHolderFactory;
 use ProxyManager\GeneratorStrategy\EvaluatingGeneratorStrategy;
 use Zend\ServiceManager\Exception\ContainerModificationsNotAllowedException;
+use Zend\ServiceManager\Exception\CyclicAliasException;
 use Zend\ServiceManager\Exception\InvalidArgumentException;
 use Zend\ServiceManager\Exception\ServiceNotCreatedException;
+use Zend\ServiceManager\Exception\ServiceNotFoundException;
 use Zend\ServiceManager\Factory\AbstractFactoryInterface;
 use Zend\ServiceManager\Factory\DelegatorFactoryInterface;
-use Zend\ServiceManager\Exception\ServiceNotFoundException;
 use Zend\ServiceManager\Initializer\InitializerInterface;
 
 /**
@@ -569,10 +570,16 @@ private function resolveInitializers(array $initializers)
     private function resolveAliases(array $aliases)
     {
         foreach ($aliases as $alias => $service) {
-            $name = $alias;
+            $visited = [];
+            $name    = $alias;
 
             while (isset($this->aliases[$name])) {
-                $name = $this->aliases[$name];
+                if (isset($visited[$name])) {
+                    throw CyclicAliasException::fromAliasesMap($aliases);
+                }
+
+                $visited[$name] = true;
+                $name           = $this->aliases[$name];
             }
 
             $this->resolvedAliases[$alias] = $name;

test/CommonServiceLocatorBehaviorsTrait.php

@@ -11,16 +11,16 @@
 
 use DateTime;
 use Interop\Container\Exception\ContainerException;
-use PHPUnit_Framework_TestCase as TestCase;
 use ReflectionProperty;
 use stdClass;
 use Zend\ServiceManager\Exception\ContainerModificationsNotAllowedException;
+use Zend\ServiceManager\Exception\CyclicAliasException;
 use Zend\ServiceManager\Exception\InvalidArgumentException;
 use Zend\ServiceManager\Exception\ServiceNotCreatedException;
 use Zend\ServiceManager\Factory\FactoryInterface;
+use Zend\ServiceManager\Factory\InvokableFactory;
 use Zend\ServiceManager\Initializer\InitializerInterface;
 use Zend\ServiceManager\ServiceLocatorInterface;
-use Zend\ServiceManager\Factory\InvokableFactory;
 use ZendTest\ServiceManager\TestAsset\FailingAbstractFactory;
 use ZendTest\ServiceManager\TestAsset\FailingFactory;
 use ZendTest\ServiceManager\TestAsset\InvokableObject;
@@ -783,4 +783,19 @@ public function testCanRetrieveParentContainerViaGetServiceLocatorWithDeprecatio
         $this->assertSame($this->creationContext, $container->getServiceLocator());
         restore_error_handler();
     }
+
+    /**
+     * @group zendframework/zend-servicemanager#83
+     */
+    public function testCrashesOnCyclicAliases()
+    {
+        $this->setExpectedException(CyclicAliasException::class);
+
+        $this->createContainer([
+            'aliases' => [
+                'a' => 'b',
+                'b' => 'a',
+            ],
+        ]);
+    }
 }