Reject any duplicated parameter name

doctrine · Dec 14, 2024 · 3fdc8d8 · 3fdc8d8
1 parent aab2338
commit 3fdc8d8
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 1 deletion.
diff --git a/src/Query/QueryBuilder.php b/src/Query/QueryBuilder.php
@@ -18,13 +18,16 @@
 use Doctrine\DBAL\Statement;
 use Doctrine\DBAL\Types\Type;
 
+use function array_filter;
+use function array_intersect;
 use function array_key_exists;
 use function array_keys;
 use function array_merge;
 use function array_unshift;
 use function count;
 use function implode;
 use function is_object;
+use function sprintf;
 use function substr;
 
 /**
@@ -332,10 +335,12 @@ private function buildParametersAndTypes(): array
         $cteParams = $cteParamTypes = [];
 
         foreach ($this->commonTableExpressions as $cte) {
-            if (! $cte->query instanceof self) {
+            if (! $cte->query instanceof self || count($cte->query->params) === 0) {
                 continue;
             }
 
+            $this->guardDuplicatedParameterNames($cteParams, $cte->query->params);
+
             $cteParams     = array_merge($cteParams, $cte->query->params);
             $cteParamTypes = array_merge($cteParamTypes, $cte->query->types);
         }
@@ -344,12 +349,39 @@ private function buildParametersAndTypes(): array
             return [$this->params, $this->types];
         }
 
+        $this->guardDuplicatedParameterNames($cteParams, $this->params);
+
         return [
             array_merge($cteParams, $this->params),
             array_merge($cteParamTypes, $this->types),
         ];
     }
 
+    /**
+     * Guards against duplicated parameter names.
+     *
+     * @param list<mixed>|array<string, mixed> $params
+     * @param list<mixed>|array<string, mixed> $paramsToMerge
+     *
+     * @throws QueryException
+     */
+    private function guardDuplicatedParameterNames(array $params, array $paramsToMerge): void
+    {
+        if (count($params) === 0 || count($paramsToMerge) === 0) {
+            return;
+        }
+
+        $paramKeys    = array_filter(array_keys($params), 'is_string');
+        $cteParamKeys = array_filter(array_keys($paramsToMerge), 'is_string');
+        $duplicated   = array_intersect($paramKeys, $cteParamKeys);
+        if (count($duplicated) > 0) {
+            throw new QueryException(sprintf(
+                'Found duplicated parameter in query. The duplicated parameter names are: "%s".',
+                implode(', ', $duplicated),
+            ));
+        }
+    }
+
     /**
      * Executes an SQL statement and returns the number of affected rows.
      *

diff --git a/tests/Functional/Query/QueryBuilderTest.php b/tests/Functional/Query/QueryBuilderTest.php
@@ -17,13 +17,15 @@
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\DBAL\Query\ForUpdate\ConflictResolutionMode;
+use Doctrine\DBAL\Query\QueryException;
 use Doctrine\DBAL\Query\UnionType;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Tests\FunctionalTestCase;
 use Doctrine\DBAL\Tests\TestUtil;
 use Doctrine\DBAL\Types\Types;
 
 use function array_change_key_case;
+use function sprintf;
 
 use const CASE_UPPER;
 
@@ -441,6 +443,71 @@ public function testSelectWithCTEAndCreateNamedParametersForEachQuery(): void
         self::assertSame($expectedRows, $qb->executeQuery()->fetchAllAssociative());
     }
 
+    /**
+     * @param array<string, string> $parameters
+     *
+     * @dataProvider selectWithCTEAndCreateNamedParametersAreDuplicatedProvider
+     */
+    public function testSelectWithCTEAndCreateNamedParametersAreDuplicated(
+        array $parameters,
+        string $expectedDuplicated,
+    ): void {
+        $qb = $this->connection->createQueryBuilder();
+
+        $cteQueryBuilder1 = $this->connection->createQueryBuilder();
+        $cteQueryBuilder1->select('id')
+            ->from('for_update')
+            ->where($cteQueryBuilder1->expr()->eq(
+                'id',
+                $cteQueryBuilder1->createNamedParameter(1, ParameterType::INTEGER, $parameters['cte_a']),
+            ));
+
+        $cteQueryBuilder2 = $this->connection->createQueryBuilder();
+        $cteQueryBuilder2->select('id')
+            ->from('for_update')
+            ->where($cteQueryBuilder2->expr()->in(
+                'id',
+                $cteQueryBuilder2->createNamedParameter([1, 2], ArrayParameterType::INTEGER, $parameters['cte_b']),
+            ));
+
+        $qb->with('cte_a', $cteQueryBuilder1)
+            ->with('cte_b', $cteQueryBuilder2)
+            ->where($qb->expr()->eq(
+                'id',
+                $qb->createNamedParameter(1, ParameterType::INTEGER, $parameters['main_query']),
+            ));
+
+        self::expectException(QueryException::class);
+        self::expectExceptionMessage(sprintf(
+            'Found duplicated parameter in query. The duplicated parameter names are: "%s".',
+            $expectedDuplicated,
+        ));
+        $qb->executeQuery();
+    }
+
+    /** @return array<string, array<string, array<string, string>|string>> */
+    public static function selectWithCTEAndCreateNamedParametersAreDuplicatedProvider(): array
+    {
+        return [
+            'duplicated parameters in CTE' => [
+                'parameters' => [
+                    'cte_a' => ':id1',
+                    'cte_b' => ':id1',
+                    'main_query' => ':id2',
+                ],
+                'expectedDuplicated' => 'id1',
+            ],
+            'duplicated parameters in main query' => [
+                'parameters' => [
+                    'cte_a' => ':id1',
+                    'cte_b' => ':id2',
+                    'main_query' => ':id1',
+                ],
+                'expectedDuplicated' => 'id1',
+            ],
+        ];
+    }
+
     public function testSelectWithCTEAndCreatePositionalParametersForEachQuery(): void
     {
         if (! $this->platformSupportsCTEs()) {