[DependencyInjection] fixes validation of non-scalar attribute values

symfony · Jan 13, 2023 · 7867301 · 7867301
1 parent be8072b
commit 7867301
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 20 deletions.
diff --git a/Compiler/CheckDefinitionValidityPass.php b/Compiler/CheckDefinitionValidityPass.php
@@ -62,11 +62,7 @@ public function process(ContainerBuilder $container)
             // tag attribute values must be scalars
             foreach ($definition->getTags() as $name => $tags) {
                 foreach ($tags as $attributes) {
-                    foreach ($attributes as $attribute => $value) {
-                        if (!\is_scalar($value) && null !== $value) {
-                            throw new RuntimeException(sprintf('A "tags" attribute must be of a scalar-type for service "%s", tag "%s", attribute "%s".', $id, $name, $attribute));
-                        }
-                    }
+                    $this->validateAttributes($id, $name, $attributes);
                 }
             }
 
@@ -87,4 +83,16 @@ public function process(ContainerBuilder $container)
             }
         }
     }
+
+    private function validateAttributes(string $id, string $tag, array $attributes, array $path = []): void
+    {
+        foreach ($attributes as $name => $value) {
+            if (\is_array($value)) {
+                $this->validateAttributes($id, $tag, $value, [...$path, $name]);
+            } elseif (!\is_scalar($value) && null !== $value) {
+                $name = implode('.', [...$path, $name]);
+                throw new RuntimeException(sprintf('A "tags" attribute must be of a scalar-type for service "%s", tag "%s", attribute "%s".', $id, $tag, $name));
+            }
+        }
+    }
 }
diff --git a/Loader/Configurator/DefaultsConfigurator.php b/Loader/Configurator/DefaultsConfigurator.php
@@ -63,13 +63,14 @@ final public function instanceof(string $fqcn): InstanceofConfigurator
         return $this->parent->instanceof($fqcn);
     }
 
-    private function validateAttributes(string $tagName, array $attributes, string $prefix = ''): void
+    private function validateAttributes(string $tag, array $attributes, array $path = []): void
     {
-        foreach ($attributes as $attribute => $value) {
+        foreach ($attributes as $name => $value) {
             if (\is_array($value)) {
-                $this->validateAttributes($tagName, $value, $attribute.'.');
+                $this->validateAttributes($tag, $value, [...$path, $name]);
             } elseif (!\is_scalar($value ?? '')) {
-                throw new InvalidArgumentException(sprintf('Tag "%s", attribute "%s" in "_defaults" must be of a scalar-type or an array of scalar-type.', $tagName, $prefix.$attribute));
+                $name = implode('.', [...$path, $name]);
+                throw new InvalidArgumentException(sprintf('Tag "%s", attribute "%s" in "_defaults" must be of a scalar-type or an array of scalar-type.', $tag, $name));
             }
         }
     }

diff --git a/Loader/Configurator/Traits/TagTrait.php b/Loader/Configurator/Traits/TagTrait.php
@@ -33,13 +33,14 @@ final public function tag(string $name, array $attributes = []): static
         return $this;
     }
 
-    private function validateAttributes(string $tagName, array $attributes, string $prefix = ''): void
+    private function validateAttributes(string $tag, array $attributes, array $path = []): void
     {
-        foreach ($attributes as $attribute => $value) {
+        foreach ($attributes as $name => $value) {
             if (\is_array($value)) {
-                $this->validateAttributes($tagName, $value, $attribute.'.');
+                $this->validateAttributes($tag, $value, [...$path, $name]);
             } elseif (!\is_scalar($value ?? '')) {
-                throw new InvalidArgumentException(sprintf('A tag attribute must be of a scalar-type or an array of scalar-types for service "%s", tag "%s", attribute "%s".', $this->id, $tagName, $prefix.$attribute));
+                $name = implode('.', [...$path, $name]);
+                throw new InvalidArgumentException(sprintf('A tag attribute must be of a scalar-type or an array of scalar-types for service "%s", tag "%s", attribute "%s".', $this->id, $tag, $name));
             }
         }
     }

diff --git a/Loader/YamlFileLoader.php b/Loader/YamlFileLoader.php
@@ -937,13 +937,14 @@ private function checkDefinition(string $id, array $definition, string $file)
         }
     }
 
-    private function validateAttributes(string $message, array $attributes, string $prefix = ''): void
+    private function validateAttributes(string $message, array $attributes, array $path = []): void
     {
-        foreach ($attributes as $attribute => $value) {
+        foreach ($attributes as $name => $value) {
             if (\is_array($value)) {
-                $this->validateAttributes($message, $value, $attribute.'.');
+                $this->validateAttributes($message, $value, [...$path, $name]);
             } elseif (!\is_scalar($value ?? '')) {
-                throw new InvalidArgumentException(sprintf($message, $prefix.$attribute));
+                $name = implode('.', [...$path, $name]);
+                throw new InvalidArgumentException(sprintf($message, $name));
             }
         }
     }

diff --git a/Tests/Compiler/CheckDefinitionValidityPassTest.php b/Tests/Compiler/CheckDefinitionValidityPassTest.php
@@ -80,21 +80,46 @@ public function testValidTags()
         $container->register('b', 'class')->addTag('foo', ['bar' => null]);
         $container->register('c', 'class')->addTag('foo', ['bar' => 1]);
         $container->register('d', 'class')->addTag('foo', ['bar' => 1.1]);
+        $container->register('d', 'class')->addTag('foo', ['bar' => ['baz' => 'baz']]);
+        $container->register('e', 'class')->addTag('foo', ['deep' => ['foo' => ['bar' => 'baz']]]);
 
         $this->process($container);
 
         $this->addToAssertionCount(1);
     }
 
-    public function testInvalidTags()
+    /**
+     * @dataProvider provideInvalidTags
+     */
+    public function testInvalidTags(string $name, array $attributes, string $message)
     {
         $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage($message);
         $container = new ContainerBuilder();
-        $container->register('a', 'class')->addTag('foo', ['bar' => ['baz' => 'baz']]);
-
+        $container->register('a', 'class')->addTag($name, $attributes);
         $this->process($container);
     }
 
+    public static function provideInvalidTags(): iterable
+    {
+        $message = 'A "tags" attribute must be of a scalar-type for service "a", tag "%s", attribute "%s".';
+        yield 'object attribute value' => [
+            'foo',
+            ['bar' => new class() {}],
+            sprintf($message, 'foo', 'bar'),
+        ];
+        yield 'nested object attribute value' => [
+            'foo',
+            ['bar' => ['baz' => new class() {}]],
+            sprintf($message, 'foo', 'bar.baz'),
+        ];
+        yield 'deeply nested object attribute value' => [
+            'foo',
+            ['bar' => ['baz' => ['qux' => new class() {}]]],
+            sprintf($message, 'foo', 'bar.baz.qux'),
+        ];
+    }
+
     public function testDynamicPublicServiceName()
     {
         $this->expectException(EnvParameterException::class);