E571 [DI] Fix false-positive circular ref leading to wrong exceptions or i… · symfony/symfony@19f0880 · GitHub
[go: up one dir, main page]
Skip to content

Commit 19f0880

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] Fix false-positive circular ref leading to wrong exceptions or infinite loops at runtime
1 parent 7b80bc2 commit 19f0880

File tree

9 files changed

+267
-139
lines changed

9 files changed

+267
-139
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ private function getDefinitionId($id, ContainerBuilder $container)
6666
$seen = array();
6767
while ($container->hasAlias($id)) {
6868
if (isset($seen[$id])) {
69-
throw new ServiceCircularReferenceException($id, array_keys($seen));
69+
throw new ServiceCircularReferenceException($id, array_merge(array_keys($seen), array($id)));
7070
}
7171
$seen[$id] = true;
7272
$id = $container->normalizeId($container->getAlias($id));

‎src/Symfony/Component/DependencyInjection/Container.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ public function get($id, $invalidBehavior = /* self::EXCEPTION_ON_INVALID_REFERE
294294
}
295295

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

300300
$this->loading[$id] = true;

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php‎

Lines changed: 45 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
122122
private $autoconfiguredInstanceof = array();
123123

124124
private $removedIds = array();
125-
private $alreadyLoading = array();
126125

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

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

595594
if (isset($inlineServices[$id])) {
596595
return $inlineServices[$id];
597596
}
598-
if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
599-
return parent::get($id, $invalidBehavior);
597+
if (null === $inlineServices) {
598+
$isConstructorArgument = true;
599+
$inlineServices = array();
600600
}
601-
if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
602-
return $service;
601+
try {
602+
if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
603+
return parent::get($id, $invalidBehavior);
604+
}
605+
if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
606+
return $service;
607+
}
608+
} catch (ServiceCircularReferenceException $e) {
609+
if ($isConstructorArgument) {
610+
throw $e;
611+
}
603612
}
604613

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

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

619-
$loading = isset($this->alreadyLoading[$id]) ? 'loading' : 'alreadyLoading';
620-
$this->{$loading}[$id] = true;
628+
if ($isConstructorArgument) {
629+
$this->loading[$id] = true;
630+
}
621631

622632
try {
623-
$service = $this->createService($definition, $inlineServices, $id);
633+
return $this->createService($definition, $inlineServices, $isConstructorArgument, $id);
624634
} finally {
625-
unset($this->{$loading}[$id]);
635+
if ($isConstructorArgument) {
636+
unset($this->loading[$id]);
637+
}
626638
}
627-
628-
return $service;
629639
}
630640

631641
/**
@@ -1092,7 +1102,7 @@ public function findDefinition($id)
10921102
* @throws RuntimeException When the service is a synthetic service
10931103
* @throws InvalidArgumentException When configure callable is not callable
10941104
*/
1095-
private function createService(Definition $definition, array &$inlineServices, $id = null, $tryProxy = true)
1105+
private function createService(Definition $definition, array &$inlineServices, $isConstructorArgument = false, $id = null, $tryProxy = true)
10961106
{
10971107
if (null === $id && isset($inlineServices[$h = spl_object_hash($definition)])) {
10981108
return $inlineServices[$h];
@@ -1110,16 +1120,14 @@ private function createService(Definition $definition, array &$inlineServices, $
11101120
@trigger_error($definition->getDeprecationMessage($id), E_USER_DEPRECATED);
11111121
}
11121122

1113-
if ($tryProxy && $definition->isLazy()) {
1114-
$proxy = $this
1115-
->getProxyInstantiator()
1116-
->instantiateProxy(
1117-
$this,
1118-
$definition,
1119-
$id, function () use ($definition, &$inlineServices, $id) {
1120-
return $this->createService($definition, $inlineServices, $id, false);
1121-
}
1122-
);
1123+
if ($tryProxy && $definition->isLazy() && !$tryProxy = !($proxy = $this->proxyInstantiator) || $proxy instanceof RealServiceInstantiator) {
1124+
$proxy = $proxy->instantiateProxy(
1125+
$this,
1126+
$definition,
1127+
$id, function () use ($definition, &$inlineServices, $id) {
1128+
return $this->createService($definition, $inlineServices, true, $id, false);
1129+
}
1130+
);
11231131
$this->shareService($definition, $proxy, $id, $inlineServices);
11241132

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

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

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

1152+
if (null !== $id && $definition->isShared() && isset($this->services[$id]) && ($tryProxy || !$definition->isLazy())) {
1153+
return $this->services[$id];
1154+
}
1155+
1156+
if (null !== $factory) {
11471157
$service = \call_user_func_array($factory, $arguments);
11481158

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

1217-
private function doResolveServices($value, array &$inlineServices = array())
1227+
private function doResolveServices($value, array &$inlineServices = array(), $isConstructorArgument = false)
12181228
{
12191229
if (\is_array($value)) {
12201230
foreach ($value as $k => $v) {
1221-
$value[$k] = $this->doResolveServices($v, $inlineServices);
1231+
$value[$k] = $this->doResolveServices($v, $inlineServices, $isConstructorArgument);
12221232
}
12231233
} elseif ($value instanceof ServiceClosureArgument) {
12241234
$reference = $value->getValues()[0];
@@ -1261,9 +1271,9 @@ private function doResolveServices($value, array &$inlineServices = array())
12611271
return $count;
12621272
});
12631273
} elseif ($value instanceof Reference) {
1264-
$value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices);
1274+
$value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices, $isConstructorArgument);
12651275
} elseif ($value instanceof Definition) {
1266-
$value = $this->createService($value, $inlineServices);
1276+
$value = $this->createService($value, $inlineServices, $isConstructorArgument);
12671277
} elseif ($value instanceof Parameter) {
12681278
$value = $this->getParameter((string) $value);
12691279
} elseif ($value instanceof Expression) {
@@ -1584,20 +1594,6 @@ protected function getEnv($name)
15841594
}
15851595
}
15861596

1587-
/**
1588-
* Retrieves the currently set proxy instantiator or instantiates one.
1589-
*
1590-
* @return InstantiatorInterface
1591-
*/
1592-
private function getProxyInstantiator()
1593-
{
1594-
if (!$this->proxyInstantiator) {
1595-
$this->proxyInstantiator = new RealServiceInstantiator();
1596-
}
1597-
1598-
return $this->proxyInstantiator;
1599-
}
1600-
16011597
private function callMethod($service, $call, array &$inlineServices)
16021598
{
16031599
foreach (self::getServiceConditionals($call[1]) as $s) {
@@ -1627,7 +1623,7 @@ private function shareService(Definition $definition, $service, $id, array &$inl
16271623

16281624
if (null !== $id && $definition->isShared()) {
16291625
$this->services[$id] = $service;
1630-
unset($this->loading[$id], $this->alreadyLoading[$id]);
1626+
unset($this->loading[$id]);
16311627
}
16321628
}
16331629

0 commit comments

Comments
 (0)
0