Skip to content

Commit 3fd01ab

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '3.4' into 4.2
* 3.4: [DI] Check service IDs are valid
2 parents 9191645 + 47cd029 commit 3fd01ab

File tree

12 files changed

+239
-39
lines changed

12 files changed

+239
-39
lines changed

‎src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function getProxyFactoryCode(Definition $definition, $id, $factoryCode =
5353
$instantiation = 'return';
5454

5555
if ($definition->isShared()) {
56-
$instantiation .= sprintf(' $this->%s[\'%s\'] =', $definition->isPublic() && !$definition->isPrivate() ? 'services' : 'privates', $id);
56+
$instantiation .= sprintf(' $this->%s[%s] =', $definition->isPublic() && !$definition->isPrivate() ? 'services' : 'privates', var_export($id, true));
5757
}
5858

5959
if (null === $factoryCode) {

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,10 @@ public function setAlias($alias, $id)
828828
{
829829
$alias = (string) $alias;
830830

831+
if ('' === $alias || '\\' === $alias[-1] || \strlen($alias) !== strcspn($alias, "\0\r\n'")) {
832+
throw new InvalidArgumentException(sprintf('Invalid alias id: "%s"', $alias));
833+
}
834+
831835
if (\is_string($id)) {
832836
$id = new Alias($id);
833837
} elseif (!$id instanceof Alias) {
@@ -981,6 +985,10 @@ public function setDefinition($id, Definition $definition)
981985

982986
$id = (string) $id;
983987

988+
if ('' === $id || '\\' === $id[-1] || \strlen($id) !== strcspn($id, "\0\r\n'")) {
989+
throw new InvalidArgumentException(sprintf('Invalid service id: "%s"', $id));
990+
}
991+
984992
unset($this->aliasDefinitions[$id], $this->removedIds[$id]);
985993

986994
return $this->definitions[$id] = $definition;

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ private function addServiceInstance(string $id, Definition $definition, bool $is
506506
$instantiation = '';
507507

508508
if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
509-
$instantiation = sprintf('$this->%s[\'%s\'] = %s', $this->container->getDefinition($id)->isPublic() ? 'services' : 'privates', $id, $isSimpleInstance ? '' : '$instance');
509+
$instantiation = sprintf('$this->%s[%s] = %s', $this->container->getDefinition($id)->isPublic() ? 'services' : 'privates', $this->doExport($id), $isSimpleInstance ? '' : '$instance');
510510
} elseif (!$isSimpleInstance) {
511511
$instantiation = '$instance';
512512
}
@@ -674,6 +674,9 @@ private function addService(string $id, Definition $definition): array
674674
* Gets the $public '$id'$shared$autowired service.
675675
*
676676
* $return
677+
EOF;
678+
$code = str_replace('*/', ' ', $code).<<<EOF
679+
677680
*/
678681
protected function {$methodName}($lazyInitialization)
679682
{
@@ -687,8 +690,8 @@ protected function {$methodName}($lazyInitialization)
687690
$code .= $this->addServiceInclude($id, $definition);
688691

689692
if ($this->getProxyDumper()->isProxyCandidate($definition)) {
690-
$factoryCode = $asFile ? ($definition->isShared() ? "\$this->load('%s.php', false)" : "\$this->factories['%2$s'](false)") : '$this->%s(false)';
691-
$code .= $this->getProxyDumper()->getProxyFactoryCode($definition, $id, sprintf($factoryCode, $methodName, $this->export($id)));
693+
$factoryCode = $asFile ? ($definition->isShared() ? "\$this->load('%s.php', false)" : "\$this->factories[%2\$s](false)") : '$this->%s(false)';
694+
$code .= $this->getProxyDumper()->getProxyFactoryCode($definition, $id, sprintf($factoryCode, $methodName, $this->doExport($id)));
692695
}
693696

694697
if ($definition->isDeprecated()) {
@@ -762,14 +765,14 @@ private function addInlineReference(string $id, Definition $definition, string $
762765

763766
$code .= sprintf(<<<'EOTXT'
764767
765-
if (isset($this->%s['%s'])) {
766-
return $this->%1$s['%2$s'];
768+
if (isset($this->%s[%s])) {
769+
return $this->%1$s[%2$s];
767770
}
768771

769772
EOTXT
770773
,
771774
$this->container->getDefinition($id)->isPublic() ? 'services' : 'privates',
772-
$id
775+
$this->doExport($id)
773776
);
774777

775778
return $code;
@@ -861,7 +864,7 @@ private function generateServiceFiles(array $services)
861864
$code = ["\n", $code];
862865
}
863866
$code[1] = implode("\n", array_map(function ($line) { return $line ? ' '.$line : $line; }, explode("\n", $code[1])));
864-
$factory = sprintf('$this->factories%s[\'%s\']', $definition->isPublic() ? '' : "['service_container']", $id);
867+
$factory = sprintf('$this->factories%s[%s]', $definition->isPublic() ? '' : "['service_container']", $this->doExport($id));
865868
$lazyloadInitialization = $definition->isLazy() ? '$lazyLoad = true' : '';
866869

867870
$code[1] = sprintf("%s = function (%s) {\n%s};\n\nreturn %1\$s();\n", $factory, $lazyloadInitialization, $code[1]);
@@ -1383,14 +1386,14 @@ private function getServiceConditionals($value): string
13831386
if (!$this->container->hasDefinition($service)) {
13841387
return 'false';
13851388
}
1386-
$conditions[] = sprintf("isset(\$this->%s['%s'])", $this->container->getDefinition($service)->isPublic() ? 'services' : 'privates', $service);
1389+
$conditions[] = sprintf("isset(\$this->%s[%s])", $this->container->getDefinition($service)->isPublic() ? 'services' : 'privates', $this->doExport($service));
13871390
}
13881391
foreach (ContainerBuilder::getServiceConditionals($value) as $service) {
13891392
if ($this->container->hasDefinition($service) && !$this->container->getDefinition($service)->isPublic()) {
13901393
continue;
13911394
}
13921395

1393-
$conditions[] = sprintf("\$this->has('%s')", $service);
1396+
$conditions[] = sprintf("\$this->has(%s)", $this->doExport($service));
13941397
}
13951398

13961399
if (!$conditions) {
@@ -1508,7 +1511,7 @@ private function dumpValue($value, bool $interpolate = true): string
15081511
$serviceMap .= sprintf("\n %s => [%s, %s, %s, %s],",
15091512
$this->export($k),
15101513
$this->export($definition->isShared() ? ($definition->isPublic() ? 'services' : 'privates') : false),
1511-
$this->export($id),
1514+
$this->doExport($id),
15121515
$this->export(ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE !== $v->getInvalidBehavior() && !\is_string($load) ? $this->generateMethodName($id).($load ? '.php' : '') : null),
15131516
$this->export($load)
15141517
);
@@ -1605,11 +1608,11 @@ private function dumpParameter(string $name): string
16051608
}
16061609

16071610
if (!preg_match("/\\\$this->(?:getEnv\('(?:\w++:)*+\w++'\)|targetDirs\[\d++\])/", $dumpedValue)) {
1608-
return sprintf("\$this->parameters['%s']", $name);
1611+
return sprintf('$this->parameters[%s]', $this->doExport($name));
16091612
}
16101613
}
16111614

1612-
return sprintf("\$this->getParameter('%s')", $name);
1615+
return sprintf('$this->getParameter(%s)', $this->doExport($name));
16131616
}
16141617

16151618
private function getServiceCall(string $id, Reference $reference = null): string
@@ -1624,7 +1627,7 @@ private function getServiceCall(string $id, Reference $reference = null): string
16241627

16251628
if ($this->container->hasDefinition($id) && $definition = $this->container->getDefinition($id)) {
16261629
if ($definition->isSynthetic()) {
1627-
$code = sprintf('$this->get(\'%s\'%s)', $id, null !== $reference ? ', '.$reference->getInvalidBehavior() : '');
1630+
$code = sprintf('$this->get(%s%s)', $this->doExport($id), null !== $reference ? ', '.$reference->getInvalidBehavior() : '');
16281631
} elseif (null !== $reference && ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $reference->getInvalidBehavior()) {
16291632
$code = 'null';
16301633
if (!$definition->isShared()) {
@@ -1638,20 +1641,20 @@ private function getServiceCall(string $id, Reference $reference = null): string
16381641
}
16391642
$code = $this->addNewInstance($definition, '', $id);
16401643
if ($definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
1641-
$code = sprintf('$this->%s[\'%s\'] = %s', $definition->isPublic() ? 'services' : 'privates', $id, $code);
1644+
$code = sprintf('$this->%s[%s] = %s', $definition->isPublic() ? 'services' : 'privates', $this->doExport($id), $code);
16421645
}
16431646
$code = "($code)";
16441647
} elseif ($this->asFiles && !$this->isHotPath($definition)) {
16451648
$code = sprintf("\$this->load('%s.php')", $this->generateMethodName($id));
16461649
if (!$definition->isShared()) {
1647-
$factory = sprintf('$this->factories%s[\'%s\']', $definition->isPublic() ? '' : "['service_container']", $id);
1650+
$factory = sprintf('$this->factories%s[%s]', $definition->isPublic() ? '' : "['service_container']", $this->doExport($id));
16481651
$code = sprintf('(isset(%s) ? %1$s() : %s)', $factory, $code);
16491652
}
16501653
} else {
16511654
$code = sprintf('$this->%s()', $this->generateMethodName($id));
16521655
}
16531656
if ($definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
1654-
$code = sprintf('($this->%s[\'%s\'] ?? %s)', $definition->isPublic() ? 'services' : 'privates', $id, $code);
1657+
$code = sprintf('($this->%s[%s] ?? %s)', $definition->isPublic() ? 'services' : 'privates', $this->doExport($id), $code);
16551658
}
16561659

16571660
return $code;
@@ -1660,12 +1663,12 @@ private function getServiceCall(string $id, Reference $reference = null): string
16601663
return 'null';
16611664
}
16621665
if (null !== $reference && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE < $reference->getInvalidBehavior()) {
1663-
$code = sprintf('$this->get(\'%s\', /* ContainerInterface::NULL_ON_INVALID_REFERENCE */ %d)', $id, ContainerInterface::NULL_ON_INVALID_REFERENCE);
1666+
$code = sprintf('$this->get(%s, /* ContainerInterface::NULL_ON_INVALID_REFERENCE */ %d)', $this->doExport($id), ContainerInterface::NULL_ON_INVALID_REFERENCE);
16641667
} else {
1665-
$code = sprintf('$this->get(\'%s\')', $id);
1668+
$code = sprintf('$this->get(%s)', $this->doExport($id));
16661669
}
16671670

1668-
return sprintf('($this->services[\'%s\'] ?? %s)', $id, $code);
1671+
return sprintf('($this->services[%s] ?? %s)', $this->doExport($id), $code);
16691672
}
16701673

16711674
/**

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,38 @@ public function testNonSharedServicesReturnsDifferentInstances()
195195
$this->assertNotSame($builder->get('bar'), $builder->get('bar'));
196196
}
197197

198+
/**
199+
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
200+
* @dataProvider provideBadId
201+
*/
202+
public function testBadAliasId($id)
203+
{
204+
$builder = new ContainerBuilder();
205+
$builder->setAlias($id, 'foo');
206+
}
207+
208+
/**
209+
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
210+
* @dataProvider provideBadId
211+
*/
212+
public function testBadDefinitionId($id)
213+
{
214+
$builder = new ContainerBuilder();
215+
$builder->setDefinition($id, new Definition('Foo'));
216+
}
217+
218+
public function provideBadId()
219+
{
220+
return [
221+
[''],
222+
["\0"],
223+
["\r"],
224+
["\n"],
225+
["'"],
226+
['ab\\'],
227+
];
228+
}
229+
198230
/**
199231
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
200232
* @expectedExceptionMessage You have requested a synthetic service ("foo"). The DIC does not know how to construct this service.

‎src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,12 +261,18 @@ public function testAddServiceIdWithUnsupportedCharacters()
261261
{
262262
$class = 'Symfony_DI_PhpDumper_Test_Unsupported_Characters';
263263
$container = new ContainerBuilder();
264+
$container->setParameter("'", 'oh-no');
265+
$container->register("foo*/oh-no", 'FooClass')->setPublic(true);
264266
$container->register('bar$', 'FooClass')->setPublic(true);
265267
$container->register('bar$!', 'FooClass')->setPublic(true);
266268
$container->compile();
267269
$dumper = new PhpDumper($container);
268-
eval('?>'.$dumper->dump(['class' => $class]));
269270

271+
$this->assertStringEqualsFile(self::$fixturesPath.'/php/services_unsupported_characters.php', $dumper->dump(['class' => $class]));
272+
273+
require_once self::$fixturesPath.'/php/services_unsupported_characters.php';
274+
275+
$this->assertTrue(method_exists($class, 'getFooOhNoService'));
270276
$this->assertTrue(method_exists($class, 'getBarService'));
271277
$this->assertTrue(method_exists($class, 'getBar2Service'));
272278
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services33.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function getRemovedIds()
5555
*/
5656
protected function getFooService()
5757
{
58-
return $this->services['Bar\Foo'] = new \Bar\Foo();
58+
return $this->services['Bar\\Foo'] = new \Bar\Foo();
5959
}
6060

6161
/**
@@ -65,6 +65,6 @@ protected function getFooService()
6565
*/
6666
protected function getFoo2Service()
6767
{
68-
return $this->services['Foo\Foo'] = new \Foo\Foo();
68+
return $this->services['Foo\\Foo'] = new \Foo\Foo();
6969
}
7070
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_adawson.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ public function getRemovedIds()
6060
*/
6161
protected function getBusService()
6262
{
63-
$a = ($this->services['App\Db'] ?? $this->getDbService());
63+
$a = ($this->services['App\\Db'] ?? $this->getDbService());
6464

65-
$this->services['App\Bus'] = $instance = new \App\Bus($a);
65+
$this->services['App\\Bus'] = $instance = new \App\Bus($a);
6666

67-
$b = ($this->privates['App\Schema'] ?? $this->getSchemaService());
67+
$b = ($this->privates['App\\Schema'] ?? $this->getSchemaService());
6868
$c = new \App\Registry();
6969
$c->processor = [0 => $a, 1 => $instance];
7070

@@ -83,9 +83,9 @@ protected function getBusService()
8383
*/
8484
protected function getDbService()
8585
{
86-
$this->services['App\Db'] = $instance = new \App\Db();
86+
$this->services['App\\Db'] = $instance = new \App\Db();
8787

88-
$instance->schema = ($this->privates['App\Schema'] ?? $this->getSchemaService());
88+
$instance->schema = ($this->privates['App\\Schema'] ?? $this->getSchemaService());
8989

9090
return $instance;
9191
}
@@ -97,12 +97,12 @@ protected function getDbService()
9797
*/
9898
protected function getSchemaService()
9999
{
100-
$a = ($this->services['App\Db'] ?? $this->getDbService());
100+
$a = ($this->services['App\\Db'] ?? $this->getDbService());
101101

102-
if (isset($this->privates['App\Schema'])) {
103-
return $this->privates['App\Schema'];
102+
if (isset($this->privates['App\\Schema'])) {
103+
return $this->privates['App\\Schema'];
104104
}
105105

106-
return $this->privates['App\Schema'] = new \App\Schema($a);
106+
return $this->privates['App\\Schema'] = new \App\Schema($a);
107107
}
108108
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_requires.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function getRemovedIds()
7070
*/
7171
protected function getParentNotExistsService()
7272
{
73-
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\ParentNotExists'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\ParentNotExists();
73+
return $this->services['Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\ParentNotExists'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\ParentNotExists();
7474
}
7575

7676
/**
@@ -80,7 +80,7 @@ protected function getParentNotExistsService()
8080
*/
8181
protected function getC1Service()
8282
{
83-
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C1'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C1();
83+
return $this->services['Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\includes\\HotPath\\C1'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C1();
8484
}
8585

8686
/**
@@ -93,7 +93,7 @@ protected function getC2Service()
9393
include_once $this->targetDirs[1].'/includes/HotPath/C2.php';
9494
include_once $this->targetDirs[1].'/includes/HotPath/C3.php';
9595

96-
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2(new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3());
96+
return $this->services['Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\includes\\HotPath\\C2'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C2(new \Symfony\Component\DependencyInjection\Tests\Fixtures\includes\HotPath\C3());
9797
}
9898

9999
public function getParameter($name)

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_inline_self_ref.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ protected function getFooService()
5959
$b = new \App\Baz($a);
6060
$b->bar = $a;
6161

62-
$this->services['App\Foo'] = $instance = new \App\Foo($b);
62+
$this->services['App\\Foo'] = $instance = new \App\Foo($b);
6363

6464
$a->foo = $instance;
6565

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_rot13_env.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function getRemovedIds()
6060
*/
6161
protected function getRot13EnvVarProcessorService()
6262
{
63-
return $this->services['Symfony\Component\DependencyInjection\Tests\Dumper\Rot13EnvVarProcessor'] = new \Symfony\Component\DependencyInjection\Tests\Dumper\Rot13EnvVarProcessor();
63+
return $this->services['Symfony\\Component\\DependencyInjection\\Tests\\Dumper\\Rot13EnvVarProcessor'] = new \Symfony\Component\DependencyInjection\Tests\Dumper\Rot13EnvVarProcessor();
6464
}
6565

6666
/**

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function getRemovedIds()
6060
*/
6161
protected function getTestServiceSubscriberService()
6262
{
63-
return $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber();
63+
return $this->services['Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\TestServiceSubscriber'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber();
6464
}
6565

6666
/**
@@ -85,6 +85,6 @@ protected function getFooServiceService()
8585
*/
8686
protected function getCustomDefinitionService()
8787
{
88-
return $this->privates['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition();
88+
return $this->privates['Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition();
8989
}
9090
}

0 commit comments

Comments
 (0)