bug #62485 [DependencyInjection] Fix loop corruption in CheckTypeDeclarationsPass (yoeunes)

nicolas-grekas · nicolas-grekas · commit 20103a8a466b · 2025-11-24T10:34:31.000+01:00
This PR was merged into the 6.4 branch. Discussion ---------- [DependencyInjection] Fix loop corruption in `CheckTypeDeclarationsPass` | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT In `CheckTypeDeclarationsPass`, the loop counter `$i` was incorrectly overwritten with the parameter name when processing **named arguments**. Comparing a string to an integer [here](https://github.com/yoeunes/symfony/blob/3866ff11d86331d1acf49043f5c86b2a054db60f/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php#L140) (e.g. `'b' < 2`) evaluates to `false`. This caused the loop to terminate early, skipping type validation for all subsequent arguments. Commits ------- 3866ff1 [DependencyInjection] Fix loop corruption in CheckTypeDeclarationsPass
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php
@@ -142,13 +142,14 @@ private function checkTypeDeclarations(Definition $checkedDefinition, \Reflectio
             if (!$p->hasType() || $p->isVariadic()) {
                 continue;
             }
+            $key = $i;
             if (\array_key_exists($p->name, $values)) {
-                $i = $p->name;
+                $key = $p->name;
             } elseif (!\array_key_exists($i, $values)) {
                 continue;
             }
 
-            $this->checkType($checkedDefinition, $values[$i], $p, $envPlaceholderUniquePrefix);
+            $this->checkType($checkedDefinition, $values[$key], $p, $envPlaceholderUniquePrefix);
         }
 
         if ($reflectionFunction->isVariadic() && ($lastParameter = end($reflectionParameters))->hasType()) {
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php
@@ -1023,6 +1023,18 @@ public function testErroredDefinitionsAreNotChecked()
 
         $this->addToAssertionCount(1);
     }
+
+    public function testCheckTypeDeclarationsSkipsSubsequentNamedArguments()
+    {
+        $container = new ContainerBuilder();
+        $container->register('service', ServiceWithTwoInts::class)
+            ->setArguments(['a' => 1, 'b' => []]);
+
+        $this->expectException(InvalidParameterTypeException::class);
+        $this->expectExceptionMessage('argument 2 of "Symfony\Component\DependencyInjection\Tests\Compiler\ServiceWithTwoInts::__construct()" accepts "int", "array" passed');
+
+        (new CheckTypeDeclarationsPass(true))->process($container);
+    }
 }
 
 class CallableClass
@@ -1038,3 +1050,10 @@ public static function __callStatic($name, $arguments)
     {
     }
 }
+
+class ServiceWithTwoInts
+{
+    public function __construct(int $a, int $b)
+    {
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -142,13 +142,14 @@ private function checkTypeDeclarations(Definition $checkedDefinition, \Reflectio`
`142`	`142`	`if (!$p->hasType() \|\| $p->isVariadic()) {`
`143`	`143`	`continue;`
`144`	`144`	`}`
	`145`	`+ $key = $i;`
`145`	`146`	`if (\array_key_exists($p->name, $values)) {`
`146`		`- $i = $p->name;`
	`147`	`+ $key = $p->name;`
`147`	`148`	`} elseif (!\array_key_exists($i, $values)) {`
`148`	`149`	`continue;`
`149`	`150`	`}`
`150`	`151`
`151`		`- $this->checkType($checkedDefinition, $values[$i], $p, $envPlaceholderUniquePrefix);`
	`152`	`+ $this->checkType($checkedDefinition, $values[$key], $p, $envPlaceholderUniquePrefix);`
`152`	`153`	`}`
`153`	`154`
`154`	`155`	`if ($reflectionFunction->isVariadic() && ($lastParameter = end($reflectionParameters))->hasType()) {`
Original file line number	Diff line number	Diff line change
`@@ -1023,6 +1023,18 @@ public function testErroredDefinitionsAreNotChecked()`
`1023`	`1023`
`1024`	`1024`	`$this->addToAssertionCount(1);`
`1025`	`1025`	`}`
	`1026`	`+`
	`1027`	`+ public function testCheckTypeDeclarationsSkipsSubsequentNamedArguments()`
	`1028`	`+ {`
	`1029`	`+ $container = new ContainerBuilder();`
	`1030`	`+ $container->register('service', ServiceWithTwoInts::class)`
	`1031`	`+ ->setArguments(['a' => 1, 'b' => []]);`
	`1032`	`+`
	`1033`	`+ $this->expectException(InvalidParameterTypeException::class);`
	`1034`	`+ $this->expectExceptionMessage('argument 2 of "Symfony\Component\DependencyInjection\Tests\Compiler\ServiceWithTwoInts::__construct()" accepts "int", "array" passed');`
	`1035`	`+`
	`1036`	`+ (new CheckTypeDeclarationsPass(true))->process($container);`
	`1037`	`+ }`
`1026`	`1038`	`}`
`1027`	`1039`
`1028`	`1040`	`class CallableClass`
`@@ -1038,3 +1050,10 @@ public static function __callStatic($name, $arguments)`
`1038`	`1050`	`{`
`1039`	`1051`	`}`
`1040`	`1052`	`}`
	`1053`	`+`
	`1054`	`+class ServiceWithTwoInts`
	`1055`	`+{`
	`1056`	`+ public function __construct(int $a, int $b)`
	`1057`	`+ {`
	`1058`	`+ }`
	`1059`	`+}`