From ef22580a327de1e683a4c3c2af5814e1864cabdb Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 6 Nov 2020 11:48:52 +0100 Subject: [PATCH 1/3] Use query builder instead of OC_DB in trashbin Signed-off-by: Joas Schilling --- apps/files_trashbin/lib/Trashbin.php | 77 ++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 21 deletions(-) diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php index 3a430948f0..d0ad7eb913 100644 --- a/apps/files_trashbin/lib/Trashbin.php +++ b/apps/files_trashbin/lib/Trashbin.php @@ -128,17 +128,20 @@ class Trashbin { * @return array (filename => array (timestamp => original location)) */ public static function getLocations($user) { - $query = \OC_DB::prepare('SELECT `id`, `timestamp`, `location`' - . ' FROM `*PREFIX*files_trash` WHERE `user`=?'); - $result = $query->execute([$user]); + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->select('id', 'timestamp', 'location') + ->from('files_trash') + ->where($query->expr()->eq('user', $query->createNamedParameter($user))); + $result = $query->execute(); $array = []; - while ($row = $result->fetchRow()) { + while ($row = $result->fetch()) { if (isset($array[$row['id']])) { $array[$row['id']][$row['timestamp']] = $row['location']; } else { $array[$row['id']] = [$row['timestamp'] => $row['location']]; } } + $result->closeCursor(); return $array; } @@ -151,11 +154,19 @@ class Trashbin { * @return string original location */ public static function getLocation($user, $filename, $timestamp) { - $query = \OC_DB::prepare('SELECT `location` FROM `*PREFIX*files_trash`' - . ' WHERE `user`=? AND `id`=? AND `timestamp`=?'); - $result = $query->execute([$user, $filename, $timestamp])->fetchAll(); - if (isset($result[0]['location'])) { - return $result[0]['location']; + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->select('location') + ->from('files_trash') + ->where($query->expr()->eq('user', $query->createNamedParameter($user))) + ->andWhere($query->expr()->eq('id', $query->createNamedParameter($filename))) + ->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp))); + + $result = $query->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + if (isset($row['location'])) { + return $row['location']; } else { return false; } @@ -208,8 +219,13 @@ class Trashbin { if ($view->file_exists($target)) { - $query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)"); - $result = $query->execute([$targetFilename, $timestamp, $targetLocation, $user]); + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->insert('files_trash') + ->setValue('id', $query->createNamedParameter($targetFilename)) + ->setValue('timestamp', $query->createNamedParameter($timestamp)) + ->setValue('location', $query->createNamedParameter($targetLocation)) + ->setValue('user', $query->createNamedParameter($user)); + $result = $query->execute(); if (!$result) { \OC::$server->getLogger()->error('trash bin database couldn\'t be updated for the files owner', ['app' => 'files_trashbin']); } @@ -322,8 +338,13 @@ class Trashbin { } if ($moveSuccessful) { - $query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)"); - $result = $query->execute([$filename, $timestamp, $location, $owner]); + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->insert('files_trash') + ->setValue('id', $query->createNamedParameter($filename)) + ->setValue('timestamp', $query->createNamedParameter($timestamp)) + ->setValue('location', $query->createNamedParameter($location)) + ->setValue('user', $query->createNamedParameter($owner)); + $result = $query->execute(); if (!$result) { \OC::$server->getLogger()->error('trash bin database couldn\'t be updated', ['app' => 'files_trashbin']); } @@ -481,8 +502,12 @@ class Trashbin { self::restoreVersions($view, $file, $filename, $uniqueFilename, $location, $timestamp); if ($timestamp) { - $query = \OC_DB::prepare('DELETE FROM `*PREFIX*files_trash` WHERE `user`=? AND `id`=? AND `timestamp`=?'); - $query->execute([$user, $filename, $timestamp]); + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->delete('files_trash') + ->where($query->expr()->eq('user', $query->createNamedParameter($user))) + ->andWhere($query->expr()->eq('id', $query->createNamedParameter($filename))) + ->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp))); + $query->execute(); } return true; @@ -568,8 +593,11 @@ class Trashbin { // actual file deletion $trash->delete(); - $query = \OC_DB::prepare('DELETE FROM `*PREFIX*files_trash` WHERE `user`=?'); - $query->execute([$user]); + + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->delete('files_trash') + ->where($query->expr()->eq('user', $query->createNamedParameter($user))); + $query->execute(); // Bulk PostDelete-Hook \OC_Hook::emit('\OCP\Trashbin', 'deleteAll', ['paths' => $filePaths]); @@ -618,8 +646,13 @@ class Trashbin { $size = 0; if ($timestamp) { - $query = \OC_DB::prepare('DELETE FROM `*PREFIX*files_trash` WHERE `user`=? AND `id`=? AND `timestamp`=?'); - $query->execute([$user, $filename, $timestamp]); + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->delete('files_trash') + ->where($query->expr()->eq('user', $query->createNamedParameter($user))) + ->andWhere($query->expr()->eq('id', $query->createNamedParameter($filename))) + ->andWhere($query->expr()->eq('timestamp', $query->createNamedParameter($timestamp))); + $query->execute(); + $file = $filename . '.d' . $timestamp; } else { $file = $filename; @@ -701,8 +734,10 @@ class Trashbin { * @return bool result of db delete operation */ public static function deleteUser($uid) { - $query = \OC_DB::prepare('DELETE FROM `*PREFIX*files_trash` WHERE `user`=?'); - return $query->execute([$uid]); + $query = \OC::$server->getDatabaseConnection()->getQueryBuilder(); + $query->delete('files_trash') + ->where($query->expr()->eq('user', $query->createNamedParameter($uid))); + return (bool) $query->execute(); } /** From a01da78f1d604882745cfe0ef537772d391376de Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Sat, 3 Oct 2020 16:33:42 +0200 Subject: [PATCH 2/3] Add explicit typecast for $value. Signed-off-by: Daniel Kesselberg --- lib/private/DB/QueryBuilder/QueryBuilder.php | 2 +- lib/public/DB/QueryBuilder/IQueryBuilder.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index 4fde0fb451..0b4f983129 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -868,7 +868,7 @@ class QueryBuilder implements IQueryBuilder { * * * @param string $column The column into which the value should be inserted. - * @param string $value The value that should be inserted into the column. + * @param IParameter|string $value The value that should be inserted into the column. * * @return $this This QueryBuilder instance. */ diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 3a9c846043..c1e7f6bc4f 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -651,7 +651,7 @@ interface IQueryBuilder { * * * @param string $column The column into which the value should be inserted. - * @param string $value The value that should be inserted into the column. + * @param IParameter|string $value The value that should be inserted into the column. * * @return $this This QueryBuilder instance. * @since 8.2.0 From f28c436fd508fea5b34076f76e494105ea3c1741 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 9 Nov 2020 10:21:05 +0100 Subject: [PATCH 3/3] Check in failing psalm checks Signed-off-by: Morris Jobke --- build/psalm-baseline.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index e2bd4ac0e9..b38f1400a1 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3904,6 +3904,9 @@ + + $this->functionBuilder->lower($x) + parent::castColumn($column, $type) @@ -3917,6 +3920,9 @@ + + $value + $this->connection