Merge pull request #3799 from owncloud/fix_pdo_statement_wrapper_numrows_on_modification

Fix pdo statement wrapper numrows on modification

7 files changed, 76 insertions, 43 deletions

diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php
index e8e53859bd8..b3de85254e2 100644
--- a/apps/files_encryption/lib/util.php
+++ b/apps/files_encryption/lib/util.php
@@ -362,17 +362,7 @@ class Util {
 
 		}
 
-		$query = \OCP\DB::prepare($sql);
-
-		if ($query->execute($args)) {
-
-			return true;
-
-		} else {
-
-			return false;
-
-		}
+		return is_numeric(\OC_DB::executeAudited($sql, $args));
 
 	}
 
@@ -1063,8 +1053,7 @@ class Util {
 		$sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ? and `migration_status` = ?';
 		$args = array(self::MIGRATION_IN_PROGRESS, $this->userId, self::MIGRATION_OPEN);
 		$query = \OCP\DB::prepare($sql);
-		$result = $query->execute($args);
-		$manipulatedRows = $result->numRows();
+		$manipulatedRows = $query->execute($args);
 
 		if ($manipulatedRows === 1) {
 			$return = true;
@@ -1087,8 +1076,7 @@ class Util {
 		$sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ? and `migration_status` = ?';
 		$args = array(self::MIGRATION_COMPLETED, $this->userId, self::MIGRATION_IN_PROGRESS);
 		$query = \OCP\DB::prepare($sql);
-		$result = $query->execute($args);
-		$manipulatedRows = $result->numRows();
+		$manipulatedRows = $query->execute($args);
 
 		if ($manipulatedRows === 1) {
 			$return = true;
diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index 04f73cf01fe..6f6b8d0f016 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -578,14 +578,12 @@ abstract class Access {
 		');
 
 		//feed the DB
-		$res = $insert->execute(array($dn, $ocname, $this->getUUID($dn), $dn, $ocname));
+		$insRows = $insert->execute(array($dn, $ocname, $this->getUUID($dn), $dn, $ocname));
 
-		if(\OCP\DB::isError($res)) {
+		if(\OCP\DB::isError($insRows)) {
 			return false;
 		}
 
-		$insRows = $res->numRows();
-
 		if($insRows === 0) {
 			return false;
 		}
diff --git a/apps/user_ldap/lib/helper.php b/apps/user_ldap/lib/helper.php
index 10ed40ebd6a..f65f466789f 100644
--- a/apps/user_ldap/lib/helper.php
+++ b/apps/user_ldap/lib/helper.php
@@ -90,13 +90,13 @@ class Helper {
 				AND `appid` = \'user_ldap\'
 				AND `configkey` NOT IN (\'enabled\', \'installed_version\', \'types\', \'bgjUpdateGroupsLastRun\')
 		');
-		$res = $query->execute(array($prefix.'%'));
+		$delRows = $query->execute(array($prefix.'%'));
 
-		if(\OCP\DB::isError($res)) {
+		if(\OCP\DB::isError($delRows)) {
 			return false;
 		}
 
-		if($res->numRows() === 0) {
+		if($delRows === 0) {
 			return false;
 		}
 
diff --git a/lib/connector/sabre/locks.php b/lib/connector/sabre/locks.php
index 7aca2e43712..69496c15ada 100644
--- a/lib/connector/sabre/locks.php
+++ b/lib/connector/sabre/locks.php
@@ -182,7 +182,7 @@ class OC_Connector_Sabre_Locks extends Sabre_DAV_Locks_Backend_Abstract {
 		}
 		$result = OC_DB::executeAudited( $sql, array(OC_User::getUser(), $uri, $lockInfo->token));
 
-		return $result->numRows() === 1;
+		return $result === 1;
 
 	}
 
diff --git a/lib/db.php b/lib/db.php
index f6acf6af1b7..4d6788f2bda 100644
--- a/lib/db.php
+++ b/lib/db.php
@@ -326,11 +326,12 @@ class OC_DB {
 	 * @param string $query Query string
 	 * @param int $limit
 	 * @param int $offset
+	 * @param bool $isManipulation
 	 * @return MDB2_Statement_Common prepared SQL query
 	 *
 	 * SQL query via MDB2 prepare(), needs to be execute()'d!
 	 */
-	static public function prepare( $query , $limit=null, $offset=null ) {
+	static public function prepare( $query , $limit = null, $offset = null, $isManipulation = null) {
 
 		if (!is_null($limit) && $limit != -1) {
 			if (self::$backend == self::BACKEND_MDB2) {
@@ -367,12 +368,23 @@ class OC_DB {
 			OC_Log::write('core', 'DB prepare : '.$query, OC_Log::DEBUG);
 		}
 		self::connect();
+		
+		if ($isManipulation === null) {
+			//try to guess, so we return the number of rows on manipulations
+			$isManipulation = self::isManipulation($query);
+		}
+		
 		// return the result
 		if(self::$backend==self::BACKEND_MDB2) {
-			$result = self::$connection->prepare( $query );
+			// differentiate between query and manipulation
+			if ($isManipulation) {
+				$result = self::$connection->prepare( $query, null, MDB2_PREPARE_MANIP );
+			} else {
+				$result = self::$connection->prepare( $query, null, MDB2_PREPARE_RESULT );
+			}
 
 			// Die if we have an error (error means: bad query, not 0 results!)
-			if( PEAR::isError($result)) {
+			if( self::isError($result)) {
 				throw new DatabaseException($result->getMessage(), $query);
 			}
 		}else{
@@ -381,7 +393,8 @@ class OC_DB {
 			}catch(PDOException $e) {
 				throw new DatabaseException($e->getMessage(), $query);
 			}
-			$result=new PDOStatementWrapper($result);
+			// differentiate between query and manipulation
+			$result = new PDOStatementWrapper($result, $isManipulation);
 		}
 		if ((is_null($limit) || $limit == -1) and self::$cachingEnabled ) {
 			$type = OC_Config::getValue( "dbtype", "sqlite" );
@@ -391,7 +404,33 @@ class OC_DB {
 		}
 		return $result;
 	}
-
+	
+	/**
+	 * tries to guess the type of statement based on the first 10 characters
+	 * the current check allows some whitespace but does not work with IF EXISTS or other more complex statements
+	 * 
+	 * @param string $sql
+	 */
+	static public function isManipulation( $sql ) {
+		$selectOccurence = stripos ($sql, "SELECT");
+		if ($selectOccurence !== false && $selectOccurence < 10) {
+			return false;
+		}
+		$insertOccurence = stripos ($sql, "INSERT");
+		if ($insertOccurence !== false && $insertOccurence < 10) {
+			return true;
+		}
+		$updateOccurence = stripos ($sql, "UPDATE");
+		if ($updateOccurence !== false && $updateOccurence < 10) {
+			return true;
+		}
+		$deleteOccurance = stripos ($sql, "DELETE");
+		if ($deleteOccurance !== false && $deleteOccurance < 10) {
+			return true;
+		}
+		return false;
+	}
+	
 	/**
 	 * @brief execute a prepared statement, on error write log and throw exception
 	 * @param mixed $stmt PDOStatementWrapper | MDB2_Statement_Common ,
@@ -718,6 +757,9 @@ class OC_DB {
 		} catch(PDOException $e) {
 			OC_Template::printExceptionErrorPage( $e );
 		}
+		if ($result === 0) {
+			return true;
+		}
 
 		return $result;
 	}
@@ -920,7 +962,7 @@ class OC_DB {
 	 * @return bool
 	 */
 	public static function isError($result) {
-		if(!$result) {
+		if(self::$backend==self::BACKEND_PDO and $result === false) {
 			return true;
 		}elseif(self::$backend==self::BACKEND_MDB2 and PEAR::isError($result)) {
 			return true;
@@ -998,11 +1040,13 @@ class PDOStatementWrapper{
 	/**
 	 * @var PDOStatement
 	 */
-	private $statement=null;
-	private $lastArguments=array();
+	private $statement = null;
+	private $isManipulation = false;
+	private $lastArguments = array();
 
-	public function __construct($statement) {
-		$this->statement=$statement;
+	public function __construct($statement, $isManipulation = false) {
+		$this->statement = $statement;
+		$this->isManipulation = $isManipulation;
 	}
 
 	/**
@@ -1024,16 +1068,19 @@ class PDOStatementWrapper{
 				$input = $this->tryFixSubstringLastArgumentDataForMSSQL($input);
 			}
 
-			$result=$this->statement->execute($input);
+			$result = $this->statement->execute($input);
 		} else {
-			$result=$this->statement->execute();
+			$result = $this->statement->execute();
 		}
 		
-		if ($result) {
-			return $this;
-		} else {
+		if ($result === false) {
 			return false;
 		}
+		if ($this->isManipulation) {
+			return $this->statement->rowCount();
+		} else {
+			return $this;
+		}
 	}
 
 	private function tryFixSubstringLastArgumentDataForMSSQL($input) {
diff --git a/lib/vcategories.php b/lib/vcategories.php
index 7bac6e7d4e3..84036958359 100644
--- a/lib/vcategories.php
+++ b/lib/vcategories.php
@@ -125,7 +125,7 @@ class OC_VCategories {
 				OC_Log::write('core', __METHOD__. 'DB error: ' . OC_DB::getErrorMessage($result), OC_Log::ERROR);
 				return false;
 			}
-			return ($result->numRows() == 0);
+			return ($result->numRows() === 0);
 		} catch(Exception $e) {
 			OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(),
 				OCP\Util::ERROR);
diff --git a/tests/lib/db.php b/tests/lib/db.php
index afbdb413c3d..0ba7d5d4b06 100644
--- a/tests/lib/db.php
+++ b/tests/lib/db.php
@@ -40,7 +40,7 @@ class Test_DB extends PHPUnit_Framework_TestCase {
 		$this->assertFalse((bool)$row); //PDO returns false, MDB2 returns null
 		$query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`,`uri`) VALUES (?,?)');
 		$result = $query->execute(array('fullname test', 'uri_1'));
-		$this->assertTrue((bool)$result);
+		$this->assertEquals('1', $result);
 		$query = OC_DB::prepare('SELECT `fullname`,`uri` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?');
 		$result = $query->execute(array('uri_1'));
 		$this->assertTrue((bool)$result);
@@ -57,7 +57,7 @@ class Test_DB extends PHPUnit_Framework_TestCase {
 	public function testNOW() {
 		$query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`,`uri`) VALUES (NOW(),?)');
 		$result = $query->execute(array('uri_2'));
-		$this->assertTrue((bool)$result);
+		$this->assertEquals('1', $result);
 		$query = OC_DB::prepare('SELECT `fullname`,`uri` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?');
 		$result = $query->execute(array('uri_2'));
 		$this->assertTrue((bool)$result);
@@ -66,7 +66,7 @@ class Test_DB extends PHPUnit_Framework_TestCase {
 	public function testUNIX_TIMESTAMP() {
 		$query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`,`uri`) VALUES (UNIX_TIMESTAMP(),?)');
 		$result = $query->execute(array('uri_3'));
-		$this->assertTrue((bool)$result);
+		$this->assertEquals('1', $result);
 		$query = OC_DB::prepare('SELECT `fullname`,`uri` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?');
 		$result = $query->execute(array('uri_3'));
 		$this->assertTrue((bool)$result);
@@ -105,7 +105,7 @@ class Test_DB extends PHPUnit_Framework_TestCase {
 		// Normal test to have same known data inserted.
 		$query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`, `uri`, `carddata`) VALUES (?, ?, ?)');
 		$result = $query->execute(array($fullname, $uri, $carddata));
-		$this->assertTrue((bool)$result);
+		$this->assertEquals('1', $result);
 		$query = OC_DB::prepare('SELECT `fullname`, `uri`, `carddata` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?');
 		$result = $query->execute(array($uri));
 		$this->assertTrue((bool)$result);