From a189f504ff1b7373d49d52181cd63e7a4534706d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 28 Sep 2023 15:32:02 +0200 Subject: [PATCH 1/6] Reproduce named placeholder bug --- .../PdoStatementExecuteMethodRuleTest.php | 5 ++++ tests/rules/data/named-placeholder-bug.php | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/rules/data/named-placeholder-bug.php diff --git a/tests/rules/PdoStatementExecuteMethodRuleTest.php b/tests/rules/PdoStatementExecuteMethodRuleTest.php index 124b2ee5a..d4b17548b 100644 --- a/tests/rules/PdoStatementExecuteMethodRuleTest.php +++ b/tests/rules/PdoStatementExecuteMethodRuleTest.php @@ -116,4 +116,9 @@ public function testParameterErrors(): void ], ]); } + + public function testNamedPlaceholderBug(): void { + $this->analyse([__DIR__ . '/data/pdo-stmt-execute-error.php'], [ + ]); + } } diff --git a/tests/rules/data/named-placeholder-bug.php b/tests/rules/data/named-placeholder-bug.php new file mode 100644 index 000000000..e755c52a0 --- /dev/null +++ b/tests/rules/data/named-placeholder-bug.php @@ -0,0 +1,23 @@ +prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid ' . $fromCondition); + $stmt->execute([ + 'adaid' => 1, + 'vkFrom' => 'hello world + ]); + } +} From 8e5d8d445130f134562423e5d80a149989d0ae4f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 28 Sep 2023 15:34:08 +0200 Subject: [PATCH 2/6] Update named-placeholder-bug.php --- tests/rules/data/named-placeholder-bug.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rules/data/named-placeholder-bug.php b/tests/rules/data/named-placeholder-bug.php index e755c52a0..4eed2ecf2 100644 --- a/tests/rules/data/named-placeholder-bug.php +++ b/tests/rules/data/named-placeholder-bug.php @@ -17,7 +17,7 @@ public function errors(PDO $pdo, $vkFrom) $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid ' . $fromCondition); $stmt->execute([ 'adaid' => 1, - 'vkFrom' => 'hello world + 'vkFrom' => 'hello world' ]); } } From b25513d01bfec0b750d304f2fd441d1782eccf96 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 28 Sep 2023 15:38:11 +0200 Subject: [PATCH 3/6] Update PdoStatementExecuteMethodRuleTest.php --- tests/rules/PdoStatementExecuteMethodRuleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rules/PdoStatementExecuteMethodRuleTest.php b/tests/rules/PdoStatementExecuteMethodRuleTest.php index d4b17548b..3ce0201af 100644 --- a/tests/rules/PdoStatementExecuteMethodRuleTest.php +++ b/tests/rules/PdoStatementExecuteMethodRuleTest.php @@ -118,7 +118,7 @@ public function testParameterErrors(): void } public function testNamedPlaceholderBug(): void { - $this->analyse([__DIR__ . '/data/pdo-stmt-execute-error.php'], [ + $this->analyse([__DIR__ . '/data/named-placeholder-bug.php'], [ ]); } } From 8f2e9998f9de6bbf370761d1aee7927c48ac4e4b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 28 Sep 2023 16:03:45 +0200 Subject: [PATCH 4/6] fix --- src/QueryReflection/PlaceholderValidation.php | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/QueryReflection/PlaceholderValidation.php b/src/QueryReflection/PlaceholderValidation.php index 35a85c0d6..d65349717 100644 --- a/src/QueryReflection/PlaceholderValidation.php +++ b/src/QueryReflection/PlaceholderValidation.php @@ -18,29 +18,26 @@ public function checkQuery(Expr $queryExpr, Scope $scope, array $parameters): it { $queryReflection = new QueryReflection(); + $queryStrings = []; + $namedPlaceholders = false; foreach ($queryReflection->resolveQueryStrings($queryExpr, $scope) as $queryString) { - foreach ($this->checkErrors($queryString, $parameters) as $error) { - yield $error; + $queryStrings[] = $queryString; + + if ($queryReflection->containsNamedPlaceholders($queryString, $parameters)) { + $namedPlaceholders = true; } } - } - /** - * @param array $parameters - * - * @return iterable - */ - private function checkErrors(string $queryString, array $parameters): iterable - { - $queryReflection = new QueryReflection(); - if ($queryReflection->containsNamedPlaceholders($queryString, $parameters)) { - yield from $this->validateNamedPlaceholders($queryString, $parameters); + if ($namedPlaceholders) { + yield from $this->validateNamedPlaceholders($queryStrings, $parameters); return; } - $placeholderCount = $queryReflection->countPlaceholders($queryString); - yield from $this->validateUnnamedPlaceholders($parameters, $placeholderCount); + foreach($queryStrings as $queryString) { + $placeholderCount = $queryReflection->countPlaceholders($queryString); + yield from $this->validateUnnamedPlaceholders($parameters, $placeholderCount); + } } /** @@ -85,18 +82,25 @@ private function validateUnnamedPlaceholders(array $parameters, int $placeholder } /** + * @param list $queryStrings * @param array $parameters * * @return iterable */ - private function validateNamedPlaceholders(string $queryString, array $parameters): iterable + private function validateNamedPlaceholders(array $queryStrings, array $parameters): iterable { $queryReflection = new QueryReflection(); - $namedPlaceholders = $queryReflection->extractNamedPlaceholders($queryString); - foreach ($namedPlaceholders as $namedPlaceholder) { - if (! \array_key_exists($namedPlaceholder, $parameters)) { - yield sprintf('Query expects placeholder %s, but it is missing from values given.', $namedPlaceholder); + $allNamedPlaceholders = []; + foreach($queryStrings as $queryString) { + $namedPlaceholders = $queryReflection->extractNamedPlaceholders($queryString); + + foreach ($namedPlaceholders as $namedPlaceholder) { + if (! \array_key_exists($namedPlaceholder, $parameters)) { + yield sprintf('Query expects placeholder %s, but it is missing from values given.', $namedPlaceholder); + } + + $allNamedPlaceholders[] = $namedPlaceholder; } } @@ -107,7 +111,7 @@ private function validateNamedPlaceholders(string $queryString, array $parameter if ($parameter->isOptional) { continue; } - if (! \in_array($placeholderKey, $namedPlaceholders, true)) { + if (! \in_array($placeholderKey, $allNamedPlaceholders, true)) { yield sprintf('Value %s is given, but the query does not contain this placeholder.', $placeholderKey); } } From d2181a1fb66f4d41eed3bff519792eee242d097f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 28 Sep 2023 16:06:32 +0200 Subject: [PATCH 5/6] Update PdoStatementExecuteMethodRuleTest.php --- tests/rules/PdoStatementExecuteMethodRuleTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/rules/PdoStatementExecuteMethodRuleTest.php b/tests/rules/PdoStatementExecuteMethodRuleTest.php index 3ce0201af..6ed4dfb0c 100644 --- a/tests/rules/PdoStatementExecuteMethodRuleTest.php +++ b/tests/rules/PdoStatementExecuteMethodRuleTest.php @@ -86,10 +86,6 @@ public function testParameterErrors(): void 'Query expects placeholder :asdsa, but it is missing from values given.', 54, ], - [ - 'Value :adaid is given, but the query does not contain this placeholder.', - 54, - ], [ 'Query expects placeholder :adaid, but it is missing from values given.', 61, From 6afef16526298223b03b463448ab3a638bc58928 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 28 Sep 2023 16:10:36 +0200 Subject: [PATCH 6/6] cs --- src/QueryReflection/PlaceholderValidation.php | 4 ++-- tests/rules/PdoStatementExecuteMethodRuleTest.php | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/QueryReflection/PlaceholderValidation.php b/src/QueryReflection/PlaceholderValidation.php index d65349717..a260750b9 100644 --- a/src/QueryReflection/PlaceholderValidation.php +++ b/src/QueryReflection/PlaceholderValidation.php @@ -34,7 +34,7 @@ public function checkQuery(Expr $queryExpr, Scope $scope, array $parameters): it return; } - foreach($queryStrings as $queryString) { + foreach ($queryStrings as $queryString) { $placeholderCount = $queryReflection->countPlaceholders($queryString); yield from $this->validateUnnamedPlaceholders($parameters, $placeholderCount); } @@ -92,7 +92,7 @@ private function validateNamedPlaceholders(array $queryStrings, array $parameter $queryReflection = new QueryReflection(); $allNamedPlaceholders = []; - foreach($queryStrings as $queryString) { + foreach ($queryStrings as $queryString) { $namedPlaceholders = $queryReflection->extractNamedPlaceholders($queryString); foreach ($namedPlaceholders as $namedPlaceholder) { diff --git a/tests/rules/PdoStatementExecuteMethodRuleTest.php b/tests/rules/PdoStatementExecuteMethodRuleTest.php index 6ed4dfb0c..c3f4bceb0 100644 --- a/tests/rules/PdoStatementExecuteMethodRuleTest.php +++ b/tests/rules/PdoStatementExecuteMethodRuleTest.php @@ -113,7 +113,8 @@ public function testParameterErrors(): void ]); } - public function testNamedPlaceholderBug(): void { + public function testNamedPlaceholderBug(): void + { $this->analyse([__DIR__ . '/data/named-placeholder-bug.php'], [ ]); }