bug #16956 [DependencyInjection] XmlFileLoader: enforce tags to have a name (xabbuh)

This PR was merged into the 2.3 branch.

Discussion
----------

[DependencyInjection] XmlFileLoader: enforce tags to have a name

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #16908
| License       | MIT
| Doc PR        |

Commits
-------

d86a3a7 [DependencyInjection] enforce tags to have a name

src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

@@ -186,6 +186,10 @@ class XmlFileLoader extends FileLoader
                 $parameters[$name] = SimpleXMLElement::phpize($value);
             }
 
+            if ('' === (string) $tag['name']) {
+                throw new InvalidArgumentException(sprintf('The tag name for service "%s" in %s must be a non-empty string.', $id, $file));
+            }
+
             $definition->addTag((string) $tag['name'], $parameters);
         }
 

src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

@@ -245,6 +245,10 @@ class YamlFileLoader extends FileLoader
                     throw new InvalidArgumentException(sprintf('A "tags" entry is missing a "name" key for service "%s" in %s.', $id, $file));
                 }
 
+                if (!is_string($tag['name']) || '' === $tag['name']) {
+                    throw new InvalidArgumentException(sprintf('The tag name for service "%s" in %s must be a non-empty string.', $id, $file));
+                }
+
                 $name = $tag['name'];
                 unset($tag['name']);
 

src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd

@@ -97,7 +97,7 @@
   </xsd:complexType>
 
   <xsd:complexType name="tag">
-    <xsd:attribute name="name" type="xsd:string" />
+    <xsd:attribute name="name" type="xsd:string" use="required" />
     <xsd:anyAttribute namespace="##any" processContents="lax" />
   </xsd:complexType>
 

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/tag_with_empty_name.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" ?>
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <services>
+        <service id="foo" class="BarClass">
+            <tag name="" foo="bar" />
+        </service>
+    </services>
+</container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/tag_without_name.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" ?>
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <services>
+        <service id="foo" class="BarClass">
+            <tag foo="bar" />
+        </service>
+    </services>
+</container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/tag_name_empty_string.yml

@@ -0,0 +1,6 @@
+services:
+    foo_service:
+        class:    FooClass
+        tags:
+          # tag name is an empty string
+          - { name: '', foo: bar }

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/tag_name_no_string.yml

@@ -0,0 +1,6 @@
+services:
+    foo_service:
+        class:    FooClass
+        tags:
+          # tag name is not a string
+          - { name: [], foo: bar }

src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php

@@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Loader;
 
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
@@ -207,6 +208,27 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase
         }
     }
 
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
+     */
+    public function testParseTagsWithoutNameThrowsException()
+    {
+        $container = new ContainerBuilder();
+        $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
+        $loader->load('tag_without_name.xml');
+    }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The tag name for service ".+" in .* must be a non-empty string/
+     */
+    public function testParseTagWithEmptyNameThrowsException()
+    {
+        $container = new ContainerBuilder();
+        $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
+        $loader->load('tag_with_empty_name.xml');
+    }
+
     public function testConvertDomElementToArray()
     {
         $doc = new \DOMDocument('1.0');

src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php

@@ -233,4 +233,24 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase
             $this->assertStringStartsWith('A "tags" attribute must be of a scalar-type for service ', $e->getMessage(), '->load() throws an InvalidArgumentException if a tag-attribute is not a scalar');
         }
     }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The tag name for service ".+" in .+ must be a non-empty string/
+     */
+    public function testTagWithEmptyNameThrowsException()
+    {
+        $loader = new YamlFileLoader(new ContainerBuilder(), new FileLocator(self::$fixturesPath.'/yaml'));
+        $loader->load('tag_name_empty_string.yml');
+    }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
+     * @expectedExceptionMessageREgExp /The tag name for service "\.+" must be a non-empty string/
+     */
+    public function testTagWithNonStringNameThrowsException()
+    {
+        $loader = new YamlFileLoader(new ContainerBuilder(), new FileLocator(self::$fixturesPath.'/yaml'));
+        $loader->load('tag_name_no_string.yml');
+    }
 }