From 503d93d94aa7498100a41ad8eb1276e5d5a0f9e9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 19 Aug 2015 13:04:53 +0200 Subject: [PATCH] Explicitely check the return value of PDOStatement::execute() PHP apparently does not always throw an Exception when a statement could not be executed successfully. An example of this is: $sql = "SELECT ?"; $statement = WCF::getDB()->prepareStatement($sql); $statement->execute([ 1, 2 ]); // returns false This code is erroneous, as we try to send more parameters than there are placeholders in the query. Thus execute() properly returns false, but it does not throw an Exception. Interestingly enough the reverse case properly throws: Sending less parameters than placeholders. --- .../database/statement/PreparedStatement.class.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/wcfsetup/install/files/lib/system/database/statement/PreparedStatement.class.php b/wcfsetup/install/files/lib/system/database/statement/PreparedStatement.class.php index 2322df15c4..70cfb3f914 100644 --- a/wcfsetup/install/files/lib/system/database/statement/PreparedStatement.class.php +++ b/wcfsetup/install/files/lib/system/database/statement/PreparedStatement.class.php @@ -86,9 +86,14 @@ class PreparedStatement { try { if (WCF::benchmarkIsEnabled()) Benchmark::getInstance()->start($this->query, Benchmark::TYPE_SQL_QUERY); - if (empty($parameters)) $this->pdoStatement->execute(); - else $this->pdoStatement->execute($parameters); + if (empty($parameters)) $result = $this->pdoStatement->execute(); + else $result = $this->pdoStatement->execute($parameters); + if (!$result) { + $errorInfo = $this->pdoStatement->errorInfo(); + throw new DatabaseException('Could not execute prepared statement: '.$errorInfo[0].' '.$errorInfo[2], $this->database, $this); + } + if (WCF::benchmarkIsEnabled()) Benchmark::getInstance()->stop(); } catch (\PDOException $e) { -- 2.20.1