Skip to content

Commit 6152822

Browse files
fabpotfabpot
committed
bug #25671 Remove randomness from dumped containers (nicolas-grekas)
This PR was merged into the 3.4 branch. Discussion ---------- Remove randomness from dumped containers | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - With this PR, the generated container is immutable by cache clearing: it doesn't contain any random string anymore (well, third party bundles can add random things back, but at least core doesn't). Since the class+file name of the container is based on a hash of its content, it means that they are now stable also. This should help fix some edge cases/race conditions during cache clears/rebuilds. (fabbot failure is false positive) Commits ------- 14dd5d1dbd Remove randomness from dumped containers
2 parents 4226500 + a9bb2eb commit 6152822

File tree

4 files changed

+57
-12
lines changed

4 files changed

+57
-12
lines changed

‎ContainerBuilder.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,8 @@ private function doResolveServices($value, array &$inlineServices = array())
12391239
$value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices);
12401240
} elseif ($value instanceof Definition) {
12411241
$value = $this->createService($value, $inlineServices);
1242+
} elseif ($value instanceof Parameter) {
1243+
$value = $this->getParameter((string) $value);
12421244
} elseif ($value instanceof Expression) {
12431245
$value = $this->getExpressionLanguage()->evaluate($value, array('container' => $this));
12441246
}

‎Dumper/PhpDumper.php

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ public function dump(array $options = array())
211211
array_pop($code);
212212
$code["Container{$hash}/{$options['class']}.php"] = substr_replace($files[$options['class'].'.php'], "<?php\n\nnamespace Container{$hash};\n", 0, 6);
213213
$namespaceLine = $this->namespace ? "\nnamespace {$this->namespace};\n" : '';
214+
$time = time();
215+
$id = hash('crc32', $hash.$time);
214216

215217
$code[$options['class'].'.php'] = <<<EOF
216218
<?php
@@ -229,7 +231,11 @@ public function dump(array $options = array())
229231
\\class_alias(\\Container{$hash}\\{$options['class']}::class, {$options['class']}::class, false);
230232
}
231233
232-
return new \\Container{$hash}\\{$options['class']}();
234+
return new \\Container{$hash}\\{$options['class']}(array(
235+
'container.build_hash' => '$hash',
236+
'container.build_id' => '$id',
237+
'container.build_time' => $time,
238+
));
233239
234240
EOF;
235241
} else {
@@ -564,15 +570,15 @@ private function isTrivialInstance(Definition $definition)
564570
}
565571

566572
foreach ($definition->getArguments() as $arg) {
567-
if (!$arg) {
573+
if (!$arg || $arg instanceof Parameter) {
568574
continue;
569575
}
570576
if (is_array($arg) && 3 >= count($arg)) {
571577
foreach ($arg as $k => $v) {
572578
if ($this->dumpValue($k) !== $this->dumpValue($k, false)) {
573579
return false;
574580
}
575-
if (!$v) {
581+
if (!$v || $v instanceof Parameter) {
576582
continue;
577583
}
578584
if ($v instanceof Reference && $this->container->has($id = (string) $v) && $this->container->findDefinition($id)->isSynthetic()) {
@@ -892,10 +898,10 @@ private function addNewInstance(Definition $definition, $return, $instantiation,
892898
}
893899

894900
if (0 === strpos($class, 'new ')) {
895-
return $return.sprintf("(%s)->%s(%s);\n", $this->dumpValue($callable[0]), $callable[1], $arguments ? implode(', ', $arguments) : '');
901+
return $return.sprintf("(%s)->%s(%s);\n", $class, $callable[1], $arguments ? implode(', ', $arguments) : '');
896902
}
897903

898-
return $return.sprintf("\\call_user_func(array(%s, '%s')%s);\n", $this->dumpValue($callable[0]), $callable[1], $arguments ? ', '.implode(', ', $arguments) : '');
904+
return $return.sprintf("\\call_user_func(array(%s, '%s')%s);\n", $class, $callable[1], $arguments ? ', '.implode(', ', $arguments) : '');
899905
}
900906

901907
return $return.sprintf("%s(%s);\n", $this->dumpLiteralClass($this->dumpValue($callable)), $arguments ? implode(', ', $arguments) : '');
@@ -957,6 +963,11 @@ public function __construct()
957963
958964
EOF;
959965
}
966+
if ($this->asFiles) {
967+
$code = str_replace('$parameters', "\$buildParameters;\n private \$parameters", $code);
968+
$code = str_replace('__construct()', '__construct(array $buildParameters = array())', $code);
969+
$code .= " \$this->buildParameters = \$buildParameters;\n";
970+
}
960971

961972
if ($this->container->isCompiled()) {
962973
if ($this->container->getParameterBag()->all()) {
@@ -1283,6 +1294,9 @@ private function addDefaultParametersMethod()
12831294
12841295
public function getParameter($name)
12851296
{
1297+
if (isset($this->buildParameters[$name])) {
1298+
return $this->buildParameters[$name];
1299+
}
12861300
if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
12871301
$name = $this->normalizeParameterName($name);
12881302
@@ -1299,6 +1313,9 @@ public function getParameter($name)
12991313
13001314
public function hasParameter($name)
13011315
{
1316+
if (isset($this->buildParameters[$name])) {
1317+
return true;
1318+
}
13021319
$name = $this->normalizeParameterName($name);
13031320
13041321
return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
@@ -1316,13 +1333,19 @@ public function getParameterBag()
13161333
foreach ($this->loadedDynamicParameters as $name => $loaded) {
13171334
$parameters[$name] = $loaded ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
13181335
}
1336+
foreach ($this->buildParameters as $name => $value) {
1337+
$parameters[$name] = $value;
1338+
}
13191339
$this->parameterBag = new FrozenParameterBag($parameters);
13201340
}
13211341
13221342
return $this->parameterBag;
13231343
}
13241344

13251345
EOF;
1346+
if (!$this->asFiles) {
1347+
$code = preg_replace('/^.*buildParameters.*\n.*\n.*\n/m', '', $code);
1348+
}
13261349

13271350
if ($dynamicPhp) {
13281351
$loadedDynamicParameters = $this->exportParameters(array_combine(array_keys($dynamicPhp), array_fill(0, count($dynamicPhp), false)), '', 8);
@@ -1717,16 +1740,21 @@ private function dumpValue($value, $interpolate = true)
17171740
throw new RuntimeException(sprintf('Cannot dump definition because of invalid factory method (%s)', $factory[1] ?: 'n/a'));
17181741
}
17191742

1743+
$class = $this->dumpValue($factory[0]);
17201744
if (is_string($factory[0])) {
1721-
return sprintf('%s::%s(%s)', $this->dumpLiteralClass($this->dumpValue($factory[0])), $factory[1], implode(', ', $arguments));
1745+
return sprintf('%s::%s(%s)', $this->dumpLiteralClass($class), $factory[1], implode(', ', $arguments));
17221746
}
17231747

17241748
if ($factory[0] instanceof Definition) {
1725-
return sprintf("\\call_user_func(array(%s, '%s')%s)", $this->dumpValue($factory[0]), $factory[1], count($arguments) > 0 ? ', '.implode(', ', $arguments) : '');
1749+
if (0 === strpos($class, 'new ')) {
1750+
return sprintf('(%s)->%s(%s)', $class, $factory[1], implode(', ', $arguments));
1751+
}
1752+
1753+
return sprintf("\\call_user_func(array(%s, '%s')%s)", $class, $factory[1], count($arguments) > 0 ? ', '.implode(', ', $arguments) : '');
17261754
}
17271755

17281756
if ($factory[0] instanceof Reference) {
1729-
return sprintf('%s->%s(%s)', $this->dumpValue($factory[0]), $factory[1], implode(', ', $arguments));
1757+
return sprintf('%s->%s(%s)', $class, $factory[1], implode(', ', $arguments));
17301758
}
17311759
}
17321760

‎Tests/Fixtures/php/services9_as_files.txt

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,15 +276,17 @@ use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
276276
*/
277277
class ProjectServiceContainer extends Container
278278
{
279+
private $buildParameters;
279280
private $parameters;
280281
private $targetDirs = array();
281282

282-
public function __construct()
283+
public function __construct(array $buildParameters = array())
283284
{
284285
$dir = $this->targetDirs[0] = \dirname(__DIR__);
285286
for ($i = 1; $i <= 5; ++$i) {
286287
$this->targetDirs[$i] = $dir = \dirname($dir);
287288
}
289+
$this->buildParameters = $buildParameters;
288290
$this->parameters = $this->getDefaultParameters();
289291

290292
$this->services = array();
@@ -382,6 +384,9 @@ class ProjectServiceContainer extends Container
382384

383385
public function getParameter($name)
384386
{
387+
if (isset($this->buildParameters[$name])) {
388+
return $this->buildParameters[$name];
389+
}
385390
if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
386391
$name = $this->normalizeParameterName($name);
387392

@@ -398,6 +403,9 @@ class ProjectServiceContainer extends Container
398403

399404
public function hasParameter($name)
400405
{
406+
if (isset($this->buildParameters[$name])) {
407+
return true;
408+
}
401409
$name = $this->normalizeParameterName($name);
402410

403411
return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
@@ -415,6 +423,9 @@ class ProjectServiceContainer extends Container
415423
foreach ($this->loadedDynamicParameters as $name => $loaded) {
416424
$parameters[$name] = $loaded ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
417425
}
426+
foreach ($this->buildParameters as $name => $value) {
427+
$parameters[$name] = $value;
428+
}
418429
$this->parameterBag = new FrozenParameterBag($parameters);
419430
}
420431

@@ -485,6 +496,10 @@ if (!\class_exists(ProjectServiceContainer::class, false)) {
485496
\class_alias(\Container%s\ProjectServiceContainer::class, ProjectServiceContainer::class, false);
486497
}
487498

488-
return new \Container%s\ProjectServiceContainer();
499+
return new \Container%s\ProjectServiceContainer(array(
500+
'container.build_hash' => '%s',
501+
'container.build_id' => '%s',
502+
'container.build_time' => %d,
503+
));
489504

490505
)

‎Tests/Fixtures/php/services_subscriber.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,15 @@ protected function getTestServiceSubscriberService()
8383
*/
8484
protected function getFooServiceService()
8585
{
86-
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber(\call_user_func(array(new \Symfony\Component\DependencyInjection\ServiceLocator(array('Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => function () {
86+
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber((new \Symfony\Component\DependencyInjection\ServiceLocator(array('Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => function () {
8787
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v = null) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition()) && false ?: '_'});
8888
}, 'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\TestServiceSubscriber' => function () {
8989
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber $v) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber()) && false ?: '_'});
9090
}, 'bar' => function () {
9191
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber()) && false ?: '_'});
9292
}, 'baz' => function () {
9393
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v = null) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition()) && false ?: '_'});
94-
})), 'withContext'), 'foo_service', $this));
94+
})))->withContext('foo_service', $this));
9595
}
9696

9797
/**

0 commit comments

Comments
 (0)