From 61437630eadfe505f41c357f421ad05744d3f787 Mon Sep 17 00:00:00 2001
From: Matthew Weier O'Phinney <matthew@zend.com>
Date: Mon, 7 Jan 2019 15:04:51 -0600
Subject: [PATCH] fix: Prevent circular dependency lookup in
 RpcControllerFactory

As reported in #18, a circular dependency lookup can occur when
specifying a static method of a class as a `callable` for an RPC service
that matches the class name for the RPC service. As an example:

```php
return [
    'zf-rpc' => [
        SomeController::class => [
            'callable' => SomeController::class . '::bar',
        ],
    ],
];
```

In the above case, the `RpcControllerFactory` would be invoked for
`SomeController::class`, find the `callable` entry of
`{NS}\SomeController::bar`, split this into the class and method, and
then attempt to fetch `SomeController::class` again, ad infinitum.

This patch adds a private property, `$lastRequestedControllerService`,
which is set to the service being pulled whenever we attempt to marshal
a service from the controller manager. If a subsequent invocation finds
that the service requested matches that property value, then we do not
attempt to pull from the controller manager again, but move on to the
parent container (if we have one), or directly instantiating the class.
(The `$lastRequestedControllerService` is reset otherwise.)

Fixes #18
---
 src/Factory/RpcControllerFactory.php      | 16 ++++++++-
 test/Factory/RpcControllerFactoryTest.php | 40 +++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/src/Factory/RpcControllerFactory.php b/src/Factory/RpcControllerFactory.php
index 9f21339..a8aa4eb 100644
--- a/src/Factory/RpcControllerFactory.php
+++ b/src/Factory/RpcControllerFactory.php
@@ -14,6 +14,15 @@
 
 class RpcControllerFactory implements AbstractFactoryInterface
 {
+    /**
+     * Marker used to ensure we do not end up in a circular dependency lookup
+     * loop.
+     *
+     * @see https://github.com/zfcampus/zf-rpc/issues/18
+     * @var null|string
+     */
+    private $lastRequestedControllerService;
+
     /**
      * Determine if we can create a service with name
      *
@@ -118,10 +127,15 @@ private function marshalCallable($string, ContainerInterface $container)
         $callable = false;
         list($class, $method) = explode('::', $string, 2);
 
-        if ($container->has('ControllerManager')) {
+        if ($container->has('ControllerManager')
+            && $this->lastRequestedControllerService !== $class
+        ) {
+            $this->lastRequestedControllerService = $class;
             $callable = $this->marshalCallableFromContainer($class, $method, $container->get('ControllerManager'));
         }
 
+        $this->lastRequestedControllerService = null;
+
         if (! $callable) {
             $callable = $this->marshalCallableFromContainer($class, $method, $container);
         }
diff --git a/test/Factory/RpcControllerFactoryTest.php b/test/Factory/RpcControllerFactoryTest.php
index 4492b2a..d8ec7d4 100644
--- a/test/Factory/RpcControllerFactoryTest.php
+++ b/test/Factory/RpcControllerFactoryTest.php
@@ -10,7 +10,9 @@
 use PHPUnit\Framework\TestCase;
 use Prophecy\Prophecy\ProphecyInterface;
 use ReflectionProperty;
+use Zend\EventManager\EventManagerInterface;
 use Zend\Mvc\Controller\ControllerManager;
+use Zend\Mvc\Controller\PluginManager;
 use Zend\ServiceManager\Exception\ServiceNotCreatedException;
 use Zend\ServiceManager\ServiceLocatorInterface;
 use ZF\Rpc\Factory\RpcControllerFactory;
@@ -287,4 +289,42 @@ public function testServiceCreationReturnsRpcControllerWrappingCallableForValidC
         $this->assertInstanceOf(RpcController::class, $controller);
         $this->assertAttributeSame($callable, 'wrappedCallable', $controller);
     }
+
+    /**
+     * @group 7
+     * @see https://github.com/zfcampus/zf-rpc/issues/18
+     */
+    public function testFactoryDoesNotEnterACircularDependencyLookupCondition()
+    {
+        $config = [
+            'controllers' => [
+                'abstract_factories' => [
+                    RpcControllerFactory::class,
+                ],
+            ],
+            'zf-rpc' => [
+                TestAsset\Foo::class => [
+                    'callable' => TestAsset\Foo::class . '::bar',
+                ],
+            ],
+        ];
+
+        $this->services->has('config')->willReturn(true);
+        $this->services->get('config')->willReturn($config);
+
+        $this->services->has(TestAsset\Foo::class)->willReturn(false);
+
+        $this->services->get('EventManager')->willReturn($this->prophesize(EventManagerInterface::class)->reveal());
+        $this->services->get('ControllerPluginManager')->willReturn($this->prophesize(PluginManager::class)->reveal());
+
+        $controllerManager = new ControllerManager($this->services->reveal(), $config['controllers']);
+
+        $this->services->has('ControllerManager')->willReturn(true);
+        $this->services->get('ControllerManager')->willReturn($controllerManager);
+
+        $this->assertTrue($controllerManager->has(TestAsset\Foo::class));
+
+        $controller = $controllerManager->get(TestAsset\Foo::class);
+        $this->assertInstanceOf(RpcController::class, $controller);
+    }
 }