From 5630c828d7af894bcb88d4e6ea81aba231d783ed Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Thu, 10 Feb 2022 12:26:54 +0200 Subject: [PATCH 1/3] Add support for PDOStatement bindValue/bindParam --- src/PdoReflection/PdoStatementReflection.php | 10 ++++++ src/QueryReflection/ExpressionFinder.php | 33 +++++++++++++++++ src/Rules/PdoStatementExecuteMethodRule.php | 36 ++++++++++++++----- .../PdoStatementExecuteMethodRuleTest.php | 24 +++++++++++++ tests/default/data/pdo-stmt-execute-error.php | 19 ++++++++++ tests/default/data/pdo-stmt-execute.php | 21 +++++++++++ .../data/unresolvable-pdo-statement.php | 8 +++++ 7 files changed, 143 insertions(+), 8 deletions(-) diff --git a/src/PdoReflection/PdoStatementReflection.php b/src/PdoReflection/PdoStatementReflection.php index 3ded8a629..f659bdfad 100644 --- a/src/PdoReflection/PdoStatementReflection.php +++ b/src/PdoReflection/PdoStatementReflection.php @@ -35,6 +35,16 @@ public function findPrepareQueryStringExpression(MethodCall $methodCall): ?Expr return null; } + /** + * @return MethodCall[] + */ + public function findPrepareBindCalls(MethodCall $methodCall): array + { + $exprFinder = new ExpressionFinder(); + + return $exprFinder->findBindCalls($methodCall); + } + /** * Turns a PDO::FETCH_* parameter-type into a QueryReflector::FETCH_TYPE* constant. * diff --git a/src/QueryReflection/ExpressionFinder.php b/src/QueryReflection/ExpressionFinder.php index 44120dd3f..ced2a642f 100644 --- a/src/QueryReflection/ExpressionFinder.php +++ b/src/QueryReflection/ExpressionFinder.php @@ -63,6 +63,39 @@ public function findQueryStringExpression(Expr $expr): ?Expr return null; } + /** + * @param MethodCall $expr + * + * @return MethodCall[] + */ + public function findBindCalls(Expr $expr): array + { + $result = []; + $current = $expr; + while (null !== $current) { + /** @var Assign|MethodCall|null $call */ + $call = $this->findFirstPreviousOfNode($current, function ($node) { + return $node instanceof MethodCall || $node instanceof Assign; + }); + + if (null !== $call && $this->resolveName($call->var) === $this->resolveName($expr->var)) { + if ($call instanceof Assign) { // found the prepare call + return $result; + } + + $name = $this->resolveName($call); + + if (null !== $name && \in_array(strtolower($name), ['bindparam', 'bindvalue'], true)) { + $result[] = $call; + } + } + + $current = $call; + } + + return $result; + } + /** * XXX use astral simpleNameResolver instead. * diff --git a/src/Rules/PdoStatementExecuteMethodRule.php b/src/Rules/PdoStatementExecuteMethodRule.php index f61e4da84..ddfd5b91e 100644 --- a/src/Rules/PdoStatementExecuteMethodRule.php +++ b/src/Rules/PdoStatementExecuteMethodRule.php @@ -12,6 +12,9 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\Constant\ConstantArrayType; +use PHPStan\Type\Constant\ConstantIntegerType; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\MixedType; use staabm\PHPStanDba\PdoReflection\PdoStatementReflection; use staabm\PHPStanDba\QueryReflection\PlaceholderValidation; @@ -70,16 +73,33 @@ private function checkErrors(MethodReflection $methodReflection, MethodCall $met $args = $methodCall->getArgs(); if (\count($args) < 1) { - $parameters = []; + $parameterKeys = []; + $parameterValues = []; + + $calls = $stmtReflection->findPrepareBindCalls($methodCall); + + foreach ($calls as $bindCall) { + $args = $bindCall->getArgs(); + if (\count($args) >= 2) { + $keyType = $scope->getType($args[0]->value); + if ($keyType instanceof ConstantIntegerType || $keyType instanceof ConstantStringType) { + $parameterKeys[] = $keyType; + $parameterValues[] = $scope->getType($args[1]->value); + } + } + } + + $parameterTypes = new ConstantArrayType($parameterKeys, $parameterValues); } else { $parameterTypes = $scope->getType($args[0]->value); - try { - $parameters = $queryReflection->resolveParameters($parameterTypes) ?? []; - } catch (UnresolvableQueryException $exception) { - return [ - RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($methodCall->getLine())->build(), - ]; - } + } + + try { + $parameters = $queryReflection->resolveParameters($parameterTypes) ?? []; + } catch (UnresolvableQueryException $exception) { + return [ + RuleErrorBuilder::message($exception->asRuleMessage())->tip(UnresolvableQueryException::RULE_TIP)->line($methodCall->getLine())->build(), + ]; } try { diff --git a/tests/default/PdoStatementExecuteMethodRuleTest.php b/tests/default/PdoStatementExecuteMethodRuleTest.php index 323b4500d..1de3bd308 100644 --- a/tests/default/PdoStatementExecuteMethodRuleTest.php +++ b/tests/default/PdoStatementExecuteMethodRuleTest.php @@ -83,6 +83,30 @@ public function testParameterErrors(): void '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, + ], + [ + 'Value :wrongParamName is given, but the query does not contain this placeholder.', + 61, + ], + [ + 'Query expects placeholder :adaid, but it is missing from values given.', + 65, + ], + [ + 'Value :wrongParamName is given, but the query does not contain this placeholder.', + 65, + ], + [ + 'Query expects placeholder :email, but it is missing from values given.', + 69, + ], + [ + 'Query expects placeholder :adaid, but it is missing from values given.', + 73, + ], ]); } } diff --git a/tests/default/data/pdo-stmt-execute-error.php b/tests/default/data/pdo-stmt-execute-error.php index f0f6ca3d0..c4fe548a7 100644 --- a/tests/default/data/pdo-stmt-execute-error.php +++ b/tests/default/data/pdo-stmt-execute-error.php @@ -53,4 +53,23 @@ public function conditionalSyntaxError(PDO $pdo) $stmt = $pdo->prepare($query); $stmt->execute(['adaid' => 1]); } + + public function errorsBind(PDO $pdo) + { + $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid'); + $stmt->bindValue('wrongParamName', 1); + $stmt->execute(); + + $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid'); + $stmt->bindValue(':wrongParamName', 1); + $stmt->execute(); + + $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid and email = :email'); + $stmt->bindValue(':adaid', 1); + $stmt->execute(); // wrong number of parameters + + $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid and email = :email'); + $stmt->bindValue(':email', 'email@example.org'); + $stmt->execute(); // wrong number of parameters + } } diff --git a/tests/default/data/pdo-stmt-execute.php b/tests/default/data/pdo-stmt-execute.php index 18313345a..916daeecc 100644 --- a/tests/default/data/pdo-stmt-execute.php +++ b/tests/default/data/pdo-stmt-execute.php @@ -42,6 +42,16 @@ public function execute(PDO $pdo) assertType('PDOStatement, 1: int<0, 4294967295>}'.$bothType.'>', $stmt); } + public function executeWithBindCalls(PDO $pdo) + { + $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE email = :test1 AND email = :test2'); + $test = 1337; + $stmt->setFetchMode(PDO::FETCH_ASSOC); + $stmt->bindParam(':test1', $test); + $stmt->bindValue(':test2', 1001); + $stmt->execute(); + } + public function errors(PDO $pdo) { $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid'); @@ -58,5 +68,16 @@ public function errors(PDO $pdo) assertType('PDOStatement', $stmt); $stmt->execute(); // missing parameter assertType('PDOStatement', $stmt); + + $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid'); + assertType('PDOStatement', $stmt); + $stmt->bindParam(':wrongParamName', 1); + assertType('PDOStatement', $stmt); + + $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid'); + assertType('PDOStatement', $stmt); + $stmt->bindParam(':wrongParamValue', 'hello world'); + $stmt->execute(); + assertType('PDOStatement', $stmt); } } diff --git a/tests/default/data/unresolvable-pdo-statement.php b/tests/default/data/unresolvable-pdo-statement.php index ed3a5a406..15ab96574 100644 --- a/tests/default/data/unresolvable-pdo-statement.php +++ b/tests/default/data/unresolvable-pdo-statement.php @@ -11,6 +11,10 @@ public function mixedParam(PDO $pdo, $mixed) $query = 'SELECT email FROM ada WHERE gesperrt=:gesperrt'; $stmt = $pdo->prepare($query); $stmt->execute([':gesperrt' => $mixed]); + + $stmt = $pdo->prepare($query); + $stmt->bindValue(':gesperrt', $mixed); + $stmt->execute(); } public function noErrorOnMixedQuery(PDO $pdo, $mixed) @@ -38,5 +42,9 @@ public function noErrorOnStringValue(PDO $pdo, string $string) $query = 'SELECT adaid FROM ada WHERE email=:email'; $stmt = $pdo->prepare($query); $stmt->execute([':email' => '%|'.$string.'|%']); + + $stmt = $pdo->prepare($query); + $stmt->bindValue(':email', '%|'.$string.'|%'); + $stmt->execute(); } } From 85489280c5670538b6ce34c2e43fa9b88de53cde Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Thu, 10 Feb 2022 12:29:20 +0200 Subject: [PATCH 2/3] fix --- tests/default/UnresolvablePdoStatementRuleTest.php | 2 +- tests/default/data/pdo-stmt-execute.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/default/UnresolvablePdoStatementRuleTest.php b/tests/default/UnresolvablePdoStatementRuleTest.php index 580f9e4af..20d951c9a 100644 --- a/tests/default/UnresolvablePdoStatementRuleTest.php +++ b/tests/default/UnresolvablePdoStatementRuleTest.php @@ -35,7 +35,7 @@ public function testSyntaxErrorInQueryRule(): void $this->analyse([__DIR__.'/data/unresolvable-pdo-statement.php'], [ [ 'Unresolvable Query: Cannot simulate parameter value for type: mixed.', - 13, + 17, UnresolvableQueryException::RULE_TIP, ], ]); diff --git a/tests/default/data/pdo-stmt-execute.php b/tests/default/data/pdo-stmt-execute.php index 916daeecc..be7adbc64 100644 --- a/tests/default/data/pdo-stmt-execute.php +++ b/tests/default/data/pdo-stmt-execute.php @@ -71,12 +71,12 @@ public function errors(PDO $pdo) $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid'); assertType('PDOStatement', $stmt); - $stmt->bindParam(':wrongParamName', 1); + $stmt->bindValue(':wrongParamName', 1); assertType('PDOStatement', $stmt); $stmt = $pdo->prepare('SELECT email, adaid FROM ada WHERE adaid = :adaid'); assertType('PDOStatement', $stmt); - $stmt->bindParam(':wrongParamValue', 'hello world'); + $stmt->bindValue(':wrongParamValue', 'hello world'); $stmt->execute(); assertType('PDOStatement', $stmt); } From 1fb1877ad15c17cee2777d59f24a0ab8f8c489fa Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Thu, 10 Feb 2022 12:32:03 +0200 Subject: [PATCH 3/3] fix --- tests/default/UnresolvablePdoStatementRuleTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/default/UnresolvablePdoStatementRuleTest.php b/tests/default/UnresolvablePdoStatementRuleTest.php index 20d951c9a..158695228 100644 --- a/tests/default/UnresolvablePdoStatementRuleTest.php +++ b/tests/default/UnresolvablePdoStatementRuleTest.php @@ -33,6 +33,11 @@ public function testSyntaxErrorInQueryRule(): void require_once __DIR__.'/data/unresolvable-pdo-statement.php'; $this->analyse([__DIR__.'/data/unresolvable-pdo-statement.php'], [ + [ + 'Unresolvable Query: Cannot simulate parameter value for type: mixed.', + 13, + UnresolvableQueryException::RULE_TIP, + ], [ 'Unresolvable Query: Cannot simulate parameter value for type: mixed.', 17,