8000 Merge branch '2.3' · symfony/symfony@5b71e61 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5b71e61

Browse files
fabpotfabpot
committed
Merge branch '2.3'
* 2.3: [HttpKernel] added a check for private event listeners/subscribers [FrameworkBundle] fixed registration of the register listener pass [Form] Fixed regression causing invalid "WHERE id IN ()" statements [DependencyInjection] fixed a non-detected circular reference in PhpDumper (closes #8425) [Form] Fixed regression in BooleanToStringTransformer from ed83752 [FrameworkBundle] removed obsolete code [Process] Close unix pipes before calling `proc_close` to avoid a deadlock [Process] Fix process merge in 2.3 [Intl] made RegionBundle and LanguageBundle merge fallback data when using a country-specific locale
2 parents 035a6e4 + 0f78175 commit 5b71e61

File tree

16 files changed

+168
-39
lines changed

16 files changed

+168
-39
lines changed

‎src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,16 @@ public function getRemainingViews()
204204
*/
205205
public function getChoicesForValues(array $values)
206206
{
207+
// Performance optimization
208+
// Also prevents the generation of "WHERE id IN ()" queries through the
209+
// entity loader. At least with MySQL and on the development machine
210+
// this was tested on, no exception was thrown for such invalid
211+
// statements, consequently no test fails when this code is removed.
212+
// https://github.com/symfony/symfony/pull/8981#issuecomment-24230557
213+
if (empty($values)) {
214+
return array();
215+
}
216+
207217
if (!$this->loaded) {
208218
// Optimize performance in case we have an entity loader and
209219
// a single-field identifier
@@ -247,6 +257,11 @@ public function getChoicesForValues(array $values)
247257
*/
248258
public function getValuesForChoices(array $entities)
249259
{
260+
// Performance optimization
261+
if (empty($entities)) {
262+
return array();
263+
}
264+
250265
if (!$this->loaded) {
251266
// Optimize performance for single-field identifiers. We already
252267
// know that the IDs are used as values
@@ -284,6 +299,11 @@ public function getValuesForChoices(array $entities)
284299
*/
285300
public function getIndicesForChoices(array $entities)
286301
{
302+
// Performance optimization
303+
if (empty($entities)) {
304+
return array();
305+
}
306+
287307
if (!$this->loaded) {
288308
// Optimize performance for single-field identifiers. We already
289309
// know that the IDs are used as indices
@@ -321,6 +341,11 @@ public function getIndicesForChoices(array $entities)
321341
*/
322342
public function getIndicesForValues(array $values)
323343
{
344+
// Performance optimization
345+
if (empty($values)) {
346+
return array();
347+
}
348+
324349
if (!$this->loaded) {
325350
// Optimize performance for single-field identifiers.
326351

‎src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ public function build(ContainerBuilder $container)
6565

6666
$container->addCompilerPass(new RoutingResolverPass());
6767
$container->addCompilerPass(new ProfilerPass());
68-
$container->addCompilerPass(new RegisterListenersPass(), PassConfig::TYPE_AFTER_REMOVING);
68+
// must be registered before removing private services as some might be listeners/subscribers
69+
// but as late as possible to get resolved parameters
70+
$container->addCompilerPass(new RegisterListenersPass(), PassConfig::TYPE_BEFORE_REMOVING);
6971
$container->addCompilerPass(new TemplatingPass());
7072
$container->addCompilerPass(new AddConstraintValidatorsPass());
7173
$container->addCompilerPass(new AddValidatorInitializersPass());

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -283,13 +283,12 @@ public function testValidationPaths()
283283
protected function createContainer(array $data = array())
284284
{
285285
return new ContainerBuilder(new ParameterBag(array_merge(array(
286-
'kernel.bundles' => array('FrameworkBundle' => 'Symfony\\Bundle\\FrameworkBundle\\FrameworkBundle'),
287-
'kernel.cache_dir' => __DIR__,
288-
'kernel.compiled_classes' => array(),
289-
'kernel.debug' => false,
290-
'kernel.environment' => 'test',
291-
'kernel.name' => 'kernel',
292-
'kernel.root_dir' => __DIR__,
286+
'kernel.bundles' => array('FrameworkBundle' => 'Symfony\\Bundle\\FrameworkBundle\\FrameworkBundle'),
287+
'kernel.cache_dir' => __DIR__,
288+
'kernel.debug' => false,
289+
'kernel.environment' => 'test',
290+
'kernel.name' => 'kernel',
291+
'kernel.root_dir' => __DIR__,
293292
), $data)));
294293
}
295294

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,12 @@ private function addServiceInlinedDefinitionsSetup($id, $definition)
443443
continue;
444444
}
445445

446+
// if the instance is simple, the return statement has already been generated
447+
// so, the only possible way to get there is because of a circular reference
448+
if ($this->isSimpleInstance($id, $definition)) {
449+
throw new ServiceCircularReferenceException($id, array($id));
450+
}
451+
446452
$name = (string) $this->definitionVariables->offsetGet($iDefinition);
447453
$code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
448454
$code .= $this->addServiceProperties(null, $iDefinition, $name);

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,4 +181,19 @@ public function testOverrideServiceWhenUsingADumpedContainerAndServiceIsUsedFrom
181181

182182
$this->assertSame($bar, $container->get('foo')->bar, '->set() overrides an already defined service');
183183
}
184+
185+
/**
186+
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
187+
*/
188+
public function testCircularReference()
189+
{
190+
$container = new ContainerBuilder();
191+
$container->register('foo', 'stdClass')->addArgument(new Reference('bar'));
192+
$container->register('bar', 'stdClass')->setPublic(false)->addMethodCall('setA', array(new Reference('baz')));
193+
$container->register('baz', 'stdClass')->addMethodCall('setA', array(new Reference('foo')));
194+
$container->compile();
195+
196+
$dumper = new PhpDumper($container);
197+
$dumper->dump();
198+
}
184199
}

‎src/Symfony/Component/Form/Extension/Core/DataTransformer/BooleanToStringTransformer.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ public function __construct($trueValue)
4949
*/
5050
public function transform($value)
5151
{
52+
if (null === $value) {
53+
return null;
54+
}
55+
5256
if (!is_bool($value)) {
5357
throw new TransformationFailedException('Expected a Boolean.');
5458
}

‎src/Symfony/Component/Form/Tests/Extension/Core/DataTransformer/BooleanToStringTransformerTest.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,10 @@ public function testTransform()
3838
$this->assertNull($this->transformer->transform(false));
3939
}
4040

41-
/**
42-
* @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
43-
*/
44-
public function testTransformFailsIfNull()
41+
// https://github.com/symfony/symfony/issues/8989
42+
public function testTransformAcceptsNull()
4543
{
46-
$this->transformer->transform(null);
44+
$this->assertNull($this->transformer->transform(null));
4745
}
4846

4947
/**

‎src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ public function process(ContainerBuilder $container)
5757
$definition = $container->getDefinition($this->dispatcherService);
5858

5959
foreach ($container->findTaggedServiceIds($this->listenerTag) as $id => $events) {
60+
$def = $container->getDefinition($id);
61+
if (!$def->isPublic()) {
62+
throw new \InvalidArgumentException(sprintf('The service "%s" must be public as event listeners are lazy-loaded.', $id));
63+
}
64+
6065
foreach ($events as $event) {
6166
$priority = isset($event['priority']) ? $event['priority'] : 0;
6267

@@ -77,8 +82,13 @@ public function process(ContainerBuilder $container)
7782
}
7883

7984
foreach ($container->findTaggedServiceIds($this->subscriberTag) as $id => $attributes) {
85+
$def = $container->getDefinition($id);
86+
if (!$def->isPublic()) {
87+
throw new \InvalidArgumentException(sprintf('The service "%s" must be public as event subscribers are lazy-loaded.', $id));
88+
}
89+
8090
// We must assume that the class value has been correctly filled, even if the service is created by a factory
81-
$class = $container->getDefinition($id)->getClass();
91+
$class = $def->getClass();
8292

8393
$refClass = new \ReflectionClass($class);
8494
$interface = 'Symfony\Component\EventDispatcher\EventSubscriberInterface';

‎src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public function testEventSubscriberWithoutInterface()
3131
);
3232

3333
$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
34+
$definition->expects($this->atLeastOnce())
35+
->method('isPublic')
36+
->will($this->returnValue(true));
3437
$definition->expects($this->atLeastOnce())
3538
->method('getClass')
3639
->will($this->returnValue('stdClass'));
@@ -60,6 +63,9 @@ public function testValidEventSubscriber()
6063
);
6164

6265
$definition = $this->getMock('Symfony\Component\DependencyInjection\Definition');
66+
$definition->expects($this->atLeastOnce())
67+
->method('isPublic')
68+
->will($this->returnValue(true));
6369
$definition->expects($this->atLeastOnce())
6470
->method('getClass')
6571
->will($this->returnValue('Symfony\Component\HttpKernel\Tests\DependencyInjection\SubscriberService'));
@@ -81,6 +87,34 @@ public function testValidEventSubscriber()
8187
$registerListenersPass = new RegisterListenersPass();
8288
$registerListenersPass->process($builder);
8389
}
90+
91+
/**
92+
* @expectedException \InvalidArgumentException
93+
* @expectedExceptionMessage The service "foo" must be public as event listeners are lazy-loaded.
94+
*/
95+
public function testPrivateEventListener()
96+
{
97+
$container = new ContainerBuilder();
98+
$container->register('foo', 'stdClass')->setPublic(false)->addTag('kernel.event_listener', array());
99+
$container->register('event_dispatcher', 'stdClass');
100+
101+
$registerListenersPass = new RegisterListenersPass();
102+
$registerListenersPass->process($container);
103+
}
104+
105+
/**
106+
* @expectedException \InvalidArgumentException
107+
* @expectedExceptionMessage The service "foo" must be public as event subscribers are lazy-loaded.
108+
*/
109+
public function testPrivateEventSubscriber()
110+
{
111+
$container = new ContainerBuilder();
112+
$container->register('foo', 'stdClass')->setPublic(false)->addTag('kernel.event_subscriber', array());
113+
$container->register('event_dispatcher', 'stdClass');
114+
115+
$registerListenersPass = new RegisterListenersPass();
116+
$registerListenersPass->process($container);
117+
}
84118
}
85119

86120
class SubscriberService implements \Symfony\Component\EventDispatcher\EventSubscriberInterface

‎src/Symfony/Component/Intl/ResourceBundle/LanguageBundle.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function getLanguageName($lang, $region = null, $locale = null)
2727
$locale = \Locale::getDefault();
2828
}
2929

30-
if (null === ($languages = $this->readEntry($locale, array('Languages')))) {
30+
if (null === ($languages = $this->readEntry($locale, array('Languages'), true))) {
3131
return null;
3232
}
3333

@@ -49,7 +49,7 @@ public function getLanguageNames($locale = null)
4949
$locale = \Locale::getDefault();
5050
}
5151

52-
if (null === ($languages = $this->readEntry($locale, array('Languages')))) {
52+
if (null === ($languages = $this->readEntry($locale, array('Languages'), true))) {
5353
return array();
5454
}
5555

@@ -102,7 +102,7 @@ public function getScriptNames($locale = null)
102102
$locale = \Locale::getDefault();
103103
}
104104

105-
if (null === ($scripts = $this->readEntry($locale, array('Scripts')))) {
105+
if (null === ($scripts = $this->readEntry($locale, array('Scripts'), true))) {
106106
return array();
107107
}
108108

‎src/Symfony/Component/Intl/ResourceBundle/RegionBundle.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function getCountryName($country, $locale = null)
2727
$locale = \Locale::getDefault();
2828
}
2929

30-
return $this->readEntry($locale, array('Countries', $country));
30+
return $this->readEntry($locale, array('Countries', $country), true);
3131
}
3232

3333
/**
@@ -39,7 +39,7 @@ public function getCountryNames($locale = null)
3939
$locale = \Locale::getDefault();
4040
}
4141

42-
if (null === ($countries = $this->readEntry($locale, array('Countries')))) {
42+
if (null === ($countries = $this->readEntry($locale, array('Countries'), true))) {
4343
return array();
4444
}
4545

‎src/Symfony/Component/Intl/Tests/ResourceBundle/RegionBundleTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public function testGetCountryName()
4040
{
4141
$this->reader->expects($this->once())
4242
->method('readEnt 10000 ry')
43-
->with(self::RES_DIR, 'en', array('Countries', 'AT'))
43+
->with(self::RES_DIR, 'en', array('Countries', 'AT'), true)
4444
->will($this->returnValue('Austria'));
4545

4646
$this->assertSame('Austria', $this->bundle->getCountryName('AT', 'en'));
@@ -55,7 +55,7 @@ public function testGetCountryNames()
5555

5656
$this->reader->expects($this->once())
5757
->method('readEntry')
58-
->with(self::RES_DIR, 'en', array('Countries'))
58+
->with(self::RES_DIR, 'en', array('Countries'), true)
5959
->will($this->returnValue($sortedCountries));
6060

6161
$this->assertSame($sortedCountries, $this->bundle->getCountryNames('en'));

‎src/Symfony/Component/Process/Process.php

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -308,18 +308,6 @@ public function wait($callback = null)
308308
}
309309
$this->updateStatus(false);
310310

311-
while ($this->processPipes->hasOpenHandles()) {
312-
usleep(100);
313-
foreach ($this->processPipes->readAndCloseHandles(true) as $type => $data) {
314-
if (3 == $type) {
315-
$this->fallbackExitcode = (int) $data;
316-
} else {
317-
call_user_func($this->callback, $type === self::STDOUT ? self::OUT : self::ERR, $data);
318-
}
319-
}
320-
}
321-
$this->close();
322-
323311
if ($this->processInformation['signaled']) {
324312
if ($this->isSigchildEnabled()) {
325313
throw new RuntimeException('The process has been signaled.');
@@ -1111,13 +1099,29 @@ private function captureExitCode()
11111099
*/
11121100
private function close()
11131101
{
1114-
$this->processPipes->close();
11151102
$exitcode = -1;
11161103

11171104
if (is_resource($this->process)) {
1105+
// Unix pipes must be closed before calling proc_close to void deadlock
1106+
// see manual http://php.net/manual/en/function.proc-close.php
1107+
$this->processPipes->closeUnixPipes();
11181108
$exitcode = proc_close($this->process);
11191109
}
11201110

1111+
// Windows only : when using file handles, some activity may occur after
1112+
// calling proc_close
1113+
while ($this->processPipes->hasOpenHandles()) {
1114+
usleep(100);
1115+
foreach ($this->processPipes->readAndCloseHandles(true) as $type => $data) {
1116+
if (3 == $type) {
1117+
$this->fallbackExitcode = (int) $data;
1118+
} else {
1119+
call_user_func($this->callback, $type === self::STDOUT ? self::OUT : self::ERR, $data);
1120+
}
1121+
}
1122+
}
1123+
$this->processPipes->close();
1124+
11211125
$this->exitcode = $this->exitcode !== null ? $this->exitcode : -1;
11221126
$this->exitcode = -1 != $exitcode ? $exitcode : $this->exitcode;
11231127

‎src/Symfony/Component/Process/ProcessPipes.php

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,24 @@ public function unblock()
7777
*/
7878
public function close()
7979
{
80-
foreach ($this->pipes as $offset => $pipe) {
81-
fclose($pipe);
82-
}
83-
80+
$this->closeUnixPipes();
8481
foreach ($this->fileHandles as $offset => $handle) {
8582
fclose($handle);
8683
}
87-
$this->fileHandles = $this->pipes = array();
84+
$this->fileHandles = array();
85+
}
86+
87+
/**
88+
* Closes unix pipes.
89+
*
90+
* Nothing happens in case file handles are used.
91+
*/
92+
public function closeUnixPipes()
93+
{
94+
foreach ($this->pipes as $pipe) {
95+
fclose($pipe);
96+
}
97+
$this->pipes = array();
8898
}
8999

90100
/**

‎src/Symfony/Component/Validator/Tests/Constraints/CountryValidatorTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,4 +104,14 @@ public function getInvalidCountries()
104104
array('EN'),
105105
);
106106
}
107+
108+
public function testValidateUsingCountrySpecificLocale()
109+
{
110+
\Locale::setDefault('en_GB');
111+
$existingCountry = 'GB';
112+
$this->context->expects($this->never())
113+
->method('addViolation');
114+
115+
$this->validator->validate($existingCountry, new Country());
116+
}
107117
}

0 commit comments

Comments
 (0)
0