Skip to content

Commit

Permalink
bug #28480 [DI] Detect circular references with ChildDefinition paren…
Browse files Browse the repository at this point in the history
…t (Seb33300)

This PR was submitted for the master branch but it was squashed and merged into the 3.4 branch instead (closes #28480).

Discussion
----------

[DI] Detect circular references with ChildDefinition parent

| Q             | A
| ------------- | ---
| Branch?       | 3.4 (be careful when merging)
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28312
| License       | MIT
| Doc PR        |

I will provide a test case if the fix looks good for you :)

Commits
-------

2a59c8e [DI] Detect circular references with ChildDefinition parent
  • Loading branch information
nicolas-grekas committed Sep 18, 2018
2 parents 7a19350 + 2a59c8e commit c4c2981
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\ExceptionInterface;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;

/**
* This replaces all ChildDefinition instances with their equivalent fully
Expand All @@ -25,6 +26,8 @@
*/
class ResolveChildDefinitionsPass extends AbstractRecursivePass
{
private $currentPath;

protected function processValue($value, $isRoot = false)
{
if (!$value instanceof Definition) {
Expand All @@ -36,6 +39,7 @@ protected function processValue($value, $isRoot = false)
$value = $this->container->getDefinition($this->currentId);
}
if ($value instanceof ChildDefinition) {
$this->currentPath = array();
$value = $this->resolveDefinition($value);
if ($isRoot) {
$this->container->setDefinition($this->currentId, $value);
Expand All @@ -56,6 +60,8 @@ private function resolveDefinition(ChildDefinition $definition)
{
try {
return $this->doResolveDefinition($definition);
} catch (ServiceCircularReferenceException $e) {
throw $e;
} catch (ExceptionInterface $e) {
$r = new \ReflectionProperty($e, 'message');
$r->setAccessible(true);
Expand All @@ -71,6 +77,13 @@ private function doResolveDefinition(ChildDefinition $definition)
throw new RuntimeException(sprintf('Parent definition "%s" does not exist.', $parent));
}

$searchKey = array_search($parent, $this->currentPath);
$this->currentPath[] = $parent;

if (false !== $searchKey) {
throw new ServiceCircularReferenceException($parent, \array_slice($this->currentPath, $searchKey));
}

$parentDef = $this->container->findDefinition($parent);
if ($parentDef instanceof ChildDefinition) {
$id = $this->currentId;
Expand Down
Expand Up @@ -431,4 +431,21 @@ protected function process(ContainerBuilder $container)
$pass = new ResolveChildDefinitionsPass();
$pass->process($container);
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
* @expectedExceptionMessageRegExp /^Circular reference detected for service "c", path: "c -> b -> a -> c"./
*/
public function testProcessDetectsChildDefinitionIndirectCircularReference()
{
$container = new ContainerBuilder();

$container->register('a');

$container->setDefinition('b', new ChildDefinition('a'));
$container->setDefinition('c', new ChildDefinition('b'));
$container->setDefinition('a', new ChildDefinition('c'));

$this->process($container);
}
}

0 comments on commit c4c2981

Please sign in to comment.