Skip to content

Commit

Permalink
bug #26355 [DI] Fix missing "id" normalization when dumping the conta…
Browse files Browse the repository at this point in the history
…iner (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Fix missing "id" normalization when dumping the container

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26306
| License       | MIT
| Doc PR        | -

Commits
-------

4a5e43e [DI] Fix missing "id" normalization when dumping the container
  • Loading branch information
nicolas-grekas committed Mar 2, 2018
2 parents 8f1a6b5 + 4a5e43e commit 6ac7b50
Show file tree
Hide file tree
Showing 17 changed files with 76 additions and 43 deletions.
Expand Up @@ -64,7 +64,8 @@ public function process(ContainerBuilder $container)
$this->lazy = false;

foreach ($container->getAliases() as $id => $alias) {
$this->graph->connect($id, $alias, (string) $alias, $this->getDefinition((string) $alias), null);
$targetId = $this->getDefinitionId((string) $alias);
$this->graph->connect($id, $alias, $targetId, $this->getDefinition($targetId), null);
}

parent::process($container);
Expand All @@ -87,12 +88,13 @@ protected function processValue($value, $isRoot = false)
return $value;
}
if ($value instanceof Reference) {
$targetDefinition = $this->getDefinition((string) $value);
$targetId = $this->getDefinitionId((string) $value);
$targetDefinition = $this->getDefinition($targetId);

$this->graph->connect(
$this->currentId,
$this->currentDefinition,
$this->getDefinitionId((string) $value),
$targetId,
$targetDefinition,
$value,
$this->lazy || ($targetDefinition && $targetDefinition->isLazy()),
Expand Down Expand Up @@ -134,8 +136,6 @@ protected function processValue($value, $isRoot = false)
*/
private function getDefinition($id)
{
$id = $this->getDefinitionId($id);

return null === $id ? null : $this->container->getDefinition($id);
}

Expand All @@ -149,7 +149,7 @@ private function getDefinitionId($id)
return;
}

return $id;
return $this->container->normalizeId($id);
}

private function getExpressionLanguage()
Expand All @@ -163,11 +163,12 @@ private function getExpressionLanguage()
$this->expressionLanguage = new ExpressionLanguage(null, $providers, function ($arg) {
if ('""' === substr_replace($arg, '', 1, -1)) {
$id = stripcslashes(substr($arg, 1, -1));
$id = $this->getDefinitionId($id);

$this->graph->connect(
$this->currentId,
$this->currentDefinition,
$this->getDefinitionId($id),
$id,
$this->getDefinition($id)
);
}
Expand Down
Expand Up @@ -281,7 +281,7 @@ private function getAutowiredReference(TypedReference $reference, $deprecationMe
$this->lastFailure = null;
$type = $reference->getType();

if ($type !== (string) $reference || ($this->container->has($type) && !$this->container->findDefinition($type)->isAbstract())) {
if ($type !== $this->container->normalizeId($reference) || ($this->container->has($type) && !$this->container->findDefinition($type)->isAbstract())) {
return $reference;
}

Expand Down
Expand Up @@ -50,7 +50,7 @@ public function process(ContainerBuilder $container)
$alias = $container->getAlias($inner);
$public = $alias->isPublic();
$private = $alias->isPrivate();
$container->setAlias($renamedId, new Alias((string) $alias, false));
$container->setAlias($renamedId, new Alias($container->normalizeId($alias), false));
} else {
$decoratedDefinition = $container->getDefinition($inner);
$definition->setTags(array_merge($decoratedDefinition->getTags(), $definition->getTags()));
Expand Down
Expand Up @@ -51,13 +51,12 @@ public function process(ContainerBuilder $container)
private function updateDefinition(ContainerBuilder $container, $id, Definition $definition, array $resolveClassPassChanges, array $previous = array())
{
// circular reference
$lcId = strtolower($id);
if (isset($previous[$lcId])) {
if (isset($previous[$id])) {
return;
}

$factory = $definition->getFactory();
if (null === $factory || (!isset($resolveClassPassChanges[$lcId]) && null !== $definition->getClass())) {
if (null === $factory || (!isset($resolveClassPassChanges[$id]) && null !== $definition->getClass())) {
return;
}

Expand All @@ -73,9 +72,10 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $
}
} else {
if ($factory[0] instanceof Reference) {
$previous[$lcId] = true;
$factoryDefinition = $container->findDefinition((string) $factory[0]);
$this->updateDefinition($container, $factory[0], $factoryDefinition, $resolveClassPassChanges, $previous);
$previous[$id] = true;
$factoryId = $container->normalizeId($factory[0]);
$factoryDefinition = $container->findDefinition($factoryId);
$this->updateDefinition($container, $factoryId, $factoryDefinition, $resolveClassPassChanges, $previous);
$class = $factoryDefinition->getClass();
} else {
$class = $factory[0];
Expand Down Expand Up @@ -103,7 +103,7 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $
}
}

if (null !== $returnType && (!isset($resolveClassPassChanges[$lcId]) || $returnType !== $resolveClassPassChanges[$lcId])) {
if (null !== $returnType && (!isset($resolveClassPassChanges[$id]) || $returnType !== $resolveClassPassChanges[$id])) {
@trigger_error(sprintf('Relying on its factory\'s return-type to define the class of service "%s" is deprecated since Symfony 3.3 and won\'t work in 4.0. Set the "class" attribute to "%s" on the service definition instead.', $id, $returnType), E_USER_DEPRECATED);
}
$definition->setClass($returnType);
Expand Down
Expand Up @@ -67,7 +67,7 @@ protected function processValue($value, $isRoot = false)
$value = clone $value;
}

if (!$value instanceof Reference || !$this->container->hasDefinition($id = (string) $value)) {
if (!$value instanceof Reference || !$this->container->hasDefinition($id = $this->container->normalizeId($value))) {
return parent::processValue($value, $isRoot);
}

Expand Down
Expand Up @@ -85,7 +85,7 @@ protected function processValue($value, $isRoot = false)
$serviceMap[$key] = new Reference($type);
}

$subscriberMap[$key] = new TypedReference((string) $serviceMap[$key], $type, $declaringClass, $optionalBehavior ?: ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE);
$subscriberMap[$key] = new TypedReference($this->container->normalizeId($serviceMap[$key]), $type, $declaringClass, $optionalBehavior ?: ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE);
unset($serviceMap[$key]);
}

Expand Down
Expand Up @@ -36,7 +36,7 @@ public function process(ContainerBuilder $container)
$seenAliasTargets = array();
$replacements = array();
foreach ($container->getAliases() as $definitionId => $target) {
$targetId = (string) $target;
$targetId = $container->normalizeId($target);
// Special case: leave this target alone
if ('service_container' === $targetId) {
continue;
Expand Down Expand Up @@ -77,7 +77,7 @@ public function process(ContainerBuilder $container)
*/
protected function processValue($value, $isRoot = false)
{
if ($value instanceof Reference && isset($this->replacements[$referenceId = (string) $value])) {
if ($value instanceof Reference && isset($this->replacements[$referenceId = $this->container->normalizeId($value)])) {
// Perform the replacement
$newId = $this->replacements[$referenceId];
$value = new Reference($newId, $value->getInvalidBehavior());
Expand Down
Expand Up @@ -49,7 +49,7 @@ public function process(ContainerBuilder $container)
*/
protected function processValue($value, $isRoot = false)
{
if ($value instanceof TypedReference && $value->getType() === (string) $value) {
if ($value instanceof TypedReference && $value->getType() === $this->container->normalizeId($value)) {
// Already checked
$bindings = $this->container->getDefinition($this->currentId)->getBindings();

Expand Down
Expand Up @@ -55,7 +55,7 @@ protected function processValue($value, $isRoot = false)
if ($value instanceof Definition && $isRoot && (isset($this->resolvedIds[$this->currentId]) || !$value->hasTag($this->tagName) || $value->isDeprecated())) {
return $value->isDeprecated() ? $value->clearTag($this->tagName) : $value;
}
if ($value instanceof Reference && ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE !== $value->getInvalidBehavior() && $this->container->has($id = (string) $value)) {
if ($value instanceof Reference && ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE !== $value->getInvalidBehavior() && $this->container->has($id = $this->container->normalizeId($value))) {
$definition = $this->container->findDefinition($id);
if (!$definition->hasTag($this->tagName) && !$definition->isDeprecated()) {
$this->resolvedIds[$id] = true;
Expand Down
Expand Up @@ -90,9 +90,7 @@ private function processValue($value, $rootLevel = 0, $level = 0)
$value = array_values($value);
}
} elseif ($value instanceof Reference) {
$id = (string) $value;

if ($this->container->has($id)) {
if ($this->container->has($value)) {
return $value;
}
$invalidBehavior = $value->getInvalidBehavior();
Expand Down
Expand Up @@ -30,7 +30,7 @@ public function process(ContainerBuilder $container)
parent::process($container);

foreach ($container->getAliases() as $id => $alias) {
$aliasId = (string) $alias;
$aliasId = $container->normalizeId($alias);
if ($aliasId !== $defId = $this->getDefinitionId($aliasId, $container)) {
$container->setAlias($id, $defId)->setPublic($alias->isPublic())->setPrivate($alias->isPrivate());
}
Expand All @@ -43,7 +43,7 @@ public function process(ContainerBuilder $container)
protected function processValue($value, $isRoot = false)
{
if ($value instanceof Reference) {
$defId = $this->getDefinitionId($id = (string) $value, $this->container);
$defId = $this->getDefinitionId($id = $this->container->normalizeId($value), $this->container);

if ($defId !== $id) {
return new Reference($defId, $value->getInvalidBehavior());
Expand All @@ -69,7 +69,7 @@ private function getDefinitionId($id, ContainerBuilder $container)
throw new ServiceCircularReferenceException($id, array_keys($seen));
}
$seen[$id] = true;
$id = (string) $container->getAlias($id);
$id = $container->normalizeId($container->getAlias($id));
}

return $id;
Expand Down
Expand Up @@ -26,7 +26,7 @@ class ResolveServiceSubscribersPass extends AbstractRecursivePass

protected function processValue($value, $isRoot = false)
{
if ($value instanceof Reference && $this->serviceLocator && ContainerInterface::class === (string) $value) {
if ($value instanceof Reference && $this->serviceLocator && ContainerInterface::class === $this->container->normalizeId($value)) {
return new Reference($this->serviceLocator);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/Container.php
Expand Up @@ -507,7 +507,7 @@ protected function getEnv($name)
*/
public function normalizeId($id)
{
if (!is_string($id)) {
if (!\is_string($id)) {
$id = (string) $id;
}
if (isset($this->normalizedIds[$normalizedId = strtolower($id)])) {
Expand Down
20 changes: 16 additions & 4 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Expand Up @@ -627,7 +627,7 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
*
* @throws BadMethodCallException When this ContainerBuilder is compiled
*/
public function merge(ContainerBuilder $container)
public function merge(self $container)
{
if ($this->isCompiled()) {
throw new BadMethodCallException('Cannot merge on a compiled container.');
Expand Down Expand Up @@ -1373,16 +1373,16 @@ public function resolveEnvPlaceholders($value, $format = null, array &$usedEnvs
$value = $bag->resolveValue($value);
}

if (is_array($value)) {
if (\is_array($value)) {
$result = array();
foreach ($value as $k => $v) {
$result[$this->resolveEnvPlaceholders($k, $format, $usedEnvs)] = $this->resolveEnvPlaceholders($v, $format, $usedEnvs);
$result[\is_string($k) ? $this->resolveEnvPlaceholders($k, $format, $usedEnvs) : $k] = $this->resolveEnvPlaceholders($v, $format, $usedEnvs);
}

return $result;
}

if (!is_string($value)) {
if (!\is_string($value) || 38 > \strlen($value)) {
return $value;
}
$envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : $this->envPlaceholders;
Expand Down Expand Up @@ -1455,6 +1455,18 @@ public function log(CompilerPassInterface $pass, $message)
$this->getCompiler()->log($pass, $message);
}

/**
* {@inheritdoc}
*/
public function normalizeId($id)
{
if (!\is_string($id)) {
$id = (string) $id;
}

return isset($this->definitions[$id]) || isset($this->aliasDefinitions[$id]) || isset($this->removedIds[$id]) ? $id : parent::normalizeId($id);
}

/**
* Returns the Service Conditionals.
*
Expand Down
14 changes: 8 additions & 6 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Expand Up @@ -1232,9 +1232,9 @@ private function addAliases()
$code = " \$this->aliases = array(\n";
ksort($aliases);
foreach ($aliases as $alias => $id) {
$id = (string) $id;
$id = $this->container->normalizeId($id);
while (isset($aliases[$id])) {
$id = (string) $aliases[$id];
$id = $this->container->normalizeId($aliases[$id]);
}
$code .= ' '.$this->doExport($alias).' => '.$this->doExport($id).",\n";
}
Expand Down Expand Up @@ -1555,7 +1555,7 @@ private function getServiceCallsFromArguments(array $arguments, array &$calls, $
if (is_array($argument)) {
$this->getServiceCallsFromArguments($argument, $calls, $isPreInstance, $callerId, $behavior, $step);
} elseif ($argument instanceof Reference) {
$id = (string) $argument;
$id = $this->container->normalizeId($argument);

if (!isset($calls[$id])) {
$calls[$id] = (int) ($isPreInstance && isset($this->circularReferences[$callerId][$id]));
Expand Down Expand Up @@ -1625,7 +1625,7 @@ private function hasReference($id, array $arguments, $deep = false, array &$visi

continue;
} elseif ($argument instanceof Reference) {
$argumentId = (string) $argument;
$argumentId = $this->container->normalizeId($argument);
if ($id === $argumentId) {
return true;
}
Expand Down Expand Up @@ -1790,11 +1790,12 @@ private function dumpValue($value, $interpolate = true)
} elseif ($value instanceof Variable) {
return '$'.$value;
} elseif ($value instanceof Reference) {
if (null !== $this->referenceVariables && isset($this->referenceVariables[$id = (string) $value])) {
$id = $this->container->normalizeId($value);
if (null !== $this->referenceVariables && isset($this->referenceVariables[$id])) {
return $this->dumpValue($this->referenceVariables[$id], $interpolate);
}

return $this->getServiceCall((string) $value, $value);
return $this->getServiceCall($id, $value);
} elseif ($value instanceof Expression) {
return $this->getExpressionLanguage()->compile((string) $value, array('this' => 'container'));
} elseif ($value instanceof Parameter) {
Expand Down Expand Up @@ -1881,6 +1882,7 @@ private function getServiceCall($id, Reference $reference = null)
while ($this->container->hasAlias($id)) {
$id = (string) $this->container->getAlias($id);
}
$id = $this->container->normalizeId($id);

if ('service_container' === $id) {
return '$this';
Expand Down
Expand Up @@ -173,16 +173,16 @@ public function resolve()
*/
public function resolveValue($value, array $resolving = array())
{
if (is_array($value)) {
if (\is_array($value)) {
$args = array();
foreach ($value as $k => $v) {
$args[$this->resolveValue($k, $resolving)] = $this->resolveValue($v, $resolving);
$args[\is_string($k) ? $this->resolveValue($k, $resolving) : $k] = $this->resolveValue($v, $resolving);
}

return $args;
}

if (!is_string($value)) {
if (!\is_string($value) || 2 > \strlen($value)) {
return $value;
}

Expand Down
Expand Up @@ -1001,6 +1001,26 @@ public function testParameterWithLowerCase()

$this->assertSame('bar', $container->getParameter('FOO'));
}

/**
* @group legacy
* @expectedDeprecation Service identifiers will be made case sensitive in Symfony 4.0. Using "foo" instead of "Foo" is deprecated since Symfony 3.3.
* @expectedDeprecation The "Foo" service is deprecated. You should stop using it, as it will soon be removed.
*/
public function testReferenceWithLowerCaseId()
{
$container = new ContainerBuilder();
$container->register('Bar', 'stdClass')->setProperty('foo', new Reference('foo'))->setPublic(true);
$container->register('Foo', 'stdClass')->setDeprecated();
$container->compile();

$dumper = new PhpDumper($container);
eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Reference_With_Lower_Case_Id')));

$container = new \Symfony_DI_PhpDumper_Test_Reference_With_Lower_Case_Id();

$this->assertEquals((object) array('foo' => (object) array()), $container->get('Bar'));
}
}

class Rot13EnvVarProcessor implements EnvVarProcessorInterface
Expand Down

0 comments on commit 6ac7b50

Please sign in to comment.