Skip to content

Commit ac7a54b

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '5.0'
* 5.0: [DI] Fix CheckTypeDeclarationPass [Security/Http] don't require the session to be started when tracking its id [DI] fix preloading script generation
2 parents fef50c9 + de4aab2 commit ac7a54b

File tree

9 files changed

+172
-17
lines changed

9 files changed

+172
-17
lines changed

‎Compiler/CheckTypeDeclarationsPass.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
1818
use Symfony\Component\DependencyInjection\Container;
1919
use Symfony\Component\DependencyInjection\Definition;
20-
use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException;
2120
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
2221
use Symfony\Component\DependencyInjection\Exception\InvalidParameterTypeException;
2322
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
@@ -207,7 +206,7 @@ private function checkType(Definition $checkedDefinition, $value, \ReflectionPar
207206
if ('' === preg_replace('/'.$envPlaceholderUniquePrefix.'_\w+_[a-f0-9]{32}/U', '', $value, -1, $c) && 1 === $c) {
208207
try {
209208
$value = $this->container->resolveEnvPlaceholders($value, true);
210-
} catch (EnvNotFoundException | RuntimeException $e) {
209+
} catch (\Exception $e) {
211210
// If an env placeholder cannot be resolved, we skip the validation.
212211
return;
213212
}
@@ -250,7 +249,11 @@ private function checkType(Definition $checkedDefinition, $value, \ReflectionPar
250249
return;
251250
}
252251

253-
if ('iterable' === $type && (\is_array($value) || is_subclass_of($class, \Traversable::class))) {
252+
if ('iterable' === $type && (\is_array($value) || 'array' === $class || is_subclass_of($class, \Traversable::class))) {
253+
return;
254+
}
255+
256+
if ($type === $class) {
254257
return;
255258
}
256259

‎Container.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\DependencyInjection;
1313

14+
use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
15+
use Symfony\Component\DependencyInjection\Argument\ServiceLocator as ArgumentServiceLocator;
1416
use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException;
1517
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1618
use Symfony\Component\DependencyInjection\Exception\ParameterCircularReferenceException;
@@ -22,6 +24,10 @@
2224
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
2325
use Symfony\Contracts\Service\ResetInterface;
2426

27+
// Help opcache.preload discover always-needed symbols
28+
class_exists(RewindableGenerator::class);
29+
class_exists(ArgumentServiceLocator::class);
30+
2531
/**
2632
* Container is a dependency injection container.
2733
*

‎Dumper/PhpDumper.php

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ class PhpDumper extends Dumper
8383
private $inlinedRequires = [];
8484
private $circularReferences = [];
8585
private $singleUsePrivateIds = [];
86+
private $preload = [];
8687
private $addThrow = false;
8788
private $addGetService = false;
8889
private $locatedIds = [];
@@ -144,6 +145,7 @@ public function dump(array $options = [])
144145
'hot_path_tag' => 'container.hot_path',
145146
'inline_factories_parameter' => 'container.dumper.inline_factories',
146147
'inline_class_loader_parameter' => 'container.dumper.inline_class_loader',
148+
'preload_classes' => [],
147149
'service_locator_tag' => 'container.service_locator',
148150
'build_time' => time(),
149151
], $options);
@@ -228,8 +230,12 @@ public function dump(array $options = [])
228230

229231
$proxyClasses = $this->inlineFactories ? $this->generateProxyClasses() : null;
230232

233+
if ($options['preload_classes']) {
234+
$this->preload = array_combine($options['preload_classes'], $options['preload_classes']);
235+
}
236+
231237
$code =
232-
$this->startClass($options['class'], $baseClass, $preload).
238+
$this->startClass($options['class'], $baseClass).
233239
$this->addServices($services).
234240
$this->addDeprecatedAliases().
235241
$this->addDefaultParametersMethod()
@@ -304,7 +310,7 @@ public function dump(array $options = [])
304310
$id = hash('crc32', $hash.$time);
305311
$this->asFiles = false;
306312

307-
if ($preload && null !== $autoloadFile = $this->getAutoloadFile()) {
313+
if ($this->preload && null !== $autoloadFile = $this->getAutoloadFile()) {
308314
$autoloadFile = substr($this->export($autoloadFile), 2, -1);
309315

310316
$code[$options['class'].'.preload.php'] = <<<EOF
@@ -322,8 +328,13 @@ public function dump(array $options = [])
322328
323329
EOF;
324330

325-
foreach ($preload as $class) {
326-
$code[$options['class'].'.preload.php'] .= sprintf("\$classes[] = '%s';\n", $class);
331+
foreach ($this->preload as $class) {
332+
if (!$class || false !== strpos($class, '$')) {
333+
continue;
334+
}
335+
if (!(class_exists($class, false) || interface_exists($class, false) || trait_exists($class, false)) || (new \ReflectionClass($class))->isUserDefined()) {
336+
$code[$options['class'].'.preload.php'] .= sprintf("\$classes[] = '%s';\n", $class);
337+
}
327338
}
328339

329340
$code[$options['class'].'.preload.php'] .= <<<'EOF'
@@ -369,6 +380,7 @@ public function dump(array $options = [])
369380
$this->circularReferences = [];
370381
$this->locatedIds = [];
371382
$this->exportedVariables = [];
383+
$this->preload = [];
372384

373385
$unusedEnvs = [];
374386
foreach ($this->container->getEnvCounters() as $env => $use) {
@@ -547,8 +559,10 @@ private function addServiceInclude(string $cId, Definition $definition): string
547559
if ($this->inlineRequires && (!$this->isHotPath($definition) || $this->getProxyDumper()->isProxyCandidate($definition))) {
548560
$lineage = [];
549561
foreach ($this->inlinedDefinitions as $def) {
550-
if (!$def->isDeprecated() && \is_string($class = \is_array($factory = $def->getFactory()) && \is_string($factory[0]) ? $factory[0] : $def->getClass())) {
551-
$this->collectLineage($class, $lineage);
562+
if (!$def->isDeprecated()) {
563+
foreach ($this->getClasses($def) as $class) {
564+
$this->collectLineage($class, $lineage);
565+
}
552566
}
553567
}
554568

@@ -557,9 +571,10 @@ private function addServiceInclude(string $cId, Definition $definition): string
557571
&& ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE !== $behavior
558572
&& $this->container->has($id)
559573
&& $this->isTrivialInstance($def = $this->container->findDefinition($id))
560-
&& \is_string($class = \is_array($factory = $def->getFactory()) && \is_string($factory[0]) ? $factory[0] : $def->getClass())
561574
) {
562-
$this->collectLineage($class, $lineage);
575+
foreach ($this->getClasses($def) as $class) {
576+
$this->collectLineage($class, $lineage);
577+
}
563578
}
564579
}
565580

@@ -809,6 +824,12 @@ protected function {$methodName}($lazyInitialization)
809824

810825
if ($definition->isDeprecated()) {
811826
$code .= sprintf(" trigger_deprecation('', '', %s);\n\n", $this->export($definition->getDeprecationMessage($id)));
827+
} else {
828+
foreach ($this->inlinedDefinitions as $def) {
829+
foreach ($this->getClasses($def) as $class) {
830+
$this->preload[$class] = $class;
831+
}
832+
}
812833
}
813834

814835
if ($this->getProxyDumper()->isProxyCandidate($definition)) {
@@ -965,7 +986,15 @@ private function addServices(array &$services = null): string
965986
$definitions = $this->container->getDefinitions();
966987
ksort($definitions);
967988
foreach ($definitions as $id => $definition) {
968-
$services[$id] = $definition->isSynthetic() ? null : $this->addService($id, $definition);
989+
if (!$definition->isSynthetic()) {
990+
$services[$id] = $this->addService($id, $definition);
991+
} else {
992+
$services[$id] = null;
993+
994+
foreach ($this->getClasses($definition) as $class) {
995+
$this->preload[$class] = $class;
996+
}
997+
}
969998
}
970999

9711000
foreach ($definitions as $id => $definition) {
@@ -1066,7 +1095,7 @@ private function addNewInstance(Definition $definition, string $return = '', str
10661095
return $return.sprintf('new %s(%s)', $this->dumpLiteralClass($this->dumpValue($class)), implode(', ', $arguments)).$tail;
10671096
}
10681097

1069-
private function startClass(string $class, string $baseClass, ?array &$preload): string
1098+
private function startClass(string $class, string $baseClass): string
10701099
{
10711100
$namespaceLine = !$this->asFiles && $this->namespace ? "\nnamespace {$this->namespace};\n" : '';
10721101

@@ -1129,7 +1158,7 @@ public function __construct()
11291158
$code .= $this->addMethodMap();
11301159
$code .= $this->asFiles && !$this->inlineFactories ? $this->addFileMap() : '';
11311160
$code .= $this->addAliases();
1132-
$code .= $this->addInlineRequires($preload);
1161+
$code .= $this->addInlineRequires();
11331162
$code .= <<<EOF
11341163
}
11351164
@@ -1329,7 +1358,7 @@ protected function {$methodNameAlias}()
13291358
return $code;
13301359
}
13311360

1332-
private function addInlineRequires(?array &$preload): string
1361+
private function addInlineRequires(): string
13331362
{
13341363
if (!$this->hotPathTag || !$this->inlineRequires) {
13351364
return '';
@@ -1347,8 +1376,7 @@ private function addInlineRequires(?array &$preload): string
13471376
$inlinedDefinitions = $this->getDefinitionsFromArguments([$definition]);
13481377

13491378
foreach ($inlinedDefinitions as $def) {
1350-
if (\is_string($class = \is_array($factory = $def->getFactory()) && \is_string($factory[0]) ? $factory[0] : $def->getClass())) {
1351-
$preload[$class] = $class;
1379+
foreach ($this->getClasses($def) as $class) {
13521380
$this->collectLineage($class, $lineage);
13531381
}
13541382
}
@@ -2070,4 +2098,29 @@ private function getAutoloadFile(): ?string
20702098

20712099
return null;
20722100
}
2101+
2102+
private function getClasses(Definition $definition): array
2103+
{
2104+
$classes = [];
2105+
2106+
while ($definition instanceof Definition) {
2107+
$classes[] = trim($definition->getClass(), '\\');
2108+
$factory = $definition->getFactory();
2109+
2110+
if (!\is_array($factory)) {
2111+
$factory = [$factory];
2112+
}
2113+
2114+
if (\is_string($factory[0])) {
2115+
if (false !== $i = strrpos($factory[0], '::')) {
2116+
$factory[0] = substr($factory[0], 0, $i);
2117+
}
2118+
$classes[] = trim($factory[0], '\\');
2119+
}
2120+
2121+
$definition = $factory[0];
2122+
}
2123+
2124+
return array_filter($classes);
2125+
}
20732126
}

‎Tests/Compiler/CheckTypeDeclarationsPassTest.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,42 @@ public function testProcessFactoryDoesNotLoadCodeByDefault()
536536
$this->addToAssertionCount(1);
537537
}
538538

539+
public function testProcessFactoryForTypeSameAsClass()
540+
{
541+
$container = new ContainerBuilder();
542+
543+
$container->register('foo', Foo::class);
544+
$container->register('bar', 'callable')
545+
->setFactory([
546+
new Reference('foo'),
547+
'createCallable',
548+
]);
549+
$container->register('bar_call', BarMethodCall::class)
550+
->addMethodCall('setCallable', [new Reference('bar')]);
551+
552+
(new CheckTypeDeclarationsPass(true))->process($container);
553+
554+
$this->addToAssertionCount(1);
555+
}
556+
557+
public function testProcessFactoryForIterableTypeAndArrayClass()
558+
{
559+
$container = new ContainerBuilder();
560+
561+
$container->register('foo', Foo::class);
562+
$container->register('bar', 'array')
563+
->setFactory([
564+
new Reference('foo'),
565+
'createArray',
566+
]);
567+
$container->register('bar_call', BarMethodCall::class)
568+
->addMethodCall('setIterable', [new Reference('bar')]);
569+
570+
(new CheckTypeDeclarationsPass(true))->process($container);
571+
572+
$this->addToAssertionCount(1);
573+
}
574+
539575
public function testProcessPassingBuiltinTypeDoesNotLoadCodeByDefault()
540576
{
541577
$container = new ContainerBuilder();

‎Tests/Fixtures/CheckTypeDeclarationsPass/Foo.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,14 @@ public static function createBarArguments(\stdClass $stdClass, \stdClass $stdCla
1313
{
1414
return new Bar($stdClass);
1515
}
16+
17+
public static function createCallable(): callable
18+
{
19+
return function() {};
20+
}
21+
22+
public static function createArray(): array
23+
{
24+
return [];
25+
}
1626
}

‎Tests/Fixtures/php/services9_as_files.txt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,24 @@ class ProjectServiceContainer extends Container
529529
}
530530
}
531531

532+
[ProjectServiceContainer.preload.php] => <?php
533+
%A
534+
535+
$classes = [];
536+
$classes[] = 'Bar\FooClass';
537+
$classes[] = 'Baz';
538+
$classes[] = 'ConfClass';
539+
$classes[] = 'Bar';
540+
$classes[] = 'BazClass';
541+
$classes[] = 'Foo';
542+
$classes[] = 'LazyContext';
543+
$classes[] = 'FooBarBaz';
544+
$classes[] = 'FactoryClass';
545+
$classes[] = 'Request';
546+
$classes[] = 'Symfony\Component\DependencyInjection\ContainerInterface';
547+
548+
%A
549+
532550
[ProjectServiceContainer.php] => <?php
533551

534552
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.

‎Tests/Fixtures/php/services9_inlined_factories.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,16 @@ class ProjectServiceContainer extends Container
534534

535535
$classes = [];
536536
$classes[] = 'Bar\FooClass';
537+
$classes[] = 'Baz';
538+
$classes[] = 'ConfClass';
539+
$classes[] = 'Bar';
540+
$classes[] = 'BazClass';
541+
$classes[] = 'Foo';
542+
$classes[] = 'LazyContext';
543+
$classes[] = 'FooBarBaz';
544+
$classes[] = 'FactoryClass';
545+
$classes[] = 'Request';
546+
$classes[] = 'Symfony\Component\DependencyInjection\ContainerInterface';
537547

538548
%A
539549

‎Tests/Fixtures/php/services9_lazy_inlined_factories.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,16 @@ class FooClass_%s extends \Bar\FooClass implements \ProxyManager\Proxy\VirtualPr
163163
%A
164164
}
165165

166+
[ProjectServiceContainer.preload.php] => <?php
167+
%A
168+
169+
$classes = [];
170+
$classes[] = 'Bar\FooClass';
171+
$classes[] = 'Bar\FooLazyClass';
172+
$classes[] = 'Symfony\Component\DependencyInjection\ContainerInterface';
173+
174+
%A
175+
166176
[ProjectServiceContainer.php] => <?php
167177

168178
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.

‎Tests/Fixtures/php/services_non_shared_lazy_as_files.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,15 @@ class ProjectServiceContainer extends Container
8383
}
8484
}
8585

86+
[ProjectServiceContainer.preload.php] => <?php
87+
%A
88+
89+
$classes = [];
90+
$classes[] = 'Bar\FooLazyClass';
91+
$classes[] = 'Symfony\Component\DependencyInjection\ContainerInterface';
92+
93+
%A
94+
8695
[ProjectServiceContainer.php] => <?php
8796

8897
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.

0 commit comments

Comments
 (0)