Skip to content

Commit

Permalink
fix: Prevent circular dependency lookup in RpcControllerFactory
Browse files Browse the repository at this point in the history
As reported in zfcampus#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 zfcampus#18
  • Loading branch information
weierophinney committed Jan 7, 2019
1 parent c03f498 commit 474fcb4
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/Factory/RpcControllerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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);
}
Expand Down
40 changes: 40 additions & 0 deletions test/Factory/RpcControllerFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 testFailsWhenUsingRealControllerManagerWithAbstractFactory()
{
$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);
}
}

0 comments on commit 474fcb4

Please sign in to comment.