Merge branch '3.1' into 3.2

nicolas-grekas · nicolas-grekas · commit 5461c1ed53b9 · 2016-11-25T13:32:42.000+01:00
* 3.1:
  [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
diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php b/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php
@@ -46,11 +46,12 @@ public function __construct(KernelInterface $kernel)
      */
     public function parse($controller)
     {
-        $originalController = $controller;
-        if (3 !== count($parts = explode(':', $controller))) {
+        $parts = explode(':', $controller);
+        if (3 !== count($parts) || in_array('', $parts, true)) {
             throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller));
         }
 
+        $originalController = $controller;
         list($bundle, $controller, $action) = $parts;
         $controller = str_replace('/', '\\', $controller);
         $bundles = array();
diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php b/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php
@@ -20,7 +20,7 @@
  * DelegatingLoader delegates route loading to other loaders using a loader resolver.
  *
  * This implementation resolves the _controller attribute from the short notation
- * to the fully-qualified form (from a:b:c to class:method).
+ * to the fully-qualified form (from a:b:c to class::method).
  *
  * @author Fabien Potencier <fabien@symfony.com>
  */
@@ -75,15 +75,17 @@ public function load($resource, $type = null)
         }
 
         foreach ($collection->all() as $route) {
-            if ($controller = $route->getDefault('_controller')) {
-                try {
-                    $controller = $this->parser->parse($controller);
-                } catch (\InvalidArgumentException $e) {
-                    // unable to optimize unknown notation
-                }
+            if (!$controller = $route->getDefault('_controller')) {
+                continue;
+            }
 
-                $route->setDefault('_controller', $controller);
+            try {
+                $controller = $this->parser->parse($controller);
+            } catch (\InvalidArgumentException $e) {
+                // unable to optimize unknown notation
             }
+
+            $route->setDefault('_controller', $controller);
         }
 
         return $collection;
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php
@@ -20,7 +20,9 @@ abstract class CompleteConfigurationTest extends \PHPUnit_Framework_TestCase
 {
     private static $containerCache = array();
 
-    abstract protected function loadFromFile(ContainerBuilder $container, $file);
+    abstract protected function getLoader(ContainerBuilder $container);
+
+    abstract protected function getFileExtension();
 
     public function testRolesHierarchy()
     {
@@ -334,6 +336,8 @@ public function testUserCheckerConfigWithNoCheckers()
 
     protected function getContainer($file)
     {
+        $file = $file.'.'.$this->getFileExtension();
+
         if (isset(self::$containerCache[$file])) {
             return self::$containerCache[$file];
         }
@@ -343,7 +347,7 @@ protected function getContainer($file)
 
         $bundle = new SecurityBundle();
         $bundle->build($container); // Attach all default factories
-        $this->loadFromFile($container, $file);
+        $this->getLoader($container)->load($file);
 
         $container->getCompilerPassConfig()->setOptimizationPasses(array());
         $container->getCompilerPassConfig()->setRemovingPasses(array());
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/PhpCompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/PhpCompleteConfigurationTest.php
@@ -17,9 +17,13 @@
 
 class PhpCompleteConfigurationTest extends CompleteConfigurationTest
 {
-    protected function loadFromFile(ContainerBuilder $container, $file)
+    protected function getLoader(ContainerBuilder $container)
     {
-        $loadXml = new PhpFileLoader($container, new FileLocator(__DIR__.'/Fixtures/php'));
-        $loadXml->load($file.'.php');
+        return new PhpFileLoader($container, new FileLocator(__DIR__.'/Fixtures/php'));
+    }
+
+    protected function getFileExtension()
+    {
+        return 'php';
     }
 }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/XmlCompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/XmlCompleteConfigurationTest.php
@@ -17,9 +17,13 @@
 
 class XmlCompleteConfigurationTest extends CompleteConfigurationTest
 {
-    protected function loadFromFile(ContainerBuilder $container, $file)
+    protected function getLoader(ContainerBuilder $container)
     {
-        $loadXml = new XmlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/xml'));
-        $loadXml->load($file.'.xml');
+        return new XmlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/xml'));
+    }
+
+    protected function getFileExtension()
+    {
+        return 'xml';
     }
 }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/YamlCompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/YamlCompleteConfigurationTest.php
@@ -17,9 +17,13 @@
 
 class YamlCompleteConfigurationTest extends CompleteConfigurationTest
 {
-    protected function loadFromFile(ContainerBuilder $container, $file)
+    protected function getLoader(ContainerBuilder $container)
     {
-        $loadXml = new YamlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/yml'));
-        $loadXml->load($file.'.yml');
+        return new YamlFileLoader($container, new FileLocator(__DIR__.'/Fixtures/yml'));
+    }
+
+    protected function getFileExtension()
+    {
+        return 'yml';
     }
 }
diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -901,15 +901,15 @@ private function createService(Definition $definition, $id, $tryProxy = true)
             $this->shareService($definition, $service, $id);
         }
 
-        foreach ($definition->getMethodCalls() as $call) {
-            $this->callMethod($service, $call);
-        }
-
         $properties = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())));
         foreach ($properties as $name => $value) {
             $service->$name = $value;
         }
 
+        foreach ($definition->getMethodCalls() as $call) {
+            $this->callMethod($service, $call);
+        }
+
         if ($callable = $definition->getConfigurator()) {
             if (is_array($callable)) {
                 $callable[0] = $parameterBag->resolveValue($callable[0]);
diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -347,8 +347,8 @@ private function addServiceInlinedDefinitions($id, $definition)
                 $code .= $this->addNewInstance($sDefinition, '$'.$name, ' = ', $id);
 
                 if (!$this->hasReference($id, $sDefinition->getMethodCalls(), true) && !$this->hasReference($id, $sDefinition->getProperties(), true)) {
-                    $code .= $this->addServiceMethodCalls(null, $sDefinition, $name);
                     $code .= $this->addServiceProperties(null, $sDefinition, $name);
+                    $code .= $this->addServiceMethodCalls(null, $sDefinition, $name);
                     $code .= $this->addServiceConfigurator(null, $sDefinition, $name);
                 }
 
@@ -517,8 +517,8 @@ private function addServiceInlinedDefinitionsSetup($id, Definition $definition)
             }
 
             $name = (string) $this->definitionVariables->offsetGet($iDefinition);
-            $code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
             $code .= $this->addServiceProperties(null, $iDefinition, $name);
+            $code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
             $code .= $this->addServiceConfigurator(null, $iDefinition, $name);
         }
 
@@ -678,8 +678,8 @@ private function addService($id, Definition $definition)
                 $this->addServiceInlinedDefinitions($id, $definition).
                 $this->addServiceInstance($id, $definition).
                 $this->addServiceInlinedDefinitionsSetup($id, $definition).
-                $this->addServiceMethodCalls($id, $definition).
                 $this->addServiceProperties($id, $definition).
+                $this->addServiceMethodCalls($id, $definition).
                 $this->addServiceConfigurator($id, $definition).
                 $this->addServiceReturn($id, $definition)
             ;
diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@@ -796,6 +796,20 @@ public function testLazyLoadedService()
         $this->assertTrue($classInList);
     }
 
+    public function testInitializePropertiesBeforeMethodCalls()
+    {
+        $container = new ContainerBuilder();
+        $container->register('foo', 'stdClass');
+        $container->register('bar', 'MethodCallClass')
+            ->setProperty('simple', 'bar')
+            ->setProperty('complex', new Reference('foo'))
+            ->addMethodCall('callMe');
+
+        $container->compile();
+
+        $this->assertTrue($container->get('bar')->callPassed(), '->compile() initializes properties before method calls');
+    }
+
     public function testAutowiring()
     {
         $container = new ContainerBuilder();
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -321,4 +321,23 @@ public function testInlinedDefinitionReferencingServiceContainer()
         $dumper = new PhpDumper($container);
         $this->assertStringEqualsFile(self::$fixturesPath.'/php/services13.php', $dumper->dump(), '->dump() dumps inline definitions which reference service_container');
     }
+
+    public function testInitializePropertiesBeforeMethodCalls()
+    {
+        require_once self::$fixturesPath.'/includes/classes.php';
+
+        $container = new ContainerBuilder();
+        $container->register('foo', 'stdClass');
+        $container->register('bar', 'MethodCallClass')
+            ->setProperty('simple', 'bar')
+            ->setProperty('complex', new Reference('foo'))
+            ->addMethodCall('callMe');
+        $container->compile();
+
+        $dumper = new PhpDumper($container);
+        eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Properties_Before_Method_Calls')));
+
+        $container = new \Symfony_DI_PhpDumper_Test_Properties_Before_Method_Calls();
+        $this->assertTrue($container->get('bar')->callPassed(), '->dump() initializes properties before method calls');
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php
@@ -59,3 +59,20 @@ public function __construct(BarClass $bar)
         $this->bar = $bar;
     }
 }
+
+class MethodCallClass
+{
+    public $simple;
+    public $complex;
+    private $callPassed = false;
+
+    public function callMe()
+    {
+        $this->callPassed = is_scalar($this->simple) && is_object($this->complex);
+    }
+
+    public function callPassed()
+    {
+        return $this->callPassed;
+    }
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
@@ -228,11 +228,11 @@ protected function getFooService()
 
         $this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);
 
-        $instance->setBar($this->get('bar'));
-        $instance->initialize();
         $instance->foo = 'bar';
         $instance->moo = $a;
         $instance->qux = array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo'));
+        $instance->setBar($this->get('bar'));
+        $instance->initialize();
         sc_configure($instance);
 
         return $instance;
@@ -425,8 +425,8 @@ protected function getInlinedService()
     {
         $this->services['inlined'] = $instance = new \Bar();
 
-        $instance->setBaz($this->get('baz'));
         $instance->pub = 'pub';
+        $instance->setBaz($this->get('baz'));
 
         return $instance;
     }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
@@ -224,11 +224,11 @@ protected function getFooService()
 
         $this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array('bar' => 'foo is bar', 'foobar' => 'bar'), true, $this);
 
-        $instance->setBar($this->get('bar'));
-        $instance->initialize();
         $instance->foo = 'bar';
         $instance->moo = $a;
         $instance->qux = array('bar' => 'foo is bar', 'foobar' => 'bar');
+        $instance->setBar($this->get('bar'));
+        $instance->initialize();
         sc_configure($instance);
 
         return $instance;
@@ -275,8 +275,8 @@ protected function getFooWithInlineService()
 
         $this->services['foo_with_inline'] = $instance = new \Foo();
 
-        $a->setBaz($this->get('baz'));
         $a->pub = 'pub';
+        $a->setBaz($this->get('baz'));
 
         $instance->setBar($a);
 
diff --git a/src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php b/src/Symfony/Component/DomCrawler/Field/ChoiceFormField.php
@@ -155,11 +155,11 @@ public function setValue($value)
     /**
      * Adds a choice to the current ones.
      *
-     * This method should only be used internally.
-     *
      * @param \DOMElement $node
      *
      * @throws \LogicException When choice provided is not multiple nor radio
+     *
+     * @internal
      */
     public function addChoice(\DOMElement $node)
     {
diff --git a/src/Symfony/Component/Form/Util/OrderedHashMapIterator.php b/src/Symfony/Component/Form/Util/OrderedHashMapIterator.php
@@ -14,9 +14,9 @@
 /**
  * Iterator for {@link OrderedHashMap} objects.
  *
- * This class is internal and should not be used.
- *
  * @author Bernhard Schussek <bschussek@gmail.com>
+ *
+ * @internal
  */
 class OrderedHashMapIterator implements \Iterator
 {
diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php
@@ -1475,6 +1475,8 @@ public function isMethod($method)
     /**
      * Checks whether or not the method is safe.
      *
+     * @see https://tools.ietf.org/html/rfc7231#section-4.2.1
+     *
      * @param bool $andCacheable Adds the additional condition that the method should be cacheable. True by default.
      *
      * @return bool
@@ -1505,6 +1507,8 @@ public function isMethodIdempotent()
     /**
      * Checks whether the method is cacheable or not.
      *
+     * @see https://tools.ietf.org/html/rfc7231#section-4.2.3
+     *
      * @return bool
      */
     public function isMethodCacheable()
diff --git a/src/Symfony/Component/Routing/RouteCompiler.php b/src/Symfony/Component/Routing/RouteCompiler.php
@@ -28,13 +28,21 @@ class RouteCompiler implements RouteCompilerInterface
      */
     const SEPARATORS = '/,;.:-_~+*=@|';
 
+    /**
+     * The maximum supported length of a PCRE subpattern name
+     * http://pcre.org/current/doc/html/pcre2pattern.html#SEC16.
+     *
+     * @internal
+     */
+    const VARIABLE_MAXIMUM_LENGTH = 32;
+
     /**
      * {@inheritdoc}
      *
      * @throws \InvalidArgumentException If a path variable is named _fragment
      * @throws \LogicException           If a variable is referenced more than once
-     * @throws \DomainException          If a variable name is numeric because PHP raises an error for such
-     *                                   subpatterns in PCRE and thus would break matching, e.g. "(?P<123>.+)".
+     * @throws \DomainException          If a variable name starts with a digit or if it is too long to be successfully used as
+     *                                   a PCRE subpattern.
      */
     public static function compile(Route $route)
     {
@@ -121,13 +129,19 @@ private static function compilePattern(Route $route, $pattern, $isHost)
             }
             $isSeparator = '' !== $precedingChar && false !== strpos(static::SEPARATORS, $precedingChar);
 
-            if (is_numeric($varName)) {
-                throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $varName, $pattern));
+            // A PCRE subpattern name must start with a non-digit. Also a PHP variable cannot start with a digit so the
+            // variable would not be usable as a Controller action argument.
+            if (preg_match('/^\d/', $varName)) {
+                throw new \DomainException(sprintf('Variable name "%s" cannot start with a digit in route pattern "%s". Please use a different name.', $varName, $pattern));
             }
             if (in_array($varName, $variables)) {
                 throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $pattern, $varName));
             }
 
+            if (strlen($varName) > self::VARIABLE_MAXIMUM_LENGTH) {
+                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));
+            }
+
             if ($isSeparator && $precedingText !== $precedingChar) {
                 $tokens[] = array('text', substr($precedingText, 0, -strlen($precedingChar)));
             } elseif (!$isSeparator && strlen($precedingText) > 0) {
diff --git a/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php b/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
diff --git a/src/Symfony/Component/Security/Core/Authentication/RememberMe/PersistentToken.php b/src/Symfony/Component/Security/Core/Authentication/RememberMe/PersistentToken.php
diff --git a/src/Symfony/Component/Validator/README.md b/src/Symfony/Component/Validator/README.md

Original file line number	Diff line number	Diff line change
`@@ -46,11 +46,12 @@ public function __construct(KernelInterface $kernel)`
`46`	`46`	`*/`
`47`	`47`	`public function parse($controller)`
`48`	`48`	`{`
`49`		`- $originalController = $controller;`
`50`		`- if (3 !== count($parts = explode(':', $controller))) {`
	`49`	`+ $parts = explode(':', $controller);`
	`50`	`+ if (3 !== count($parts) \|\| in_array('', $parts, true)) {`
`51`	`51`	`throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller));`
`52`	`52`	`}`
`53`	`53`
	`54`	`+ $originalController = $controller;`
`54`	`55`	`list($bundle, $controller, $action) = $parts;`
`55`	`56`	`$controller = str_replace('/', '\\', $controller);`
`56`	`57`	`$bundles = array();`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,9 @@ abstract class CompleteConfigurationTest extends \PHPUnit_Framework_TestCase`
`20`	`20`	`{`
`21`	`21`	`private static $containerCache = array();`
`22`	`22`
`23`		`- abstract protected function loadFromFile(ContainerBuilder $container, $file);`
	`23`	`+ abstract protected function getLoader(ContainerBuilder $container);`
	`24`	`+`
	`25`	`+ abstract protected function getFileExtension();`
`24`	`26`
`25`	`27`	`public function testRolesHierarchy()`
`26`	`28`	`{`
`@@ -334,6 +336,8 @@ public function testUserCheckerConfigWithNoCheckers()`
`334`	`336`
`335`	`337`	`protected function getContainer($file)`
`336`	`338`	`{`
	`339`	`+ $file = $file.'.'.$this->getFileExtension();`
	`340`	`+`
`337`	`341`	`if (isset(self::$containerCache[$file])) {`
`338`	`342`	`return self::$containerCache[$file];`
`339`	`343`	`}`
`@@ -343,7 +347,7 @@ protected function getContainer($file)`
`343`	`347`
`344`	`348`	`$bundle = new SecurityBundle();`
`345`	`349`	`$bundle->build($container); // Attach all default factories`
`346`		`- $this->loadFromFile($container, $file);`
	`350`	`+ $this->getLoader($container)->load($file);`
`347`	`351`
`348`	`352`	`$container->getCompilerPassConfig()->setOptimizationPasses(array());`
`349`	`353`	`$container->getCompilerPassConfig()->setRemovingPasses(array());`
Original file line number	Diff line number	Diff line change
`@@ -17,9 +17,13 @@`
`17`	`17`
`18`	`18`	`class PhpCompleteConfigurationTest extends CompleteConfigurationTest`
`19`	`19`	`{`
`20`		`- protected function loadFromFile(ContainerBuilder $container, $file)`
	`20`	`+ protected function getLoader(ContainerBuilder $container)`
`21`	`21`	`{`
`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';`
`24`	`28`	`}`
`25`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -347,8 +347,8 @@ private function addServiceInlinedDefinitions($id, $definition)`
`347`	`347`	`$code .= $this->addNewInstance($sDefinition, '$'.$name, ' = ', $id);`
`348`	`348`
`349`	`349`	`if (!$this->hasReference($id, $sDefinition->getMethodCalls(), true) && !$this->hasReference($id, $sDefinition->getProperties(), true)) {`
`350`		`- $code .= $this->addServiceMethodCalls(null, $sDefinition, $name);`
`351`	`350`	`$code .= $this->addServiceProperties(null, $sDefinition, $name);`
	`351`	`+ $code .= $this->addServiceMethodCalls(null, $sDefinition, $name);`
`352`	`352`	`$code .= $this->addServiceConfigurator(null, $sDefinition, $name);`
`353`	`353`	`}`
`354`	`354`
`@@ -517,8 +517,8 @@ private function addServiceInlinedDefinitionsSetup($id, Definition $definition)`
`517`	`517`	`}`
`518`	`518`
`519`	`519`	`$name = (string) $this->definitionVariables->offsetGet($iDefinition);`
`520`		`- $code .= $this->addServiceMethodCalls(null, $iDefinition, $name);`
`521`	`520`	`$code .= $this->addServiceProperties(null, $iDefinition, $name);`
	`521`	`+ $code .= $this->addServiceMethodCalls(null, $iDefinition, $name);`
`522`	`522`	`$code .= $this->addServiceConfigurator(null, $iDefinition, $name);`
`523`	`523`	`}`
`524`	`524`
`@@ -678,8 +678,8 @@ private function addService($id, Definition $definition)`
`678`	`678`	`$this->addServiceInlinedDefinitions($id, $definition).`
`679`	`679`	`$this->addServiceInstance($id, $definition).`
`680`	`680`	`$this->addServiceInlinedDefinitionsSetup($id, $definition).`
`681`		`- $this->addServiceMethodCalls($id, $definition).`
`682`	`681`	`$this->addServiceProperties($id, $definition).`
	`682`	`+ $this->addServiceMethodCalls($id, $definition).`
`683`	`683`	`$this->addServiceConfigurator($id, $definition).`
`684`	`684`	`$this->addServiceReturn($id, $definition)`
`685`	`685`	`;`