merged branch GromNaN/di-dump-exception (PR #8494)

fabpot · fabpot · commit 4970770d9c4a · 2013-07-20T09:33:16.000+02:00
This PR was merged into the 2.2 branch. Discussion ---------- [DependencyInjection] Add exception for service name not dumpable in PHP | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #8485 #8030 | License | MIT | Doc PR | n/a Throws an exception when the DIC is dumped to PHP, before generating invalid PHP. The regex comes from the PHP doc: http://www.php.net/manual/en/language.oop5.basic.php Commits ------- 242b318 [DependencyInjection] Add exception for service name not dumpable in PHP
diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -436,7 +436,6 @@ private function addServiceConfigurator($id, $definition, $variableName = 'insta
      */
     private function addService($id, $definition)
     {
-        $name = Container::camelize($id);
         $this->definitionVariables = new \SplObjectStorage();
         $this->referenceVariables = array();
         $this->variableCount = 0;
@@ -479,7 +478,7 @@ private function addService($id, $definition)
      *
      * $return
      */
-    protected function get{$name}Service()
+    protected function {$this->getMethodName($id)}()
     {
 
 EOF;
@@ -527,7 +526,6 @@ protected function get{$name}Service()
      */
     private function addServiceAlias($alias, $id)
     {
-        $name = Container::camelize($alias);
         $type = 'Object';
 
         if ($this->container->hasDefinition($id)) {
@@ -542,14 +540,34 @@ private function addServiceAlias($alias, $id)
      *
      * @return $type An instance of the $id service
      */
-    protected function get{$name}Service()
+    protected function {$this->getMethodName($alias)}()
     {
         return {$this->getServiceCall($id)};
     }
 
 EOF;
     }
 
+    /**
+     * Convert a service id to a valid PHP method name.
+     *
+     * @param string $id
+     *
+     * @return string
+     *
+     * @throws InvalidArgumentException
+     */
+    private function getMethodName($id)
+    {
+        $name = Container::camelize($id);
+
+        if (!preg_match('/^[a-zA-Z0-9_\x7f-\xff]+$/', $name)) {
+            throw new InvalidArgumentException(sprintf('Service id "%s" cannot be converted to a valid PHP method name.', $id));
+        }
+
+        return 'get'.$name.'Service';
+    }
+
     /**
      * Adds multiple services
      *
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -120,6 +120,18 @@ public function testAddService()
         }
     }
 
+    /**
+     * @expectedException InvalidArgumentException
+     * @expectedExceptionMessage Service id "bar$" cannot be converted to a valid PHP method name.
+     */
+    public function testAddServiceInvalidServiceId()
+    {
+        $container = new ContainerBuilder();
+        $container->register('bar$', 'FooClass');
+        $dumper = new PhpDumper($container);
+        $dumper->dump();
+    }
+
     public function testOverrideServiceWhenUsingADumpedContainer()
     {
         require_once self::$fixturesPath.'/php/services9.php';

Original file line number	Diff line number	Diff line change
`@@ -120,6 +120,18 @@ public function testAddService()`
`120`	`120`	`}`
`121`	`121`	`}`
`122`	`122`
	`123`	`+ /**`
	`124`	`+ * @expectedException InvalidArgumentException`
	`125`	`+ * @expectedExceptionMessage Service id "bar$" cannot be converted to a valid PHP method name.`
	`126`	`+ */`
	`127`	`+ public function testAddServiceInvalidServiceId()`
	`128`	`+ {`
	`129`	`+ $container = new ContainerBuilder();`
	`130`	`+ $container->register('bar$', 'FooClass');`
	`131`	`+ $dumper = new PhpDumper($container);`
	`132`	`+ $dumper->dump();`
	`133`	`+ }`
	`134`	`+`
`123`	`135`	`public function testOverrideServiceWhenUsingADumpedContainer()`
`124`	`136`	`{`
`125`	`137`	`require_once self::$fixturesPath.'/php/services9.php';`