Skip to content

Commit

Permalink
[DI] Fix false-positive circular ref leading to wrong exceptions or i…
Browse files Browse the repository at this point in the history 
…nfinite loops at runtime
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Jul 29, 2018
1 parent f569f58 commit e843bb8
Show file tree
Hide file tree
Showing 9 changed files with 474 additions and 146 deletions.
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php
View file
Expand Up @@ -66,7 +66,7 @@ private function getDefinitionId($id, ContainerBuilder $container)
$seen = array();
while ($container->hasAlias($id)) {
if (isset($seen[$id])) {
throw new ServiceCircularReferenceException($id, array_keys($seen));
throw new ServiceCircularReferenceException($id, array_merge(array_keys($seen), array($id)));
}
$seen[$id] = true;
$id = $container->normalizeId($container->getAlias($id));
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/Container.php
View file
Expand Up @@ -294,7 +294,7 @@ public function get($id, $invalidBehavior = /* self::EXCEPTION_ON_INVALID_REFERE
}

if (isset($this->loading[$id])) {
throw new ServiceCircularReferenceException($id, array_keys($this->loading));
throw new ServiceCircularReferenceException($id, array_merge(array_keys($this->loading), array($id)));
}

$this->loading[$id] = true;
Expand Down
94 changes: 45 additions & 49 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
View file
Expand Up @@ -122,7 +122,6 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
private $autoconfiguredInstanceof = array();

private $removedIds = array();
private $alreadyLoading = array();

private static $internalTypes = array(
'int' => true,
Expand Down Expand Up @@ -588,22 +587,32 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
return $this->doGet($id, $invalidBehavior);
}

private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, array &$inlineServices = array())
private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, array &$inlineServices = null, $isConstructorArgument = false)
{
$id = $this->normalizeId($id);

if (isset($inlineServices[$id])) {
return $inlineServices[$id];
}
if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
return parent::get($id, $invalidBehavior);
if (null === $inlineServices) {
$isConstructorArgument = true;
$inlineServices = array();
}
if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
return $service;
try {
if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
return parent::get($id, $invalidBehavior);
}
if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
return $service;
}
} catch (ServiceCircularReferenceException $e) {
if ($isConstructorArgument) {
throw $e;
}
}

if (!isset($this->definitions[$id]) && isset($this->aliasDefinitions[$id])) {
return $this->doGet((string) $this->aliasDefinitions[$id], $invalidBehavior, $inlineServices);
return $this->doGet((string) $this->aliasDefinitions[$id], $invalidBehavior, $inlineServices, $isConstructorArgument);
}

try {
Expand All @@ -616,16 +625,17 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
throw $e;
}

$loading = isset($this->alreadyLoading[$id]) ? 'loading' : 'alreadyLoading';
$this->{$loading}[$id] = true;
if ($isConstructorArgument) {
$this->loading[$id] = true;
}

try {
$service = $this->createService($definition, $inlineServices, $id);
return $this->createService($definition, $inlineServices, $isConstructorArgument, $id);
} finally {
unset($this->{$loading}[$id]);
if ($isConstructorArgument) {
unset($this->loading[$id]);
}
}

return $service;
}

/**
Expand Down Expand Up @@ -1092,7 +1102,7 @@ public function findDefinition($id)
* @throws RuntimeException When the service is a synthetic service
* @throws InvalidArgumentException When configure callable is not callable
*/
private function createService(Definition $definition, array &$inlineServices, $id = null, $tryProxy = true)
private function createService(Definition $definition, array &$inlineServices, $isConstructorArgument = false, $id = null, $tryProxy = true)
{
if (null === $id && isset($inlineServices[$h = spl_object_hash($definition)])) {
return $inlineServices[$h];
Expand All @@ -1110,16 +1120,14 @@ private function createService(Definition $definition, array &$inlineServices, $
@trigger_error($definition->getDeprecationMessage($id), E_USER_DEPRECATED);
}

if ($tryProxy && $definition->isLazy()) {
$proxy = $this
->getProxyInstantiator()
->instantiateProxy(
$this,
$definition,
$id, function () use ($definition, &$inlineServices, $id) {
return $this->createService($definition, $inlineServices, $id, false);
}
);
if ($tryProxy && $definition->isLazy() && !$tryProxy = !($proxy = $this->proxyInstantiator) || $proxy instanceof RealServiceInstantiator) {
$proxy = $proxy->instantiateProxy(
$this,
$definition,
$id, function () use ($definition, &$inlineServices, $id) {
return $this->createService($definition, $inlineServices, true, $id, false);
}
);
$this->shareService($definition, $proxy, $id, $inlineServices);

return $proxy;
Expand All @@ -1131,19 +1139,21 @@ private function createService(Definition $definition, array &$inlineServices, $
require_once $parameterBag->resolveValue($definition->getFile());
}

$arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlineServices);

if (null !== $id && $definition->isShared() && isset($this->services[$id]) && ($tryProxy || !$definition->isLazy())) {
return $this->services[$id];
}
$arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlineServices, $isConstructorArgument);

if (null !== $factory = $definition->getFactory()) {
if (\is_array($factory)) {
$factory = array($this->doResolveServices($parameterBag->resolveValue($factory[0]), $inlineServices), $factory[1]);
$factory = array($this->doResolveServices($parameterBag->resolveValue($factory[0]), $inlineServices, $isConstructorArgument), $factory[1]);
} elseif (!\is_string($factory)) {
throw new RuntimeException(sprintf('Cannot create service "%s" because of invalid factory', $id));
}
}

if (null !== $id && $definition->isShared() && isset($this->services[$id]) && ($tryProxy || !$definition->isLazy())) {
return $this->services[$id];
}

if (null !== $factory) {
$service = \call_user_func_array($factory, $arguments);

if (!$definition->isDeprecated() && \is_array($factory) && \is_string($factory[0])) {
Expand Down Expand Up @@ -1214,11 +1224,11 @@ public function resolveServices($value)
return $this->doResolveServices($value);
}

private function doResolveServices($value, array &$inlineServices = array())
private function doResolveServices($value, array &$inlineServices = array(), $isConstructorArgument = false)
{
if (\is_array($value)) {
foreach ($value as $k => $v) {
$value[$k] = $this->doResolveServices($v, $inlineServices);
$value[$k] = $this->doResolveServices($v, $inlineServices, $isConstructorArgument);
}
} elseif ($value instanceof ServiceClosureArgument) {
$reference = $value->getValues()[0];
Expand Down Expand Up @@ -1261,9 +1271,9 @@ private function doResolveServices($value, array &$inlineServices = array())
return $count;
});
} elseif ($value instanceof Reference) {
$value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices);
$value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices, $isConstructorArgument);
} elseif ($value instanceof Definition) {
$value = $this->createService($value, $inlineServices);
$value = $this->createService($value, $inlineServices, $isConstructorArgument);
} elseif ($value instanceof Parameter) {
$value = $this->getParameter((string) $value);
} elseif ($value instanceof Expression) {
Expand Down Expand Up @@ -1584,20 +1594,6 @@ protected function getEnv($name)
}
}

/**
* Retrieves the currently set proxy instantiator or instantiates one.
*
* @return InstantiatorInterface
*/
private function getProxyInstantiator()
{
if (!$this->proxyInstantiator) {
$this->proxyInstantiator = new RealServiceInstantiator();
}

return $this->proxyInstantiator;
}

private function callMethod($service, $call, array &$inlineServices)
{
foreach (self::getServiceConditionals($call[1]) as $s) {
Expand Down Expand Up @@ -1627,7 +1623,7 @@ private function shareService(Definition $definition, $service, $id, array &$inl

if (null !== $id && $definition->isShared()) {
$this->services[$id] = $service;
unset($this->loading[$id], $this->alreadyLoading[$id]);
unset($this->loading[$id]);
}
}

Expand Down

0 comments on commit e843bb8

Please sign in to comment.