minor #16810 [DependencyInjection] Validate class names and factory methods (nicolas-grekas)
This PR was merged into the 2.7 branch.
Discussion
----------
[DependencyInjection] Validate class names and factory methods
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Because, you know, people do mistakes... I saw this happening during a workshop: when e.g. the factory tag has no `method` attribute, we generate an container that embeds a parse error.
Commits
-------
b2e3850
[DependencyInjection] Validate class names and factory methods
This commit is contained in:
commit
26d9a6f822
@ -759,6 +759,10 @@ EOF;
|
||||
if (null !== $definition->getFactory()) {
|
||||
$callable = $definition->getFactory();
|
||||
if (is_array($callable)) {
|
||||
if (!preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $callable[1])) {
|
||||
throw new RuntimeException(sprintf('Cannot dump definition because of invalid factory method (%s)', $callable[1] ?: 'n/a'));
|
||||
}
|
||||
|
||||
if ($callable[0] instanceof Reference
|
||||
|| ($callable[0] instanceof Definition && $this->definitionVariables->contains($callable[0]))) {
|
||||
return sprintf(" $return{$instantiation}%s->%s(%s);\n", $this->dumpValue($callable[0]), $callable[1], $arguments ? implode(', ', $arguments) : '');
|
||||
@ -1310,8 +1314,12 @@ EOF;
|
||||
}
|
||||
|
||||
if (is_array($factory)) {
|
||||
if (!preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $factory[1])) {
|
||||
throw new RuntimeException(sprintf('Cannot dump definition because of invalid factory method (%s)', $factory[1] ?: 'n/a'));
|
||||
}
|
||||
|
||||
if (is_string($factory[0])) {
|
||||
return sprintf('\\%s::%s(%s)', $factory[0], $factory[1], implode(', ', $arguments));
|
||||
return sprintf('%s::%s(%s)', $this->dumpLiteralClass($this->dumpValue($factory[0])), $factory[1], implode(', ', $arguments));
|
||||
}
|
||||
|
||||
if ($factory[0] instanceof Definition) {
|
||||
@ -1342,12 +1350,8 @@ EOF;
|
||||
if (null === $class) {
|
||||
throw new RuntimeException('Cannot dump definitions which have no class nor factory.');
|
||||
}
|
||||
$class = $this->dumpValue($class);
|
||||
if (false !== strpos($class, '$')) {
|
||||
throw new RuntimeException('Cannot dump definitions which have a variable class name.');
|
||||
}
|
||||
|
||||
return sprintf('new \\%s(%s)', substr(str_replace('\\\\', '\\', $class), 1, -1), implode(', ', $arguments));
|
||||
return sprintf('new %s(%s)', $this->dumpLiteralClass($this->dumpValue($class)), implode(', ', $arguments));
|
||||
} elseif ($value instanceof Variable) {
|
||||
return '$'.$value;
|
||||
} elseif ($value instanceof Reference) {
|
||||
@ -1388,9 +1392,18 @@ EOF;
|
||||
* @param string $class
|
||||
*
|
||||
* @return string
|
||||
*
|
||||
* @throws RuntimeException
|
||||
*/
|
||||
private function dumpLiteralClass($class)
|
||||
{
|
||||
if (false !== strpos($class, '$')) {
|
||||
throw new RuntimeException('Cannot dump definitions which have a variable class name.');
|
||||
}
|
||||
if (0 !== strpos($class, "'") || !preg_match('/^\'[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(\\\{2}[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*\'$/', $class)) {
|
||||
throw new RuntimeException(sprintf('Cannot dump definition because of invalid class name (%s)', $class ?: 'n/a'));
|
||||
}
|
||||
|
||||
return '\\'.substr(str_replace('\\\\', '\\', $class), 1, -1);
|
||||
}
|
||||
|
||||
|
@ -154,6 +154,31 @@ class PhpDumperTest extends \PHPUnit_Framework_TestCase
|
||||
$dumper->dump();
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideInvalidFactories
|
||||
* @expectedException Symfony\Component\DependencyInjection\Exception\RuntimeException
|
||||
* @expectedExceptionMessage Cannot dump definition
|
||||
*/
|
||||
public function testInvalidFactories($factory)
|
||||
{
|
||||
$container = new ContainerBuilder();
|
||||
$def = new Definition('stdClass');
|
||||
$def->setFactory($factory);
|
||||
$container->setDefinition('bar', $def);
|
||||
$dumper = new PhpDumper($container);
|
||||
$dumper->dump();
|
||||
}
|
||||
|
||||
public function provideInvalidFactories()
|
||||
{
|
||||
return array(
|
||||
array(array('', 'method')),
|
||||
array(array('class', '')),
|
||||
array(array('...', 'method')),
|
||||
array(array('class', '...')),
|
||||
);
|
||||
}
|
||||
|
||||
public function testAliases()
|
||||
{
|
||||
$container = include self::$fixturesPath.'/containers/container9.php';
|
||||
|
Reference in New Issue
Block a user