8000 Merge branch '2.8' into 3.1 · src-run/symfony@966d45f · GitHub
[go: up one dir, main page]
Skip to content

Commit 966d45f

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '2.8' into 3.1
* 2.8: [Routing] Fail properly when a route parameter name cannot be used as a PCRE subpattern name [FrameworkBundle] Improve performance of ControllerNameParser Update documentation link to the component [HttpFoundation] Add links to RFC-7231 [DI] Initialize properties before method calls Tag missing internals [WebProfilerBundle] Dont use request attributes in RouterController Fix complete config tests
2 parents 3918fff + 4d04c40 commit 966d45f

File tree

21 files changed

+146
-49
lines changed

21 files changed

+146
-49
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,12 @@ public function __construct(KernelInterface $kernel)
4646
*/
4747
public function parse($controller)
4848
{
49-
$originalController = $controller;
50-
if (3 !== count($parts = explode(':', $controller))) {
49+
$parts = explode(':', $controller);
50+
if (3 !== count($parts) || in_array('', $parts, true)) {
5151
throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller));
5252
}
5353

54+
$originalController = $controller;
5455
list($bundle, $controller, $action) = $parts;
5556
$controller = str_replace('/', '\\', $controller);
5657
$bundles = array();

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
* DelegatingLoader delegates route loading to other loaders using a loader resolver.
2121
*
2222
* This implementation resolves the _controller attribute from the short notation
23-
* to the fully-qualified form (from a:b:c to class:method).
23+
* to the fully-qualified form (from a:b:c to class::method).
2424
*
2525
* @author Fabien Potencier <fabien@symfony.com>
2626
*/
@@ -75,15 +75,17 @@ public function load($resource, $type = null)
7575
}
7676

7777
foreach ($collection->all() as $route) {
78-
if ($controller = $route->getDefault('_controller')) {
79-
try {
80-
$controller = $this->parser->parse($controller);
81-
} catch (\InvalidArgumentException $e) {
82-
// unable to optimize unknown notation
83-
}
78+
if (!$controller = $route->getDefault('_controller')) {
79+
continue;
80+
}
8481

85-
$route->setDefault('_controller', $controller);
82+
try {
83+
$controller = $this->parser->parse($controller);
84+
} catch (\InvalidArgumentException $e) {
85+
// unable to optimize unknown notation
8686
}
87+
88+
$route->setDefault('_controller', $controller);
8789
}
8890

8991
return $collection;

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ abstract class CompleteConfigurationTest extends \PHPUnit_Framework_TestCase
2020
{
2121
private static $containerCache = array();
2222

23-
abstract protected function loadFromFile(ContainerBuilder $container, $file);
23+
abstract protected function getLoader(ContainerBuilder $container);
24+
25+
abstract protected function getFileExtension();
2426

2527
public function testRolesHierarchy()
2628
{
@@ -257,6 +259,8 @@ public function testUserCheckerConfigWithNoCheckers()
257259

258260
protected function getContainer($file)
259261
{
262+
$file = $file.'.'.$this->getFileExtension();
263+
260264
if (isset(self::$containerCache[$file])) {
261265
return self::$containerCache[$file];
262266
}
@@ -266,7 +270,7 @@ protected function getContainer($file)
266270

267271
$bundle = new SecurityBundle();
268272
$bundle->build($container); // Attach all default factories
269-
$this->loadFromFile($container, $file);
273+
$this->getLoader($container)->load($file);
270274

271275
$container->getCompilerPassConfig()->setOptimizationPasses(array());
272276
$container->getCompilerPassConfig()->setRemovingPasses(array());

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/PhpCompleteConfigurationTest.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@
1717

1818
class PhpCompleteConfigurationTest extends CompleteConfigurationTest
1919
{
20-
protected function loadFromFile(ContainerBuilder $container, $file)
20+
protected function getLoader(ContainerBuilder $container)
2121
{
22-
$loadXml = new PhpFileLoader($container, new FileLocator(__DIR__.'/Fixtures/php'));
23-
$loadXml->load($file.'.php');
22+
return new PhpFileLoader($container, new FileLocator(__DIR__.'/Fixtures/php'));
23+
}
24+
25+
protected function getFileExtension()
26+
{
27+
return 'php';
2428
}
2529
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/XmlCompleteConfigurationTest.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@
1717

1818
class XmlCompleteConfigurationTest extends CompleteConfigurationTest
1919
{
20-
protected function loadFromFile(ContainerBuilder $container, $file)
20+
protected function getLoader(ContainerBuilder $container)
2121
{
22-
$loadXml = new XmlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/xml'));
23-
$loadXml->load($file.'.xml');
22+
return new XmlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/xml'));
23+
}
24+
25+
protected function getFileExtension()
26+
{
27+
return 'xml';
2428
}
2529
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/YamlCompleteConfigurationTest.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@
1717

1818
class YamlCompleteConfigurationTest extends CompleteConfigurationTest
1919
{
20-
protected function loadFromFile(ContainerBuilder $container, $file)
20+
protected function getLoader(ContainerBuilder $container)
2121
{
22-
$loadXml = new YamlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/yml'));
23-
$loadXml->load($file.'.yml');
22+
return new YamlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/yml'));
23+
}
24+
25+
protected function getFileExtension()
26+
{
27+
return 'yml';
2428
}
2529
}

‎src/Symfony/Bundle/WebProfilerBundle/Controller/RouterController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ private function getTraces(RequestDataCollector $request, $method)
8787
$traceRequest = Request::create(
8888
$request->getPathInfo(),
8989
$request->getRequestServer()->get('REQUEST_METHOD'),
90-
$request->getRequestAttributes()->all(),
90+
array(),
9191
$request->getRequestCookies()->all(),
9292
array(),
9393
$request->getRequestServer()->all()

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -860,15 +860,15 @@ private function createService(Definition $definition, $id, $tryProxy = true)
860860
$this->shareService($definition, $service, $id);
861861
}
862862

863-
foreach ($definition->getMethodCalls() as $call) {
864-
$this->callMethod($service, $call);
865-
}
866-
867863
$properties = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())));
868864
foreach ($properties as $name => $value) {
869865
$service->$name = $value;
870866
}
871867

868+
foreach ($definition->getMethodCalls() as $call) {
869+
$this->callMethod($service, $call);
870+
}
871+
872872
if ($callable = $definition->getConfigurator()) {
873873
if (is_array($callable)) {
874874
$callable[0] = $parameterBag->resolveValue($callable[0]);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,8 @@ private function addServiceInlinedDefinitions($id, $definition)
334334
$code .= $this->addNewInstance($sDefinition, '$'.$name, ' = ', $id);
335335

336336
if (!$this->hasReference($id, $sDefinition->getMethodCalls(), true) && !$this->hasReference($id, $sDefinition->getProperties(), true)) {
337-
$code .= $this->addServiceMethodCalls(null, $sDefinition, $name);
338337
$code .= $this->addServiceProperties(null, $sDefinition, $name);
338+
$code .= $this->addServiceMethodCalls(null, $sDefinition, $name);
339339
$code .= $this->addServiceConfigurator(null, $sDefinition, $name);
340340
}
341341

@@ -504,8 +504,8 @@ private function addServiceInlinedDefinitionsSetup($id, Definition $definition)
504504
}
505505

506506
$name = (string) $this->definitionVariables->offsetGet($iDefinition);
507-
$code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
508507
$code .= $this->addServiceProperties(null, $iDefinition, $name);
508+
$code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
509509
$code .= $this->addServiceConfigurator(null, $iDefinition, $name);
510510
}
511511

@@ -663,8 +663,8 @@ private function addService($id, Definition $definition)
663663
$this->addServiceInlinedDefinitions($id, $definition).
664664
$this->addServiceInstance($id, $definition).
665665
$this->addServiceInlinedDefinitionsSetup($id, $definition).
666-
$this->addServiceMethodCalls($id, $definition).
667666
$this->addServiceProperties($id, $definition).
667+
$this->addServiceMethodCalls($id, $definition).
668668
$this->addServiceConfigurator($id, $definition).
669669
$this->addServiceReturn($id, $definition)
670670
;

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,20 @@ public function testLazyLoadedService()
756756
$this->assertTrue($classInList);
757757
}
758758

759+
public function testInitializePropertiesBeforeMethodCalls()
760+
{
761+
$container = new ContainerBuilder();
762+
$container->register('foo', 'stdClass');
763+
$container->register('bar', 'MethodCallClass')
764+
->setProperty('simple', 'bar')
765+
->setProperty('complex', new Reference('foo'))
766+
->addMethodCall('callMe');
767+
768+
$container->compile();
769+
770+
$this->assertTrue($container->get('bar')->callPassed(), '->compile() initializes properties before method calls');
771+
}
772+
759773
public function testAutowiring()
760774
{
761775
$container = new ContainerBuilder();

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,4 +295,23 @@ public function testInlinedDefinitionReferencingServiceContainer()
295295
$dumper = new PhpDumper($container);
296296
$this->assertStringEqualsFile(self::$fixturesPath.'/php/services13.php', $dumper->dump(), '->dump() dumps inline definitions which reference service_container');
297297
}
298+
299+
public function testInitializePropertiesBeforeMethodCalls()
300+
{
301+
require_once self::$fixturesPath.'/includes/classes.php';
302+
303+
$container = new ContainerBuilder();
304+
$container->register('foo', 'stdClass');
305+
$container->register('bar', 'MethodCallClass')
306+
->setProperty('simple', 'bar')
307+
->setProperty('complex', new Reference('foo'))
308+
->addMethodCall('callMe');
309+
$container->compile();
310+
311+
$dumper = new PhpDumper($container);
312+
eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Properties_Before_Method_Calls')));
313+
314+
$container = new \Symfony_DI_PhpDumper_Test_Properties_Before_Method_Calls();
315+
$this->assertTrue($container->get('bar')->callPassed(), '->dump() initializes properties before method calls');
316+
}
298317
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,20 @@ public function __construct(BarClass $bar)
5959
$this->bar = $bar;
6060
}
6161
}
62+
63+
class MethodCallClass
64+
{
65+
public $simple;
66+
public $complex;
67+
private $callPassed = false;
68+
69+
public function callMe()
70+
{
71+
$this->callPassed = is_scalar($this->simple) && is_object($this->complex);
72+
}
73+
74+
public function callPassed()
75+
{
76+
return $this->callPassed;
77+
}
78+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,11 @@ protected function getFooService()
221221

222222
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);
223223

224-
$instance->setBar($this->get('bar'));
225-
$instance->initialize();
226224
$instance->foo = 'bar';
227225
$instance->moo = $a;
228226
$instance->qux = array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo'));
227+
$instance->setBar($this->get('bar'));
228+
$instance->initialize();
229229
sc_configure($instance);
230230

231231
return $instance;
@@ -418,8 +418,8 @@ protected function getInlinedService()
418418
{
419419
$this->services['inlined'] = $instance = new \Bar();
420420

421-
$instance->setBaz($this->get('baz'));
422421
$instance->pub = 'pub';
422+
$instance->setBaz($this->get('baz'));
423423

424424
return $instance;
425425
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,11 @@ protected function getFooService()
224224

225225
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array('bar' => 'foo is bar', 'foobar' => 'bar'), true, $this);
226226

227-
$instance->setBar($this->get('bar'));
228-
$instance->initialize();
229227
$instance->foo = 'bar';
230228
$instance->moo = $a;
231229
$instance->qux = array('bar' => 'foo is bar', 'foobar' => 'bar');
230+
$instance->setBar($this->get('bar'));
231+
$instance->initialize();
232232
sc_configure($instance);
233233

234234
return $instance;
@@ -275,8 +275,8 @@ protected function getFooWithInlineService()
275275

276276
$this->services['foo_with_inline'] = $instance = new \Foo();
277277

278-
$a->setBaz($this->get('baz'));
279278
$a->pub = 'pub';
279+
$a->setBaz($this->get('baz'));
280280

281281
$instance->setBar($a);
282282

‎src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,11 @@ public function setValue($value)
155155
/**
156156
* Adds a choice to the current ones.
157157
*
158-
* This method should only be used internally.
159-
*
160158
* @param \DOMElement $node
161159
*
162160
* @throws \LogicException When choice provided is not multiple nor radio
161+
*
162+
* @internal
163163
*/
164164
public function addChoice(\DOMElement $node)
165165
{

‎src/Symfony/Component/Form/Util/OrderedHashMapIterator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@
1414
/**
1515
* Iterator for {@link OrderedHashMap} objects.
1616
*
17-
* This class is internal and should not be used.
18-
*
1917
* @author Bernhard Schussek <bschussek@gmail.com>
18+
*
19+
* @internal
2020
*/
2121
class OrderedHashMapIterator implements \Iterator
2222
{

‎src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,8 @@ public function isMethod($method)
14751475
/**
14761476
* Checks whether the method is safe or not.
14771477
*
1478+
* @see https://tools.ietf.org/html/rfc7231#section-4.2.1
1479+
*
14781480
* @param bool $andCacheable Adds the additional condition that the method should be cacheable. True by default.
14791481
*
14801482
* @return bool
@@ -1487,6 +1489,8 @@ public function isMethodSafe(/* $andCacheable = true */)
14871489
/**
14881490
* Checks whether the method is cacheable or not.
14891491
*
1492+
* @see https://tools.ietf.org/html/rfc7231#section-4.2.3
1493+
*
14901494
* @return bool
14911495
*/
14921496
public function isMethodCacheable()

‎src/Symfony/Component/Routing/RouteCompiler.php

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,20 @@ class RouteCompiler implements RouteCompilerInterface
2828
*/
2929
const SEPARATORS = '/,;.:-_~+*=@|';
3030

31+
/**
32+
* The maximum supported length of a PCRE subpattern name
33+
* http://pcre.org/current/doc/html/pcre2pattern.html#SEC16.
34+
*
35+
* @internal
36+
*/
37+
const VARIABLE_MAXIMUM_LENGTH = 32;
38+
3139
/**
3240
* {@inheritdoc}
3341
*
3442
* @throws \LogicException If a variable is referenced more than once
35-
* @throws \DomainException If a variable name is numeric because PHP raises an error for such
36-
* subpatterns in PCRE and thus would break matching, e.g. "(?P<123>.+)".
43+
* @throws \DomainException If a variable name starts with a digit or if it is too long to be successfully used as
44+
* a PCRE subpattern.
3745
*/
3846
public static function compile(Route $route)
3947
{
@@ -95,13 +103,19 @@ private static function compilePattern(Route $route, $pattern, $isHost)
95103
$precedingChar = strlen($precedingText) > 0 ? substr($precedingText, -1) : '';
96104
$isSeparator = '' !== $precedingChar && false !== strpos(static::SEPARATORS, $precedingChar);
97105

98-
if (is_numeric($varName)) {
99-
throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $varName, $pattern));
106+
// A PCRE subpattern name must start with a non-digit. Also a PHP variable cannot start with a digit so the
107+
// variable would not be usable as a Controller action argument.
108+
if (preg_match('/^\d/', $varName)) {
109+
throw new \DomainException(sprintf('Variable name "%s" cannot start with a digit in route pattern "%s". Please use a different name.', $varName, $pattern));
100110
}
101111
if (in_array($varName, $variables)) {
102112
throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $pattern, $varName));
103113
}
104114

115+
if (strlen($varName) > self::VARIABLE_MAXIMUM_LENGTH) {
116+
throw new \DomainException(sprintf('Variable name "%s" cannot be longer than %s characters in route pattern "%s". Please use a shorter name.', $varName, self::VARIABLE_MAXIMUM_LENGTH, $pattern));
117+
}
118+
105119
if ($isSeparator && strlen($precedingText) > 1) {
106120
$tokens[] = array('text', substr($precedingText, 0, -1));
107121
} elseif (!$isSeparator && strlen($precedingText) > 0) {

0 commit comments

Comments
 (0)
0