Skip to content

Commit 8c859eb

Browse files
committed
Merge branch '5.4' into 6.0
* 5.4: Add missing files from 5.3 Remove unneeded files Tweak bug report template Prevent infinite nesting of lazy `ObjectManager` instances when `ObjectManager` is reset [DependencyInjection] Skip parameter attribute configurators in AttributeAutoconfigurationPass if we can't get the constructor reflector [HttpKernel] fix sending Vary: Accept-Language when appropriate
2 parents 5c4e684 + e9fca97 commit 8c859eb

File tree

9 files changed

+138
-44
lines changed

9 files changed

+138
-44
lines changed

.github/ISSUE_TEMPLATE/1_Bug_report.md

Lines changed: 0 additions & 22 deletions
This file was deleted.

.github/ISSUE_TEMPLATE/1_Bug_report.yaml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,29 @@ body:
1414
id: description
1515
attributes:
1616
label: Description
17-
description: A clear and consise description of the problem
17+
description: A clear and concise description of the problem
1818
validations:
1919
required: true
2020
- type: textarea
2121
id: how-to-reproduce
2222
attributes:
2323
label: How to reproduce
2424
description: |
25-
Code and/or config needed to reproduce the problem.
26-
If it's a complex bug, create a "bug reproducer" as explained in https://symfony.com/doc/current/contributing/code/reproducer.html
25+
⚠️ This is the most important part of the report ⚠️
26+
Without a way to easily reproduce your issue, there is little chance we will be able to help you and work on a fix.
27+
Please, take the time to show us some code and/or config that is needed for others to reproduce the problem easily.
28+
Most of the time, creating a "bug reproducer" as explained in the URL below is the best way to help us
29+
and increases the chances someone will have a look at it:
30+
https://symfony.com/doc/current/contributing/code/reproducer.html
2731
validations:
2832
required: true
2933
- type: textarea
3034
id: possible-solution
3135
attributes:
3236
label: Possible Solution
33-
description: "Optional: only if you have suggestions on a fix/reason for the bug"
37+
description: |
38+
Optional: only if you have suggestions on a fix/reason for the bug
39+
Don't hesitate to create a pull request with your solution, it helps get faster feedback.
3440
- type: textarea
3541
id: additional-context
3642
attributes:

.github/ISSUE_TEMPLATE/2_Feature_request.md

Lines changed: 0 additions & 12 deletions
This file was deleted.

src/Symfony/Bridge/Doctrine/ManagerRegistry.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ function (&$wrappedInstance, LazyLoadingInterface $manager) use ($name) {
5555
$name = $this->aliases[$name];
5656
}
5757
if (isset($this->fileMap[$name])) {
58-
$wrappedInstance = $this->load($this->fileMap[$name]);
58+
$wrappedInstance = $this->load($this->fileMap[$name], false);
5959
} else {
6060
$wrappedInstance = $this->{$this->methodMap[$name]}(false);
6161
}

src/Symfony/Bridge/Doctrine/Tests/ManagerRegistryTest.php

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,15 @@
1212
namespace Symfony\Bridge\Doctrine\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use ProxyManager\Proxy\LazyLoadingInterface;
16+
use ProxyManager\Proxy\ValueHolderInterface;
1517
use Symfony\Bridge\Doctrine\ManagerRegistry;
18+
use Symfony\Bridge\ProxyManager\LazyProxy\PhpDumper\ProxyDumper;
1619
use Symfony\Bridge\ProxyManager\Tests\LazyProxy\Dumper\PhpDumperTest;
20+
use Symfony\Component\DependencyInjection\ContainerBuilder;
21+
use Symfony\Component\DependencyInjection\ContainerInterface;
22+
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
23+
use Symfony\Component\Filesystem\Filesystem;
1724

1825
class ManagerRegistryTest extends TestCase
1926
{
@@ -39,6 +46,92 @@ public function testResetService()
3946
$this->assertSame($foo, $container->get('foo'));
4047
$this->assertObjectNotHasAttribute('bar', $foo);
4148
}
49+
50+
/**
51+
* When performing an entity manager lazy service reset, the reset operations may re-use the container
52+
* to create a "fresh" service: when doing so, it can happen that the "fresh" service is itself a proxy.
53+
*
54+
* Because of that, the proxy will be populated with a wrapped value that is itself a proxy: repeating
55+
* the reset operation keeps increasing this nesting until the application eventually runs into stack
56+
* overflow or memory overflow operations, which can happen for long-running processes that rely on
57+
* services that are reset very often.
58+
*/
59+
public function testResetServiceWillNotNestFurtherLazyServicesWithinEachOther()
60+
{
61+
// This test scenario only applies to containers composed as a set of generated sources
62+
$this->dumpLazyServiceProjectAsFilesServiceContainer();
63+
64+
/** @var ContainerInterface $container */
65+
$container = new \LazyServiceProjectAsFilesServiceContainer();
66+
67+
$registry = new TestManagerRegistry(
68+
'irrelevant',
69+
[],
70+
['defaultManager' => 'foo'],
71+
'irrelevant',
72+
'defaultManager',
73+
'irrelevant'
74+
);
75+
$registry->setTestContainer($container);
76+
77+
$service = $container->get('foo');
78+
79+
self::assertInstanceOf(\stdClass::class, $service);
80+
self::assertInstanceOf(LazyLoadingInterface::class, $service);
81+
self::assertInstanceOf(ValueHolderInterface::class, $service);
82+
self::assertFalse($service->isProxyInitialized());
83+
84+
$service->initializeProxy();
85+
86+
self::assertTrue($container->initialized('foo'));
87+
self::assertTrue($service->isProxyInitialized());
88+
89+
$registry->resetManager();
90+
$service->initializeProxy();
91+
92+
$wrappedValue = $service->getWrappedValueHolderValue();
93+
self::assertInstanceOf(\stdClass::class, $wrappedValue);
94+
self::assertNotInstanceOf(LazyLoadingInterface::class, $wrappedValue);
95+
self::assertNotInstanceOf(ValueHolderInterface::class, $wrappedValue);
96+
}
97+
98+
/** @return void */
99+
private function dumpLazyServiceProjectAsFilesServiceContainer()
100+
{
101+
if (class_exists(\LazyServiceProjectAsFilesServiceContainer::class, false)) {
102+
return;
103+
}
104+
105+
$container = new ContainerBuilder();
106+
107+
$container->register('foo', \stdClass::class)
108+
->setPublic(true)
109+
->setLazy(true);
110+
$container->compile();
111+
112+
$fileSystem = new Filesystem();
113+
114+
$temporaryPath = $fileSystem->tempnam(sys_get_temp_dir(), 'symfonyManagerRegistryTest');
115+
$fileSystem->remove($temporaryPath);
116+
$fileSystem->mkdir($temporaryPath);
117+
118+
$dumper = new PhpDumper($container);
119+
120+
$dumper->setProxyDumper(new ProxyDumper());
121+
$containerFiles = $dumper->dump([
122+
'class' => 'LazyServiceProjectAsFilesServiceContainer',
123+
'as_files' => true,
124+
]);
125+
126+
array_walk(
127+
$containerFiles,
128+
static function (string $containerSources, string $fileName) use ($temporaryPath): void {
129+
(new Filesystem())->dumpFile($temporaryPath.'/'.$fileName, $containerSources);
130+
}
131+
);
132+
133+
require $temporaryPath.'/LazyServiceProjectAsFilesServiceContainer.php';
134+
}
42135
}
43136

44137
class TestManagerRegistry extends ManagerRegistry

src/Symfony/Component/DependencyInjection/Compiler/AttributeAutoconfigurationPass.php

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\DependencyInjection\ContainerBuilder;
1616
use Symfony\Component\DependencyInjection\Definition;
1717
use Symfony\Component\DependencyInjection\Exception\LogicException;
18+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
1819

1920
/**
2021
* @author Alexander M. Turek <[email protected]>
@@ -99,11 +100,19 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
99100
}
100101
}
101102

102-
if ($this->parameterAttributeConfigurators && $constructorReflector = $this->getConstructor($value, false)) {
103-
foreach ($constructorReflector->getParameters() as $parameterReflector) {
104-
foreach ($parameterReflector->getAttributes() as $attribute) {
105-
if ($configurator = $this->parameterAttributeConfigurators[$attribute->getName()] ?? null) {
106-
$configurator($conditionals, $attribute->newInstance(), $parameterReflector);
103+
if ($this->parameterAttributeConfigurators) {
104+
try {
105+
$constructorReflector = $this->getConstructor($value, false);
106+
} catch (RuntimeException $e) {
107+
$constructorReflector = null;
108+
}
109+
110+
if ($constructorReflector) {
111+
foreach ($constructorReflector->getParameters() as $parameterReflector) {
112+
foreach ($parameterReflector->getAttributes() as $attribute) {
113+
if ($configurator = $this->parameterAttributeConfigurators[$attribute->getName()] ?? null) {
114+
$configurator($conditionals, $attribute->newInstance(), $parameterReflector);
115+
}
107116
}
108117
}
109118
}

src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,11 @@ static function (ChildDefinition $definition, CustomAnyAttribute $attribute, \Re
932932
->setPublic(true)
933933
->setAutoconfigured(true);
934934

935+
$container->register('failing_factory', \stdClass::class);
936+
$container->register('ccc', TaggedService4::class)
937+
->setFactory([new Reference('failing_factory'), 'create'])
938+
->setAutoconfigured(true);
939+
935940
$collector = new TagCollector();
936941
$container->addCompilerPass($collector);
937942

@@ -952,6 +957,17 @@ static function (ChildDefinition $definition, CustomAnyAttribute $attribute, \Re
952957
['property' => 'name'],
953958
['someAttribute' => 'on name', 'priority' => 0, 'property' => 'name'],
954959
],
960+
'ccc' => [
961+
['class' => TaggedService4::class],
962+
['method' => 'fooAction'],
963+
['someAttribute' => 'on fooAction', 'priority' => 0, 'method' => 'fooAction'],
964+
['parameter' => 'param1'],
965+
['someAttribute' => 'on param1 in fooAction', 'priority' => 0, 'parameter' => 'param1'],
966+
['method' => 'barAction'],
967+
['someAttribute' => 'on barAction', 'priority' => 0, 'method' => 'barAction'],
968+
['property' => 'name'],
969+
['someAttribute' => 'on name', 'priority' => 0, 'property' => 'name'],
970+
],
955971
], $collector->collectedTags);
956972
}
957973

src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ private function setLocale(Request $request)
7070
$request->setLocale($locale);
7171
} elseif ($this->useAcceptLanguageHeader && $this->enabledLocales && ($preferredLanguage = $request->getPreferredLanguage($this->enabledLocales))) {
7272
$request->setLocale($preferredLanguage);
73+
$request->attributes->set('_vary_by_language', true);
7374
}
7475
}
7576

src/Symfony/Component/HttpKernel/EventListener/ResponseListener.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ public function onKernelResponse(ResponseEvent $event)
5050

5151
if ($this->addContentLanguageHeader && !$response->isInformational() && !$response->isEmpty() && !$response->headers->has('Content-Language')) {
5252
$response->headers->set('Content-Language', $event->getRequest()->getLocale());
53+
}
54+
55+
if ($event->getRequest()->attributes->get('_vary_by_language')) {
5356
$response->setVary('Accept-Language', false);
5457
}
5558

0 commit comments

Comments
 (0)