bug #22581 [DI] Fix invalid callables dumped for ArgumentInterface objects (nicolas-grekas, iltar)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Fix invalid callables dumped for ArgumentInterface objects | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Follow up of #22511, from @iltar: > Currently, when having a service definition which has more than 1 usage of that service, it will put the argument in `$a` for example. By doing so, it will also use `$a` in the callable of the `ServiceLocator`, which result in an undefined variable `$a`. > I managed to trigger this with the translator service, where I have my own loader, that I add for NL and EN, causing 2 method calls to turn the direct getter into a variable. I've added 2 test cases, 1 with only 1 method call and 1 with 2 method calls. Commits -------f81c577[DI] Test references inside ServiceLocator are not inlined8783602[DI] Fix invalid callables dumped for ArgumentInterface objects
This commit is contained in:
Nicolas Grekas
commit
5edbc13552
@ -12,6 +12,7 @@
|
||||
namespace Symfony\Component\DependencyInjection\Argument;
|
||||
|
||||
use Symfony\Component\DependencyInjection\ContainerInterface;
|
||||
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
|
||||
use Symfony\Component\DependencyInjection\Reference;
|
||||
|
||||
/**
|
||||
@ -41,6 +42,9 @@ class ClosureProxyArgument implements ArgumentInterface
|
||||
*/
|
||||
public function setValues(array $values)
|
||||
{
|
||||
if (!$values[0] instanceof Reference) {
|
||||
throw new InvalidArgumentException(sprintf('A ClosureProxyArgument must hold a Reference, "%s" given.', is_object($values[0]) ? get_class($values[0]) : gettype($values[0])));
|
||||
}
|
||||
list($this->reference, $this->method) = $values;
|
||||
}
|
||||
}
|
||||
|
||||
@ -1410,40 +1410,79 @@ EOF;
|
||||
}
|
||||
|
||||
return sprintf('array(%s)', implode(', ', $code));
|
||||
} elseif ($value instanceof ServiceClosureArgument) {
|
||||
$value = $value->getValues()[0];
|
||||
$code = $this->dumpValue($value, $interpolate);
|
||||
} elseif ($value instanceof ArgumentInterface) {
|
||||
$scope = array($this->definitionVariables, $this->referenceVariables, $this->variableCount);
|
||||
$this->definitionVariables = $this->referenceVariables = null;
|
||||
|
||||
if ($value instanceof TypedReference) {
|
||||
$code = sprintf('$f = function (\\%s $v%s) { return $v; }; return $f(%s);', $value->getType(), ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $value->getInvalidBehavior() ? ' = null' : '', $code);
|
||||
} else {
|
||||
$code = sprintf('return %s;', $code);
|
||||
}
|
||||
try {
|
||||
if ($value instanceof ServiceClosureArgument) {
|
||||
$value = $value->getValues()[0];
|
||||
$code = $this->dumpValue($value, $interpolate);
|
||||
|
||||
return sprintf("function () {\n %s\n }", $code);
|
||||
} elseif ($value instanceof IteratorArgument) {
|
||||
$countCode = array();
|
||||
$countCode[] = 'function () {';
|
||||
$operands = array(0);
|
||||
|
||||
$code = array();
|
||||
$code[] = 'new RewindableGenerator(function () {';
|
||||
foreach ($value->getValues() as $k => $v) {
|
||||
($c = $this->getServiceConditionals($v)) ? $operands[] = "(int) ($c)" : ++$operands[0];
|
||||
$v = $this->wrapServiceConditionals($v, sprintf(" yield %s => %s;\n", $this->dumpValue($k, $interpolate), $this->dumpValue($v, $interpolate)));
|
||||
foreach (explode("\n", $v) as $v) {
|
||||
if ($v) {
|
||||
$code[] = ' '.$v;
|
||||
if ($value instanceof TypedReference) {
|
||||
$code = sprintf('$f = function (\\%s $v%s) { return $v; }; return $f(%s);', $value->getType(), ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $value->getInvalidBehavior() ? ' = null' : '', $code);
|
||||
} else {
|
||||
$code = sprintf('return %s;', $code);
|
||||
}
|
||||
|
||||
return sprintf("function () {\n %s\n }", $code);
|
||||
}
|
||||
|
||||
if ($value instanceof IteratorArgument) {
|
||||
$countCode = array();
|
||||
$countCode[] = 'function () {';
|
||||
$operands = array(0);
|
||||
|
||||
$code = array();
|
||||
$code[] = 'new RewindableGenerator(function () {';
|
||||
foreach ($value->getValues() as $k => $v) {
|
||||
($c = $this->getServiceConditionals($v)) ? $operands[] = "(int) ($c)" : ++$operands[0];
|
||||
$v = $this->wrapServiceConditionals($v, sprintf(" yield %s => %s;\n", $this->dumpValue($k, $interpolate), $this->dumpValue($v, $interpolate)));
|
||||
foreach (explode("\n", $v) as $v) {
|
||||
if ($v) {
|
||||
$code[] = ' '.$v;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$countCode[] = sprintf(' return %s;', implode(' + ', $operands));
|
||||
$countCode[] = ' }';
|
||||
|
||||
$code[] = sprintf(' }, %s)', count($operands) > 1 ? implode("\n", $countCode) : $operands[0]);
|
||||
|
||||
return implode("\n", $code);
|
||||
}
|
||||
|
||||
if ($value instanceof ClosureProxyArgument) {
|
||||
list($reference, $method) = $value->getValues();
|
||||
$method = substr($this->dumpLiteralClass($this->dumpValue($method)), 1);
|
||||
|
||||
if ('service_container' === (string) $reference) {
|
||||
$class = $this->baseClass;
|
||||
} elseif (!$this->container->hasDefinition((string) $reference) && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $reference->getInvalidBehavior()) {
|
||||
return 'null';
|
||||
} else {
|
||||
$class = substr($this->dumpLiteralClass($this->dumpValue($this->container->findDefinition((string) $reference)->getClass())), 1);
|
||||
}
|
||||
if (false !== strpos($class, '$') || false !== strpos($method, '$')) {
|
||||
throw new RuntimeException(sprintf('Cannot dump definition for service "%s": dynamic class names or methods, and closure-proxies are incompatible with each other.', $reference));
|
||||
}
|
||||
if (!method_exists($class, $method)) {
|
||||
throw new InvalidArgumentException(sprintf('Cannot create closure-proxy for service "%s": method "%s::%s" does not exist.', $reference, $class, $method));
|
||||
}
|
||||
$r = $this->container->getReflectionClass($class)->getMethod($method);
|
||||
if (!$r->isPublic()) {
|
||||
throw new InvalidArgumentException(sprintf('Cannot create closure-proxy for service "%s": method "%s::%s" must be public.', $reference, $class, $method));
|
||||
}
|
||||
$signature = preg_replace('/^(&?)[^(]*/', '$1', ProxyHelper::getSignature($r, $call));
|
||||
|
||||
$return = 'void' !== ProxyHelper::getTypeHint($r);
|
||||
|
||||
return sprintf("/** @closure-proxy %s::%s */ function %s {\n %s%s->%s;\n }", $class, $method, $signature, $return ? 'return ' : '', $this->dumpValue($reference), $call);
|
||||
}
|
||||
} finally {
|
||||
list($this->definitionVariables, $this->referenceVariables, $this->variableCount) = $scope;
|
||||
}
|
||||
|
||||
$countCode[] = sprintf(' return %s;', implode(' + ', $operands));
|
||||
$countCode[] = ' }';
|
||||
|
||||
$code[] = sprintf(' }, %s)', count($operands) > 1 ? implode("\n", $countCode) : $operands[0]);
|
||||
|
||||
return implode("\n", $code);
|
||||
} elseif ($value instanceof Definition) {
|
||||
if (null !== $this->definitionVariables && $this->definitionVariables->contains($value)) {
|
||||
return $this->dumpValue($this->definitionVariables->offsetGet($value), $interpolate);
|
||||
@ -1497,32 +1536,6 @@ EOF;
|
||||
}
|
||||
|
||||
return sprintf('new %s(%s)', $this->dumpLiteralClass($this->dumpValue($class)), implode(', ', $arguments));
|
||||
} elseif ($value instanceof ClosureProxyArgument) {
|
||||
list($reference, $method) = $value->getValues();
|
||||
$method = substr($this->dumpLiteralClass($this->dumpValue($method)), 1);
|
||||
|
||||
if ('service_container' === (string) $reference) {
|
||||
$class = $this->baseClass;
|
||||
} elseif (!$this->container->hasDefinition((string) $reference) && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $reference->getInvalidBehavior()) {
|
||||
return 'null';
|
||||
} else {
|
||||
$class = substr($this->dumpLiteralClass($this->dumpValue($this->container->findDefinition((string) $reference)->getClass())), 1);
|
||||
}
|
||||
if (false !== strpos($class, '$') || false !== strpos($method, '$')) {
|
||||
throw new RuntimeException(sprintf('Cannot dump definition for service "%s": dynamic class names or methods, and closure-proxies are incompatible with each other.', $reference));
|
||||
}
|
||||
if (!method_exists($class, $method)) {
|
||||
throw new InvalidArgumentException(sprintf('Cannot create closure-proxy for service "%s": method "%s::%s" does not exist.', $reference, $class, $method));
|
||||
}
|
||||
$r = $this->container->getReflectionClass($class)->getMethod($method);
|
||||
if (!$r->isPublic()) {
|
||||
throw new InvalidArgumentException(sprintf('Cannot create closure-proxy for service "%s": method "%s::%s" must be public.', $reference, $class, $method));
|
||||
}
|
||||
$signature = preg_replace('/^(&?)[^(]*/', '$1', ProxyHelper::getSignature($r, $call));
|
||||
|
||||
$return = 'void' !== ProxyHelper::getTypeHint($r);
|
||||
|
||||
return sprintf("/** @closure-proxy %s::%s */ function %s {\n %s%s->%s;\n }", $class, $method, $signature, $return ? 'return ' : '', $this->dumpValue($reference), $call);
|
||||
} elseif ($value instanceof Variable) {
|
||||
return '$'.$value;
|
||||
} elseif ($value instanceof Reference) {
|
||||
|
||||
@ -22,6 +22,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
|
||||
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
|
||||
use Symfony\Component\DependencyInjection\Reference;
|
||||
use Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator;
|
||||
use Symfony\Component\DependencyInjection\TypedReference;
|
||||
use Symfony\Component\DependencyInjection\Definition;
|
||||
use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
|
||||
@ -534,6 +535,40 @@ class PhpDumperTest extends TestCase
|
||||
'nil' => $nil = new ServiceClosureArgument(new Reference('nil')),
|
||||
))
|
||||
;
|
||||
|
||||
// no method calls
|
||||
$container->register('translator.loader_1', 'stdClass');
|
||||
$container->register('translator.loader_1_locator', ServiceLocator::class)
|
||||
->setPublic(false)
|
||||
->addArgument(array(
|
||||
'translator.loader_1' => new ServiceClosureArgument(new Reference('translator.loader_1')),
|
||||
));
|
||||
$container->register('translator_1', StubbedTranslator::class)
|
||||
->addArgument(new Reference('translator.loader_1_locator'));
|
||||
|
||||
// one method calls
|
||||
$container->register('translator.loader_2', 'stdClass');
|
||||
$container->register('translator.loader_2_locator', ServiceLocator::class)
|
||||
->setPublic(false)
|
||||
->addArgument(array(
|
||||
'translator.loader_2' => new ServiceClosureArgument(new Reference('translator.loader_2')),
|
||||
));
|
||||
$container->register('translator_2', StubbedTranslator::class)
|
||||
->addArgument(new Reference('translator.loader_2_locator'))
|
||||
->addMethodCall('addResource', array('db', new Reference('translator.loader_2'), 'nl'));
|
||||
|
||||
// two method calls
|
||||
$container->register('translator.loader_3', 'stdClass');
|
||||
$container->register('translator.loader_3_locator', ServiceLocator::class)
|
||||
->setPublic(false)
|
||||
->addArgument(array(
|
||||
'translator.loader_3' => new ServiceClosureArgument(new Reference('translator.loader_3')),
|
||||
));
|
||||
$container->register('translator_3', StubbedTranslator::class)
|
||||
->addArgument(new Reference('translator.loader_3_locator'))
|
||||
->addMethodCall('addResource', array('db', new Reference('translator.loader_3'), 'nl'))
|
||||
->addMethodCall('addResource', array('db', new Reference('translator.loader_3'), 'en'));
|
||||
|
||||
$nil->setValues(array(null));
|
||||
$container->register('bar_service', 'stdClass')->setArguments(array(new Reference('baz_service')));
|
||||
$container->register('baz_service', 'stdClass')->setPublic(false);
|
||||
|
||||
@ -0,0 +1,29 @@
|
||||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the Symfony package.
|
||||
*
|
||||
* (c) Fabien Potencier <fabien@symfony.com>
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
|
||||
|
||||
use Psr\Container\ContainerInterface;
|
||||
|
||||
/**
|
||||
* @author Iltar van der Berg <kjarli@gmail.com>
|
||||
*/
|
||||
class StubbedTranslator
|
||||
{
|
||||
public function __construct(ContainerInterface $container)
|
||||
{
|
||||
|
||||
}
|
||||
|
||||
public function addResource($format, $resource, $locale, $domain = null)
|
||||
{
|
||||
}
|
||||
}
|
||||
@ -31,6 +31,12 @@ class ProjectServiceContainer extends Container
|
||||
'bar_service' => 'getBarServiceService',
|
||||
'baz_service' => 'getBazServiceService',
|
||||
'foo_service' => 'getFooServiceService',
|
||||
'translator.loader_1' => 'getTranslator_Loader1Service',
|
||||
'translator.loader_2' => 'getTranslator_Loader2Service',
|
||||
'translator.loader_3' => 'getTranslator_Loader3Service',
|
||||
'translator_1' => 'getTranslator1Service',
|
||||
'translator_2' => 'getTranslator2Service',
|
||||
'translator_3' => 'getTranslator3Service',
|
||||
);
|
||||
$this->privates = array(
|
||||
'baz_service' => true,
|
||||
@ -97,6 +103,101 @@ class ProjectServiceContainer extends Container
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the 'translator.loader_1' service.
|
||||
*
|
||||
* This service is shared.
|
||||
* This method always returns the same instance of the service.
|
||||
*
|
||||
* @return \stdClass A stdClass instance
|
||||
*/
|
||||
protected function getTranslator_Loader1Service()
|
||||
{
|
||||
return $this->services['translator.loader_1'] = new \stdClass();
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the 'translator.loader_2' service.
|
||||
*
|
||||
* This service is shared.
|
||||
* This method always returns the same instance of the service.
|
||||
*
|
||||
* @return \stdClass A stdClass instance
|
||||
*/
|
||||
protected function getTranslator_Loader2Service()
|
||||
{
|
||||
return $this->services['translator.loader_2'] = new \stdClass();
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the 'translator.loader_3' service.
|
||||
*
|
||||
* This service is shared.
|
||||
* This method always returns the same instance of the service.
|
||||
*
|
||||
* @return \stdClass A stdClass instance
|
||||
*/
|
||||
protected function getTranslator_Loader3Service()
|
||||
{
|
||||
return $this->services['translator.loader_3'] = new \stdClass();
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the 'translator_1' service.
|
||||
*
|
||||
* This service is shared.
|
||||
* This method always returns the same instance of the service.
|
||||
*
|
||||
* @return \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator A Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator instance
|
||||
*/
|
||||
protected function getTranslator1Service()
|
||||
{
|
||||
return $this->services['translator_1'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(array('translator.loader_1' => function () {
|
||||
return ${($_ = isset($this->services['translator.loader_1']) ? $this->services['translator.loader_1'] : $this->get('translator.loader_1')) && false ?: '_'};
|
||||
})));
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the 'translator_2' service.
|
||||
*
|
||||
* This service is shared.
|
||||
* This method always returns the same instance of the service.
|
||||
*
|
||||
* @return \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator A Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator instance
|
||||
*/
|
||||
protected function getTranslator2Service()
|
||||
{
|
||||
$this->services['translator_2'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(array('translator.loader_2' => function () {
|
||||
return ${($_ = isset($this->services['translator.loader_2']) ? $this->services['translator.loader_2'] : $this->get('translator.loader_2')) && false ?: '_'};
|
||||
})));
|
||||
|
||||
$instance->addResource('db', ${($_ = isset($this->services['translator.loader_2']) ? $this->services['translator.loader_2'] : $this->get('translator.loader_2')) && false ?: '_'}, 'nl');
|
||||
|
||||
return $instance;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the 'translator_3' service.
|
||||
*
|
||||
* This service is shared.
|
||||
* This method always returns the same instance of the service.
|
||||
*
|
||||
* @return \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator A Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator instance
|
||||
*/
|
||||
protected function getTranslator3Service()
|
||||
{
|
||||
$a = ${($_ = isset($this->services['translator.loader_3']) ? $this->services['translator.loader_3'] : $this->get('translator.loader_3')) && false ?: '_'};
|
||||
|
||||
$this->services['translator_3'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(array('translator.loader_3' => function () {
|
||||
return ${($_ = isset($this->services['translator.loader_3']) ? $this->services['translator.loader_3'] : $this->get('translator.loader_3')) && false ?: '_'};
|
||||
})));
|
||||
|
||||
$instance->addResource('db', $a, 'nl');
|
||||
$instance->addResource('db', $a, 'en');
|
||||
|
||||
return $instance;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the 'baz_service' service.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user