Explicitely check the return value of PDOStatement::execute()
authorTim Düsterhus <duesterhus@woltlab.com>
Wed, 19 Aug 2015 11:04:53 +0000 (13:04 +0200)
committerTim Düsterhus <duesterhus@woltlab.com>
Wed, 19 Aug 2015 11:09:47 +0000 (13:09 +0200)
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.

wcfsetup/install/files/lib/system/database/statement/PreparedStatement.class.php

index 2322df15c4cc1f7a3594177619b1c41eb45127bc..70cfb3f914df80fe2f5bbebd0c7296e2e2d86ef2 100644 (file)
@@ -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) {