From 8dbf1dce7b9978084a4587ce157f29ac640cc19a Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Tue, 13 May 2025 20:51:43 +0200 Subject: [PATCH 1/6] Database: start reworking the DBA to work with PDO --- models/Asset.php | 180 ++++++-------- models/AssetIterator.php | 35 ++- models/Authentication.php | 8 +- models/Database.php | 503 ++++++++++++++------------------------ models/ErrorLog.php | 10 +- models/Image.php | 8 +- models/Member.php | 42 ++-- models/Setting.php | 27 +- models/Tag.php | 84 ++++--- 9 files changed, 391 insertions(+), 506 deletions(-) diff --git a/models/Asset.php b/models/Asset.php index b3803bf..e8cdedd 100644 --- a/models/Asset.php +++ b/models/Asset.php @@ -56,7 +56,7 @@ class Asset $row = Registry::get('db')->queryAssoc(' SELECT * FROM assets - WHERE id_asset = {int:id_asset}', + WHERE id_asset = :id_asset', [ 'id_asset' => $id_asset, ]); @@ -69,7 +69,7 @@ class Asset $row = Registry::get('db')->queryAssoc(' SELECT * FROM assets - WHERE slug = {string:slug}', + WHERE slug = :slug', [ 'slug' => $slug, ]); @@ -85,7 +85,7 @@ class Asset $row['meta'] = $db->queryPair(' SELECT variable, value FROM assets_meta - WHERE id_asset = {int:id_asset}', + WHERE id_asset = :id_asset', [ 'id_asset' => $row['id_asset'], ]); @@ -94,16 +94,15 @@ class Asset $row['thumbnails'] = $db->queryPair(' SELECT CONCAT( - width, - {string:x}, - height, - IF(mode != {string:empty}, CONCAT({string:_}, mode), {string:empty}) + width, :x, height, + IF(mode != :empty1, CONCAT(:_, mode), :empty2) ) AS selector, filename FROM assets_thumbs - WHERE id_asset = {int:id_asset}', + WHERE id_asset = :id_asset', [ 'id_asset' => $row['id_asset'], - 'empty' => '', + 'empty1' => '', + 'empty2' => '', 'x' => 'x', '_' => '_', ]); @@ -121,14 +120,14 @@ class Asset $res = $db->query(' SELECT * FROM assets - WHERE id_asset IN ({array_int:id_assets}) + WHERE id_asset IN (@id_assets) ORDER BY id_asset', [ 'id_assets' => $id_assets, ]); $assets = []; - while ($asset = $db->fetch_assoc($res)) + while ($asset = $db->fetchAssoc($res)) { $assets[$asset['id_asset']] = $asset; $assets[$asset['id_asset']]['meta'] = []; @@ -138,7 +137,7 @@ class Asset $metas = $db->queryRows(' SELECT id_asset, variable, value FROM assets_meta - WHERE id_asset IN ({array_int:id_assets}) + WHERE id_asset IN (@id_assets) ORDER BY id_asset', [ 'id_assets' => $id_assets, @@ -150,17 +149,16 @@ class Asset $thumbnails = $db->queryRows(' SELECT id_asset, CONCAT( - width, - {string:x}, - height, - IF(mode != {string:empty}, CONCAT({string:_}, mode), {string:empty}) + width, :x, height, + IF(mode != :empty1, CONCAT(:_, mode), :empty2) ) AS selector, filename FROM assets_thumbs - WHERE id_asset IN ({array_int:id_assets}) + WHERE id_asset IN (@id_assets) ORDER BY id_asset', [ 'id_assets' => $id_assets, - 'empty' => '', + 'empty1' => '', + 'empty2' => '', 'x' => 'x', '_' => '_', ]); @@ -169,7 +167,9 @@ class Asset $assets[$thumb[0]]['thumbnails'][$thumb[1]] = $thumb[2]; if ($return_format === 'array') + { return $assets; + } else { $objects = []; @@ -262,10 +262,10 @@ class Asset INSERT INTO assets (id_user_uploaded, subdir, filename, title, slug, mimetype, image_width, image_height, date_captured, priority) VALUES - ({int:id_user_uploaded}, {string:subdir}, {string:filename}, {string:title}, {string:slug}, {string:mimetype}, - {int:image_width}, {int:image_height}, - IF({int:date_captured} > 0, FROM_UNIXTIME({int:date_captured}), NULL), - {int:priority})', + (:id_user_uploaded, :subdir, :filename, :title, :slug, :mimetype, + :image_width, :image_height, + IF(:date_captured > 0, FROM_UNIXTIME(:date_captured), NULL), + :priority)', [ 'id_user_uploaded' => isset($id_user) ? $id_user : Registry::get('user')->getUserId(), 'subdir' => $preferred_subdir, @@ -285,7 +285,7 @@ class Asset return false; } - $data['id_asset'] = $db->insert_id(); + $data['id_asset'] = $db->insertId(); return $return_format === 'object' ? new self($data) : $data; } @@ -324,7 +324,7 @@ class Asset $posts = Registry::get('db')->queryValues(' SELECT id_post FROM posts_assets - WHERE id_asset = {int:id_asset}', + WHERE id_asset = :id_asset', ['id_asset' => $this->id_asset]); // TODO: fix empty post iterator. @@ -495,12 +495,12 @@ class Asset return Registry::get('db')->query(' UPDATE assets SET - mimetype = {string:mimetype}, - image_width = {int:image_width}, - image_height = {int:image_height}, - date_captured = {datetime:date_captured}, - priority = {int:priority} - WHERE id_asset = {int:id_asset}', + mimetype = :mimetype, + image_width = :image_width, + image_height = :image_height, + date_captured = :date_captured, + priority = :priority + WHERE id_asset = :id_asset', [ 'id_asset' => $this->id_asset, 'mimetype' => $this->mimetype, @@ -527,8 +527,8 @@ class Asset if (!empty($to_remove)) $db->query(' DELETE FROM assets_meta - WHERE id_asset = {int:id_asset} AND - variable IN({array_string:variables})', + WHERE id_asset = :id_asset AND + variable IN(@variables)', [ 'id_asset' => $this->id_asset, 'variables' => array_keys($to_remove), @@ -559,63 +559,40 @@ class Asset { $db = Registry::get('db'); - // First: delete associated metadata + // Delete any and all thumbnails, if this is an image. + if ($this->isImage()) + { + $image = $this->getImage(); + $image->removeAllThumbnails(); + } + + // Delete all meta info for this asset. $db->query(' DELETE FROM assets_meta - WHERE id_asset = {int:id_asset}', - [ - 'id_asset' => $this->id_asset, - ]); + WHERE id_asset = :id_asset', + ['id_asset' => $this->id_asset]); - // Second: figure out what tags to recount cardinality for + // Figure out what tags to recount cardinality for $recount_tags = $db->queryValues(' SELECT id_tag FROM assets_tags - WHERE id_asset = {int:id_asset}', - [ - 'id_asset' => $this->id_asset, - ]); + WHERE id_asset = :id_asset', + ['id_asset' => $this->id_asset]); + // Delete asset association for these tags $db->query(' DELETE FROM assets_tags - WHERE id_asset = {int:id_asset}', - [ - 'id_asset' => $this->id_asset, - ]); + WHERE id_asset = :id_asset', + ['id_asset' => $this->id_asset]); Tag::recount($recount_tags); - // Third: figure out what associated thumbs to delete - $thumbs_to_delete = $db->queryValues(' - SELECT filename - FROM assets_thumbs - WHERE id_asset = {int:id_asset}', - [ - 'id_asset' => $this->id_asset, - ]); - - foreach ($thumbs_to_delete as $filename) - { - $thumb_path = THUMBSDIR . '/' . $this->subdir . '/' . $filename; - if (is_file($thumb_path)) - unlink($thumb_path); - } - - $db->query(' - DELETE FROM assets_thumbs - WHERE id_asset = {int:id_asset}', - [ - 'id_asset' => $this->id_asset, - ]); - // Reset asset ID for tags that use this asset for their thumbnail - $rows = $db->query(' + $rows = $db->queryValues(' SELECT id_tag FROM tags - WHERE id_asset_thumb = {int:id_asset}', - [ - 'id_asset' => $this->id_asset, - ]); + WHERE id_asset_thumb = :id_asset', + ['id_asset' => $this->id_asset]); if (!empty($rows)) { @@ -632,10 +609,8 @@ class Asset $return = $db->query(' DELETE FROM assets - WHERE id_asset = {int:id_asset}', - [ - 'id_asset' => $this->id_asset, - ]); + WHERE id_asset = :id_asset', + ['id_asset' => $this->id_asset]); return $return; } @@ -664,7 +639,7 @@ class Asset Registry::get('db')->query(' DELETE FROM assets_tags - WHERE id_asset = {int:id_asset} AND id_tag IN ({array_int:id_tags})', + WHERE id_asset = :id_asset AND id_tag IN (@id_tags)', [ 'id_asset' => $this->id_asset, 'id_tags' => $id_tags, @@ -682,16 +657,17 @@ class Asset public static function getOffset($offset, $limit, $order, $direction) { + $order = $order . ($direction == 'up' ? ' ASC' : ' DESC'); + return Registry::get('db')->queryAssocs(' SELECT a.id_asset, a.subdir, a.filename, a.image_width, a.image_height, a.mimetype, u.id_user, u.first_name, u.surname FROM assets AS a LEFT JOIN users AS u ON a.id_user_uploaded = u.id_user - ORDER BY {raw:order} - LIMIT {int:offset}, {int:limit}', + ORDER BY ' . $order . ' + LIMIT :offset, :limit', [ - 'order' => $order . ($direction == 'up' ? ' ASC' : ' DESC'), 'offset' => $offset, 'limit' => $limit, ]); @@ -704,18 +680,16 @@ class Asset return Registry::get('db')->query(' UPDATE assets - SET id_asset = {int:id_asset}, - id_user_uploaded = {int:id_user_uploaded}, - subdir = {string:subdir}, - filename = {string:filename}, - title = {string:title}, - slug = {string:slug}, - mimetype = {string:mimetype}, - image_width = {int:image_width}, - image_height = {int:image_height}, - date_captured = {datetime:date_captured}, - priority = {int:priority} - WHERE id_asset = {int:id_asset}', + SET subdir = :subdir, + filename = :filename, + title = :title, + slug = :slug, + mimetype = :mimetype, + image_width = :image_width, + image_height = :image_height, + date_captured = :date_captured, + priority = :priority + WHERE id_asset = :id_asset', get_object_vars($this)); } @@ -733,27 +707,27 @@ class Asset // Direction depends on whether we're browsing a tag or timeline if (isset($tag)) { - $where[] = 't.id_tag = {int:id_tag}'; + $where[] = 't.id_tag = :id_tag'; $params['id_tag'] = $tag->id_tag; - $params['where_op'] = $previous ? '<' : '>'; - $params['order_dir'] = $previous ? 'DESC' : 'ASC'; + $where_op = $previous ? '<' : '>'; + $order_dir = $previous ? 'DESC' : 'ASC'; } else { - $params['where_op'] = $previous ? '>' : '<'; - $params['order_dir'] = $previous ? 'ASC' : 'DESC'; + $where_op = $previous ? '>' : '<'; + $order_dir = $previous ? 'ASC' : 'DESC'; } // Take active filter into account as well if (!empty($activeFilter) && ($user = Member::fromSlug($activeFilter)) !== false) { - $where[] = 'id_user_uploaded = {int:id_user_uploaded}'; + $where[] = 'id_user_uploaded = :id_user_uploaded'; $params['id_user_uploaded'] = $user->getUserId(); } // Use complete ordering when sorting the set - $where[] = '(a.date_captured, a.id_asset) {raw:where_op} ' . - '({datetime:date_captured}, {int:id_asset})'; + $where[] = '(a.date_captured, a.id_asset) ' . $where_op . + ' (:date_captured, :id_asset)'; // Stringify conditions together $where = '(' . implode(') AND (', $where) . ')'; @@ -765,7 +739,7 @@ class Asset ' . (isset($tag) ? ' INNER JOIN assets_tags AS t ON a.id_asset = t.id_asset' : '') . ' WHERE ' . $where . ' - ORDER BY a.date_captured {raw:order_dir}, a.id_asset {raw:order_dir} + ORDER BY a.date_captured ' . $order_dir . ', a.id_asset ' . $order_dir . ' LIMIT 1', $params); diff --git a/models/AssetIterator.php b/models/AssetIterator.php index 2eabbf3..5690fb6 100644 --- a/models/AssetIterator.php +++ b/models/AssetIterator.php @@ -28,7 +28,7 @@ class AssetIterator extends Asset public function next() { - $row = $this->db->fetch_assoc($this->res_assets); + $row = $this->db->fetchAssoc($this->res_assets); // No more rows? if (!$row) @@ -36,7 +36,7 @@ class AssetIterator extends Asset // Looks up metadata. $row['meta'] = []; - while ($meta = $this->db->fetch_assoc($this->res_meta)) + while ($meta = $this->db->fetchAssoc($this->res_meta)) { if ($meta['id_asset'] != $row['id_asset']) continue; @@ -49,7 +49,7 @@ class AssetIterator extends Asset // Looks up thumbnails. $row['thumbnails'] = []; - while ($thumbs = $this->db->fetch_assoc($this->res_thumbs)) + while ($thumbs = $this->db->fetchAssoc($this->res_thumbs)) { if ($thumbs['id_asset'] != $row['id_asset']) continue; @@ -78,13 +78,13 @@ class AssetIterator extends Asset if (!$this->res_assets) return; - $this->db->free_result($this->res_assets); + $this->db->free($this->res_assets); $this->res_assets = null; } public function num() { - return $this->db->num_rows($this->res_assets); + return $this->db->rowCount($this->res_assets); } public static function all() @@ -114,9 +114,9 @@ class AssetIterator extends Asset { $params['mime_type'] = $options['mime_type']; if (is_array($options['mime_type'])) - $where[] = 'a.mimetype IN({array_string:mime_type})'; + $where[] = 'a.mimetype IN(@mime_type)'; else - $where[] = 'a.mimetype = {string:mime_type}'; + $where[] = 'a.mimetype = :mime_type'; } if (isset($options['id_user_uploaded'])) { @@ -129,7 +129,17 @@ class AssetIterator extends Asset $where[] = 'id_asset IN( SELECT l.id_asset FROM assets_tags AS l - WHERE l.id_tag = {int:id_tag})'; + WHERE l.id_tag = :id_tag)'; + } + elseif (isset($options['tag'])) + { + $params['tag'] = $options['tag']; + $where[] = 'id_asset IN( + SELECT l.id_asset + FROM assets_tags AS l + INNER JOIN tags AS t + ON l.id_tag = t.id_tag + WHERE t.slug = :tag)'; } // Make it valid SQL. @@ -145,7 +155,7 @@ class AssetIterator extends Asset FROM assets AS a WHERE ' . $where . ' ORDER BY ' . $order . (!empty($params['limit']) ? ' - LIMIT {int:offset}, {int:limit}' : ''), + LIMIT :offset, :limit' : ''), $params); // Get a resource object for the asset meta. @@ -165,9 +175,9 @@ class AssetIterator extends Asset SELECT id_asset, filename, CONCAT( width, - {string:x}, + :x, height, - IF(mode != {string:empty}, CONCAT({string:_}, mode), {string:empty}) + IF(mode != :empty1, CONCAT(:_, mode), :empty2) ) AS selector FROM assets_thumbs WHERE id_asset IN( @@ -177,7 +187,8 @@ class AssetIterator extends Asset ) ORDER BY id_asset', $params + [ - 'empty' => '', + 'empty1' => '', + 'empty2' => '', 'x' => 'x', '_' => '_', ]); diff --git a/models/Authentication.php b/models/Authentication.php index 324ad91..98b3fb0 100644 --- a/models/Authentication.php +++ b/models/Authentication.php @@ -23,7 +23,7 @@ class Authentication $password_hash = Registry::get('db')->queryValue(' SELECT password_hash FROM users - WHERE emailaddress = {string:emailaddress}', + WHERE emailaddress = :emailaddress', [ 'emailaddress' => $emailaddress, ]); @@ -132,9 +132,9 @@ class Authentication return Registry::get('db')->query(' UPDATE users SET - password_hash = {string:hash}, - reset_key = {string:blank} - WHERE id_user = {int:id_user}', + password_hash = :hash, + reset_key = :blank + WHERE id_user = :id_user', [ 'id_user' => $id_user, 'hash' => $hash, diff --git a/models/Database.php b/models/Database.php index c8e2e02..346a8b9 100644 --- a/models/Database.php +++ b/models/Database.php @@ -1,39 +1,29 @@ connection = new mysqli($server, $user, $password, $name); - $this->connection->set_charset('utf8mb4'); + $this->connection = new PDO("mysql:host=$host;dbname=$name;charset=utf8mb4", $user, $password, [ + PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, + PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, + PDO::ATTR_EMULATE_PREPARES => false, + ]); } - catch (mysqli_sql_exception $e) + // Give up if we have a connection error. + catch (PDOException $e) { http_response_code(503); echo '

Database Connection Problems

Our software could not connect to the database. We apologise for any inconvenience and ask you to check back later.

'; @@ -52,301 +42,178 @@ class Database } /** - * Fetches a row from a given recordset, using field names as keys. + * Fetches a row from a given statement/recordset, using field names as keys. */ - public function fetch_assoc($resource) + public function fetchAssoc($stmt) { - return mysqli_fetch_assoc($resource); + return $stmt->fetch(PDO::FETCH_ASSOC); } /** - * Fetches a row from a given recordset, encapsulating into an object. + * Fetches a row from a given statement/recordset, encapsulating into an object. */ - public function fetch_object($resource, $class) + public function fetchObject($stmt, $class) { - return mysqli_fetch_object($resource, $class); + return $stmt->fetchObject($class); } /** - * Fetches a row from a given recordset, using numeric keys. + * Fetches a row from a given statement/recordset, using numeric keys. */ - public function fetch_row($resource) + public function fetchNum($stmt) { - return mysqli_fetch_row($resource); + return $stmt->fetch(PDO::FETCH_NUM); } /** - * Destroys a given recordset. + * Destroys a given statement/recordset. */ - public function free_result($resource) + public function free($stmt) { - return mysqli_free_result($resource); - } - - public function data_seek($result, $row_num) - { - return mysqli_data_seek($result, $row_num); + return $stmt->closeCursor(); } /** - * Returns the amount of rows in a given recordset. + * Returns the amount of rows in a given statement/recordset. */ - public function num_rows($resource) + public function rowCount($stmt) { - return mysqli_num_rows($resource); + return $stmt->rowCount(); } /** - * Returns the amount of fields in a given recordset. + * Returns the amount of fields in a given statement/recordset. */ - public function num_fields($resource) + public function columnCount($stmt) { - return mysqli_num_fields($resource); - } - - /** - * Escapes a string. - */ - public function escape_string($string) - { - return mysqli_real_escape_string($this->connection, $string); - } - - /** - * Unescapes a string. - */ - public function unescape_string($string) - { - return stripslashes($string); - } - - /** - * Returns the last MySQL error. - */ - public function error() - { - return mysqli_error($this->connection); - } - - public function server_info() - { - return mysqli_get_server_info($this->connection); - } - - /** - * Selects a database on a given connection. - */ - public function select_db($database) - { - return mysqli_select_db($database, $this->connection); - } - - /** - * Returns the amount of rows affected by the previous query. - */ - public function affected_rows() - { - return mysqli_affected_rows($this->connection); + return $stmt->columnCount(); } /** * Returns the id of the row created by a previous query. */ - public function insert_id() + public function insertId($name = null) { - return mysqli_insert_id($this->connection); + return $this->connection->lastInsertId($name); } /** - * Do a MySQL transaction. + * Start a transaction. */ - public function transaction($operation = 'commit') + public function beginTransaction() { - switch ($operation) - { - case 'begin': - case 'rollback': - case 'commit': - return @mysqli_query($this->connection, strtoupper($operation)); - default: - return false; - } + return $this->connection->beginTransaction(); } /** - * Function used as a callback for the preg_match function that parses variables into database queries. + * Rollback changes in a transaction. */ - private function replacement_callback($matches) + public function rollback() { - list ($values, $connection) = $this->db_callback; + return $this->connection->rollBack(); + } - if (!isset($matches[2])) - throw new UnexpectedValueException('Invalid value inserted or no type specified.'); + /** + * Commit changes in a transaction. + */ + public function commit() + { + return $this->connection->commit(); + } - if (!isset($values[$matches[2]])) - throw new UnexpectedValueException('The database value you\'re trying to insert does not exist: ' . htmlspecialchars($matches[2])); - - $replacement = $values[$matches[2]]; - - switch ($matches[1]) + private function expandPlaceholders($db_string, array &$db_values) + { + foreach ($db_values as $key => &$value) { - case 'int': - if ((!is_numeric($replacement) || (string) $replacement !== (string) (int) $replacement) && $replacement !== 'NULL') - throw new UnexpectedValueException('Wrong value type sent to the database for field: ' . $matches[2] . '. Integer expected.'); - return $replacement !== 'NULL' ? (string) (int) $replacement : 'NULL'; - break; - - case 'string': - case 'text': - return $replacement !== 'NULL' ? sprintf('\'%1$s\'', mysqli_real_escape_string($connection, $replacement)) : 'NULL'; - break; - - case 'array_int': - if (is_array($replacement)) + if (str_contains($db_string, ':' . $key)) + { + if (is_array($value)) { - if (empty($replacement)) - throw new UnexpectedValueException('Database error, given array of integer values is empty.'); - - foreach ($replacement as $key => $value) - { - if (!is_numeric($value) || (string) $value !== (string) (int) $value) - throw new UnexpectedValueException('Wrong value type sent to the database for field: ' . $matches[2] . '. Array of integers expected.'); - - $replacement[$key] = (string) (int) $value; - } - - return implode(', ', $replacement); + throw new UnexpectedValueException('Array ' . $key . + ' is used as a scalar placeholder. Did you mean to use \'@\' instead?'); } - else - throw new UnexpectedValueException('Wrong value type sent to the database for field: ' . $matches[2] . '. Array of integers expected.'); - break; - - case 'array_string': - if (is_array($replacement)) + // Prepare date/time values + if (is_a($value, 'DateTime')) { - if (empty($replacement)) - throw new UnexpectedValueException('Database error, given array of string values is empty.'); - - foreach ($replacement as $key => $value) - $replacement[$key] = sprintf('\'%1$s\'', mysqli_real_escape_string($connection, $value)); - - return implode(', ', $replacement); + $value = $value->format('Y-m-d H:i:s'); + } + } + elseif (str_contains($db_string, '@' . $key)) + { + if (!is_array($value)) + { + throw new UnexpectedValueException('Scalar value ' . $key . + ' is used as an array placeholder. Did you mean to use \':\' instead?'); } - else - throw new UnexpectedValueException('Wrong value type sent to the database for field: ' . $matches[2] . '. Array of strings expected.'); - break; - case 'date': - if (preg_match('~^(\d{4})-([0-1]?\d)-([0-3]?\d)$~', $replacement, $date_matches) === 1) - return sprintf('\'%04d-%02d-%02d\'', $date_matches[1], $date_matches[2], $date_matches[3]); - elseif ($replacement === 'NULL') - return 'NULL'; - else - throw new UnexpectedValueException('Wrong value type sent to the database for field: ' . $matches[2] . '. Date expected.'); - break; - - case 'datetime': - if (is_a($replacement, 'DateTime')) - return $replacement->format('\'Y-m-d H:i:s\''); - elseif (preg_match('~^(\d{4})-([0-1]?\d)-([0-3]?\d) (\d{2}):(\d{2}):(\d{2})$~', $replacement, $date_matches) === 1) - return sprintf('\'%04d-%02d-%02d %02d:%02d:%02d\'', $date_matches[1], $date_matches[2], $date_matches[3], $date_matches[4], $date_matches[5], $date_matches[6]); - elseif ($replacement === 'NULL') - return 'NULL'; - else - throw new UnexpectedValueException('Wrong value type sent to the database for field: ' . $matches[2] . '. DateTime expected.'); - break; - - case 'float': - if (!is_numeric($replacement) && $replacement !== 'NULL') - throw new UnexpectedValueException('Wrong value type sent to the database for field: ' . $matches[2] . '. Floating point number expected.'); - return $replacement !== 'NULL' ? (string) (float) $replacement : 'NULL'; - break; - - case 'identifier': - // Backticks inside identifiers are supported as of MySQL 4.1. We don't need them here. - return '`' . strtr($replacement, ['`' => '', '.' => '']) . '`'; - break; - - case 'raw': - return $replacement; - break; - - case 'bool': - case 'boolean': - // In mysql this is a synonym for tinyint(1) - return (bool)$replacement ? 1 : 0; - break; - - default: - throw new UnexpectedValueException('Undefined type ' . $matches[1] . ' used in the database query'); - break; + // Create placeholders for all array elements + $placeholders = array_map(fn($num) => ':' . $key . $num, range(0, count($value) - 1)); + $db_string = str_replace('@' . $key, implode(', ', $placeholders), $db_string); + } + else + { + // throw new Exception('Warning: unused key in query: ' . $key); + } } + + return $db_string; } /** * Escapes and quotes a string using values passed, and executes the query. */ - public function query($db_string, $db_values = []) + public function query($db_string, array $db_values = []): PDOStatement { - // One more query.... - $this->query_count ++; + // One more query... + $this->query_count++; - // Overriding security? This is evil! - $security_override = $db_values === 'security_override' || !empty($db_values['security_override']); + // Error out if hardcoded strings are detected + if (strpos($db_string, '\'') !== false) + throw new UnexpectedValueException('Hack attempt: illegal character (\') used in query.'); - // Please, just use new style queries. - if (strpos($db_string, '\'') !== false && !$security_override) - throw new UnexpectedValueException('Hack attempt!', 'Illegal character (\') used in query.'); - - if (!$security_override && !empty($db_values)) - { - // Set some values for use in the callback function. - $this->db_callback = [$db_values, $this->connection]; - - // Insert the values passed to this function. - $db_string = preg_replace_callback('~{([a-z_]+)(?::([a-zA-Z0-9_-]+))?}~', [&$this, 'replacement_callback'], $db_string); - - // Save some memory. - $this->db_callback = []; - } - - if (defined("DB_LOG_QUERIES") && DB_LOG_QUERIES) + if (defined('DB_LOG_QUERIES') && DB_LOG_QUERIES) $this->logged_queries[] = $db_string; try { - $return = @mysqli_query($this->connection, $db_string, empty($this->unbuffered) ? MYSQLI_STORE_RESULT : MYSQLI_USE_RESULT); + // Preprocessing/checks: prepare any arrays for binding + $db_string = $this->expandPlaceholders($db_string, $db_values); + + // Prepare query for execution + $statement = $this->connection->prepare($db_string); + + // Bind parameters... the hard way, due to a limit/offset hack. + // NB: bindParam binds by reference, hence &$value here. + foreach ($db_values as $key => &$value) + { + // Assumption: both scalar and array values are preprocessed to use named ':' placeholders + if (!str_contains($db_string, ':' . $key)) + continue; + + if (!is_array($value)) + { + $statement->bindParam(':' . $key, $value); + continue; + } + + foreach (array_values($value) as $num => &$element) + { + $statement->bindParam(':' . $key . $num, $element); + } + } + + $statement->execute(); + return $statement; } - catch (Exception $e) + catch (PDOException $e) { - $clean_sql = implode("\n", array_map('trim', explode("\n", $db_string))); - throw new UnexpectedValueException($this->error() . '
' . $clean_sql); + ob_start(); + + $debug = ob_get_clean(); + + throw new Exception($e->getMessage() . "\n" . var_export($e->errorInfo, true) . "\n" . var_export($db_values, true)); } - - return $return; - } - - /** - * Escapes and quotes a string just like db_query, but does not execute the query. - * Useful for debugging purposes. - */ - public function quote($db_string, $db_values = []) - { - // Please, just use new style queries. - if (strpos($db_string, '\'') !== false) - throw new UnexpectedValueException('Hack attempt!', 'Illegal character (\') used in query.'); - - // Save some values for use in the callback function. - $this->db_callback = [$db_values, $this->connection]; - - // Insert the values passed to this function. - $db_string = preg_replace_callback('~{([a-z_]+)(?::([a-zA-Z0-9_-]+))?}~', [&$this, 'replacement_callback'], $db_string); - - // Save some memory. - $this->db_callback = []; - - return $db_string; } /** @@ -356,11 +223,11 @@ class Database { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if (!$res || $this->rowCount($res) === 0) return null; - $object = $this->fetch_object($res, $class); - $this->free_result($res); + $object = $this->fetchObject($res, $class); + $this->free($res); return $object; } @@ -372,14 +239,14 @@ class Database { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if (!$res || $this->rowCount($res) === 0) return []; $rows = []; - while ($object = $this->fetch_object($res, $class)) + while ($object = $this->fetchObject($res, $class)) $rows[] = $object; - $this->free_result($res); + $this->free($res); return $rows; } @@ -387,15 +254,15 @@ class Database /** * Executes a query, returning an array of all the rows it returns. */ - public function queryRow($db_string, $db_values = []) + public function queryRow($db_string, array $db_values = []) { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if ($this->rowCount($res) === 0) return []; - $row = $this->fetch_row($res); - $this->free_result($res); + $row = $this->fetchNum($res); + $this->free($res); return $row; } @@ -403,18 +270,18 @@ class Database /** * Executes a query, returning an array of all the rows it returns. */ - public function queryRows($db_string, $db_values = []) + public function queryRows($db_string, array $db_values = []) { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if ($this->rowCount($res) === 0) return []; $rows = []; - while ($row = $this->fetch_row($res)) + while ($row = $this->fetchNum($res)) $rows[] = $row; - $this->free_result($res); + $this->free($res); return $rows; } @@ -422,18 +289,18 @@ class Database /** * Executes a query, returning an array of all the rows it returns. */ - public function queryPair($db_string, $db_values = []) + public function queryPair($db_string, array $db_values = []) { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if ($this->rowCount($res) === 0) return []; $rows = []; - while ($row = $this->fetch_row($res)) + while ($row = $this->fetchNum($res)) $rows[$row[0]] = $row[1]; - $this->free_result($res); + $this->free($res); return $rows; } @@ -441,21 +308,21 @@ class Database /** * Executes a query, returning an array of all the rows it returns. */ - public function queryPairs($db_string, $db_values = []) + public function queryPairs($db_string, $db_values = array()) { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if (!$res || $this->rowCount($res) === 0) return []; $rows = []; - while ($row = $this->fetch_assoc($res)) + while ($row = $this->fetchAssoc($res)) { $key_value = reset($row); $rows[$key_value] = $row; } - $this->free_result($res); + $this->free($res); return $rows; } @@ -463,15 +330,15 @@ class Database /** * Executes a query, returning an associative array of all the rows it returns. */ - public function queryAssoc($db_string, $db_values = []) + public function queryAssoc($db_string, array $db_values = []) { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if ($this->rowCount($res) === 0) return []; - $row = $this->fetch_assoc($res); - $this->free_result($res); + $row = $this->fetchAssoc($res); + $this->free($res); return $row; } @@ -479,18 +346,18 @@ class Database /** * Executes a query, returning an associative array of all the rows it returns. */ - public function queryAssocs($db_string, $db_values = [], $connection = null) + public function queryAssocs($db_string, array $db_values = []) { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if ($this->rowCount($res) === 0) return []; $rows = []; - while ($row = $this->fetch_assoc($res)) + while ($row = $this->fetchAssoc($res)) $rows[] = $row; - $this->free_result($res); + $this->free($res); return $rows; } @@ -498,16 +365,16 @@ class Database /** * Executes a query, returning the first value of the first row. */ - public function queryValue($db_string, $db_values = []) + public function queryValue($db_string, array $db_values = []) { $res = $this->query($db_string, $db_values); // If this happens, you're doing it wrong. - if (!$res || $this->num_rows($res) == 0) + if ($this->rowCount($res) === 0) return null; - list($value) = $this->fetch_row($res); - $this->free_result($res); + list($value) = $this->fetchNum($res); + $this->free($res); return $value; } @@ -515,18 +382,18 @@ class Database /** * Executes a query, returning an array of the first value of each row. */ - public function queryValues($db_string, $db_values = []) + public function queryValues($db_string, array $db_values = []) { $res = $this->query($db_string, $db_values); - if (!$res || $this->num_rows($res) == 0) + if ($this->rowCount($res) === 0) return []; $rows = []; - while ($row = $this->fetch_row($res)) + while ($row = $this->fetchNum($res)) $rows[] = $row[0]; - $this->free_result($res); + $this->free($res); return $rows; } @@ -544,35 +411,45 @@ class Database if (!is_array($data[array_rand($data)])) $data = [$data]; - // Create the mold for a single row insert. - $insertData = '('; - foreach ($columns as $columnName => $type) - { - // Are we restricting the length? - if (strpos($type, 'string-') !== false) - $insertData .= sprintf('SUBSTRING({string:%1$s}, 1, ' . substr($type, 7) . '), ', $columnName); - else - $insertData .= sprintf('{%1$s:%2$s}, ', $type, $columnName); - } - $insertData = substr($insertData, 0, -2) . ')'; - - // Create an array consisting of only the columns. - $indexed_columns = array_keys($columns); - - // Here's where the variables are injected to the query. - $insertRows = []; - foreach ($data as $dataRow) - $insertRows[] = $this->quote($insertData, array_combine($indexed_columns, $dataRow)); - // Determine the method of insertion. - $queryTitle = $method === 'replace' ? 'REPLACE' : ($method === 'ignore' ? 'INSERT IGNORE' : 'INSERT'); + $method = $method == 'replace' ? 'REPLACE' : ($method == 'ignore' ? 'INSERT IGNORE' : 'INSERT'); - // Do the insert. - return $this->query(' - ' . $queryTitle . ' INTO ' . $table . ' (`' . implode('`, `', $indexed_columns) . '`) - VALUES - ' . implode(', - ', $insertRows), - ['security_override' => true]); + // What columns are we inserting? + $columns = array_keys($data[0]); + + // Start building the query. + $db_string = $method . ' INTO ' . $table . ' (' . implode(',', $columns) . ') VALUES '; + + // Create the mold for a single row insert. + $placeholders = '(' . substr(str_repeat('?, ', count($columns)), 0, -2) . '), '; + + // Append it for every row we're to insert. + $values = []; + foreach ($data as $row) + { + $values = array_merge($values, array_values($row)); + $db_string .= $placeholders; + } + + // Get rid of the tailing comma. + $db_string = substr($db_string, 0, -2); + + // Prepare for your impending demise! + $statement = $this->connection->prepare($db_string); + + // Bind parameters... the hard way, due to a limit/offset hack. + foreach ($values as $key => $value) + $statement->bindValue($key + 1, $values[$key]); + + // Handle errors. + try + { + $statement->execute(); + return $statement; + } + catch (PDOException $e) + { + throw new Exception($e->getMessage() . '

' . $db_string . '

' . print_r($values, true)); + } } } diff --git a/models/ErrorLog.php b/models/ErrorLog.php index f406ae3..cafa862 100644 --- a/models/ErrorLog.php +++ b/models/ErrorLog.php @@ -17,8 +17,8 @@ class ErrorLog INSERT INTO log_errors (id_user, message, debug_info, file, line, request_uri, time, ip_address) VALUES - ({int:id_user}, {string:message}, {string:debug_info}, {string:file}, {int:line}, - {string:request_uri}, CURRENT_TIMESTAMP, {string:ip_address})', + (:id_user, :message, :debug_info, :file, :line, + :request_uri, CURRENT_TIMESTAMP, :ip_address)', $data); } @@ -37,14 +37,14 @@ class ErrorLog public static function getOffset($offset, $limit, $order, $direction) { assert(in_array($order, ['id_entry', 'file', 'line', 'time', 'ipaddress', 'id_user'])); + $order = $order . ($direction === 'up' ? ' ASC' : ' DESC'); return Registry::get('db')->queryAssocs(' SELECT * FROM log_errors - ORDER BY {raw:order} - LIMIT {int:offset}, {int:limit}', + ORDER BY ' . $order . ' + LIMIT :offset, :limit', [ - 'order' => $order . ($direction === 'up' ? ' ASC' : ' DESC'), 'offset' => $offset, 'limit' => $limit, ]); diff --git a/models/Image.php b/models/Image.php index 9348467..9748d11 100644 --- a/models/Image.php +++ b/models/Image.php @@ -165,7 +165,7 @@ class Image extends Asset return Registry::get('db')->query(' DELETE FROM assets_thumbs - WHERE id_asset = {int:id_asset}', + WHERE id_asset = :id_asset', ['id_asset' => $this->id_asset]); } @@ -183,9 +183,9 @@ class Image extends Asset return Registry::get('db')->query(' DELETE FROM assets_thumbs - WHERE id_asset = {int:id_asset} AND - width = {int:width} AND - height = {int:height}', + WHERE id_asset = :id_asset AND + width = :width AND + height = :height', [ 'height' => $height, 'id_asset' => $this->id_asset, diff --git a/models/Member.php b/models/Member.php index 2df8198..36ccacd 100644 --- a/models/Member.php +++ b/models/Member.php @@ -23,7 +23,7 @@ class Member extends User return Registry::get('db')->queryObject(static::class, ' SELECT * FROM users - WHERE emailaddress = {string:email_address}', + WHERE emailaddress = :email_address', ['email_address' => $email_address]); } @@ -32,7 +32,7 @@ class Member extends User $row = Registry::get('db')->queryAssoc(' SELECT * FROM users - WHERE id_user = {int:id_user}', + WHERE id_user = :id_user', [ 'id_user' => $id_user, ]); @@ -49,7 +49,7 @@ class Member extends User $row = Registry::get('db')->queryAssoc(' SELECT * FROM users - WHERE slug = {string:slug}', + WHERE slug = :slug', [ 'slug' => $slug, ]); @@ -97,7 +97,7 @@ class Member extends User if (!$bool) return false; - $new_user['id_user'] = $db->insert_id(); + $new_user['id_user'] = $db->insertId(); $member = new Member($new_user); return $member; @@ -125,14 +125,14 @@ class Member extends User return Registry::get('db')->query(' UPDATE users SET - first_name = {string:first_name}, - surname = {string:surname}, - slug = {string:slug}, - emailaddress = {string:emailaddress}, - password_hash = {string:password_hash}, - is_admin = {int:is_admin} - WHERE id_user = {int:id_user}', - $params); + first_name = :first_name, + surname = :surname, + slug = :slug, + emailaddress = :emailaddress, + password_hash = :password_hash, + is_admin = :is_admin + WHERE id_user = :id_user', + get_object_vars($this)); } /** @@ -143,7 +143,7 @@ class Member extends User { return Registry::get('db')->query(' DELETE FROM users - WHERE id_user = {int:id_user}', + WHERE id_user = :id_user', ['id_user' => $this->id_user]); } @@ -158,7 +158,7 @@ class Member extends User $res = Registry::get('db')->queryValue(' SELECT id_user FROM users - WHERE emailaddress = {string:emailaddress}', + WHERE emailaddress = :emailaddress', [ 'emailaddress' => $emailaddress, ]); @@ -174,9 +174,9 @@ class Member extends User return Registry::get('db')->query(' UPDATE users SET - last_action_time = {int:now}, - ip_address = {string:ip} - WHERE id_user = {int:id}', + last_action_time = :now, + ip_address = :ip + WHERE id_user = :id', [ 'now' => time(), 'id' => $this->id_user, @@ -199,14 +199,14 @@ class Member extends User public static function getOffset($offset, $limit, $order, $direction) { assert(in_array($order, ['id_user', 'surname', 'first_name', 'slug', 'emailaddress', 'last_action_time', 'ip_address', 'is_admin'])); + $order = $order . ($direction === 'up' ? ' ASC' : ' DESC'); return Registry::get('db')->queryAssocs(' SELECT * FROM users - ORDER BY {raw:order} - LIMIT {int:offset}, {int:limit}', + ORDER BY ' . $order . ' + LIMIT :offset, :limit', [ - 'order' => $order . ($direction === 'up' ? ' ASC' : ' DESC'), 'offset' => $offset, 'limit' => $limit, ]); @@ -221,7 +221,7 @@ class Member extends User public static function getMemberMap() { return Registry::get('db')->queryPair(' - SELECT id_user, CONCAT(first_name, {string:blank}, surname) AS full_name + SELECT id_user, CONCAT(first_name, :blank, surname) AS full_name FROM users ORDER BY first_name, surname', [ diff --git a/models/Setting.php b/models/Setting.php index 6171c6f..9bdf062 100644 --- a/models/Setting.php +++ b/models/Setting.php @@ -21,7 +21,7 @@ class Setting REPLACE INTO settings (id_user, variable, value, time_set) VALUES - ({int:id_user}, {string:key}, {string:value}, CURRENT_TIMESTAMP())', + (:id_user, :key, :value, CURRENT_TIMESTAMP())', [ 'id_user' => $id_user, 'key' => $key, @@ -45,7 +45,7 @@ class Setting $value = Registry::get('db')->queryValue(' SELECT value FROM settings - WHERE id_user = {int:id_user} AND variable = {string:key}', + WHERE id_user = :id_user AND variable = :key', [ 'id_user' => $id_user, 'key' => $key, @@ -63,11 +63,30 @@ class Setting public static function remove($key, $id_user = null) { - $id_user = Registry::get('user')->getUserId(); + // User setting or global setting? + if ($id_user === null) + $id_user = Registry::get('user')->getUserId(); + + $pairs = Registry::get('db')->queryPair(' + SELECT variable, value + FROM settings + WHERE id_user = :id_user', + [ + 'id_user' => $id_user, + ]); + + return $pairs; + } + + public static function remove($key, $id_user = 0) + { + // User setting or global setting? + if ($id_user === null) + $id_user = Registry::get('user')->getUserId(); if (Registry::get('db')->query(' DELETE FROM settings - WHERE id_user = {int:id_user} AND variable = {string:key}', + WHERE id_user = :id_user AND variable = :key', [ 'id_user' => $id_user, 'key' => $key, diff --git a/models/Tag.php b/models/Tag.php index dac43fd..4e00d55 100644 --- a/models/Tag.php +++ b/models/Tag.php @@ -36,7 +36,7 @@ class Tag $row = $db->queryAssoc(' SELECT * FROM tags - WHERE id_tag = {int:id_tag}', + WHERE id_tag = :id_tag', [ 'id_tag' => $id_tag, ]); @@ -55,7 +55,7 @@ class Tag $row = $db->queryAssoc(' SELECT * FROM tags - WHERE slug = {string:slug}', + WHERE slug = :slug', [ 'slug' => $slug, ]); @@ -73,7 +73,7 @@ class Tag SELECT * FROM tags ORDER BY ' . ($limit > 0 ? 'count - LIMIT {int:limit}' : 'tag'), + LIMIT :limit' : 'tag'), [ 'limit' => $limit, ]); @@ -107,14 +107,14 @@ class Tag $res = $db->query(' SELECT * FROM tags - WHERE id_user_owner = {int:id_user_owner} + WHERE id_user_owner = :id_user_owner ORDER BY tag', [ 'id_user_owner' => $id_user_owner, ]); $objects = []; - while ($row = $db->fetch_assoc($res)) + while ($row = $db->fetchAssoc($res)) $objects[$row['id_tag']] = new Tag($row); return $objects; @@ -125,9 +125,9 @@ class Tag $rows = Registry::get('db')->queryAssocs(' SELECT * FROM tags - WHERE id_parent = {int:id_parent} AND kind = {string:kind} + WHERE id_parent = :id_parent AND kind = :kind ORDER BY tag ASC - LIMIT {int:offset}, {int:limit}', + LIMIT :offset, :limit', [ 'id_parent' => $id_parent, 'kind' => 'Album', @@ -153,7 +153,7 @@ class Tag FROM assets_tags AS at LEFT JOIN assets AS a ON at.id_asset = a.id_asset LEFT JOIN users AS u ON a.id_user_uploaded = u.id_user - WHERE at.id_tag = {int:id_tag} + WHERE at.id_tag = :id_tag GROUP BY a.id_user_uploaded ORDER BY u.first_name, u.surname', [ @@ -166,9 +166,9 @@ class Tag $rows = Registry::get('db')->queryAssocs(' SELECT * FROM tags - WHERE id_parent = {int:id_parent} AND kind = {string:kind} + WHERE id_parent = :id_parent AND kind = :kind ORDER BY tag ASC - LIMIT {int:offset}, {int:limit}', + LIMIT :offset, :limit', [ 'id_parent' => $id_parent, 'kind' => 'Person', @@ -195,7 +195,7 @@ class Tag WHERE id_tag IN( SELECT id_tag FROM assets_tags - WHERE id_asset = {int:id_asset} + WHERE id_asset = :id_asset ) ORDER BY count DESC', [ @@ -225,7 +225,7 @@ class Tag WHERE id_tag IN( SELECT id_tag FROM posts_tags - WHERE id_post = {int:id_post} + WHERE id_post = :id_post ) ORDER BY count DESC', [ @@ -255,7 +255,7 @@ class Tag FROM `assets_tags` AS at WHERE at.id_tag = t.id_tag )' . (!empty($id_tags) ? ' - WHERE t.id_tag IN({array_int:id_tags})' : ''), + WHERE t.id_tag IN(@id_tags)' : ''), ['id_tags' => $id_tags]); } @@ -276,14 +276,14 @@ class Tag INSERT IGNORE INTO tags (id_parent, tag, slug, kind, description, count) VALUES - ({int:id_parent}, {string:tag}, {string:slug}, {string:kind}, {string:description}, {int:count}) + (:id_parent, :tag, :slug, :kind, :description, :count) ON DUPLICATE KEY UPDATE count = count + 1', $data); if (!$res) throw new Exception('Could not create the requested tag.'); - $data['id_tag'] = $db->insert_id(); + $data['id_tag'] = $db->insertId(); return $return_format === 'object' ? new Tag($data) : $data; } @@ -297,14 +297,15 @@ class Tag return Registry::get('db')->query(' UPDATE tags SET - id_parent = {int:id_parent}, - id_asset_thumb = {int:id_asset_thumb},' . (isset($this->id_user_owner) ? ' - id_user_owner = {int:id_user_owner},' : '') . ' - tag = {string:tag}, - slug = {string:slug}, - description = {string:description}, - count = {int:count} - WHERE id_tag = {int:id_tag}', + id_parent = :id_parent, + id_asset_thumb = :id_asset_thumb,' . (isset($this->id_user_owner) ? ' + id_user_owner = :id_user_owner,' : '') . ' + tag = :tag, + slug = :slug, + kind = :kind, + description = :description, + count = :count + WHERE id_tag = :id_tag', get_object_vars($this)); } @@ -312,9 +313,10 @@ class Tag { $db = Registry::get('db'); + // Unlink any tagged assets $res = $db->query(' DELETE FROM assets_tags - WHERE id_tag = {int:id_tag}', + WHERE id_tag = :id_tag', [ 'id_tag' => $this->id_tag, ]); @@ -322,9 +324,10 @@ class Tag if (!$res) return false; + // Delete the actual tag return $db->query(' DELETE FROM tags - WHERE id_tag = {int:id_tag}', + WHERE id_tag = :id_tag', [ 'id_tag' => $this->id_tag, ]); @@ -336,15 +339,15 @@ class Tag $new_id = $db->queryValue(' SELECT MAX(id_asset) as new_id FROM assets_tags - WHERE id_tag = {int:id_tag}', + WHERE id_tag = :id_tag', [ 'id_tag' => $this->id_tag, ]); return $db->query(' UPDATE tags - SET id_asset_thumb = {int:new_id} - WHERE id_tag = {int:id_tag}', + SET id_asset_thumb = :new_id + WHERE id_tag = :id_tag', [ 'new_id' => $new_id ?? 0, 'id_tag' => $this->id_tag, @@ -359,7 +362,7 @@ class Tag return Registry::get('db')->queryPair(' SELECT id_tag, tag FROM tags - WHERE LOWER(tag) LIKE {string:tokens} + WHERE LOWER(tag) LIKE :tokens ORDER BY tag ASC', ['tokens' => '%' . strtolower(implode('%', $tokens)) . '%']); } @@ -389,7 +392,7 @@ class Tag return Registry::get('db')->queryPair(' SELECT id_tag, tag FROM tags - WHERE tag = {string:tag}', + WHERE tag = :tag', ['tag' => $tag]); } @@ -401,7 +404,7 @@ class Tag return Registry::get('db')->queryValue(' SELECT id_tag FROM tags - WHERE slug = {string:slug}', + WHERE slug = :slug', ['slug' => $slug]); } @@ -410,7 +413,7 @@ class Tag return Registry::get('db')->queryPair(' SELECT tag, id_tag FROM tags - WHERE tag IN ({array_string:tags})', + WHERE tag IN (:tags)', ['tags' => $tags]); } @@ -422,7 +425,8 @@ class Tag if (empty($kind)) $kind = 'Album'; - $where[] = 'kind {raw:operator} {string:kind}'; + $operator = $isAlbum ? '=' : '!='; + $where[] = 'kind ' . $operator . ' :kind'; $where = implode(' AND ', $where); return Registry::get('db')->queryValue(' @@ -431,32 +435,32 @@ class Tag WHERE ' . $where, [ 'kind' => $kind, - 'operator' => $isAlbum ? '=' : '!=', ]); } public static function getOffset($offset, $limit, $order, $direction, $isAlbum = false) { assert(in_array($order, ['id_tag', 'tag', 'slug', 'count'])); + $order = $order . ($direction === 'up' ? ' ASC' : ' DESC'); + + $operator = $isAlbum ? '=' : '!='; $db = Registry::get('db'); $res = $db->query(' SELECT t.*, u.id_user, u.first_name, u.surname FROM tags AS t LEFT JOIN users AS u ON t.id_user_owner = u.id_user - WHERE kind {raw:operator} {string:album} - ORDER BY id_parent, {raw:order} - LIMIT {int:offset}, {int:limit}', + WHERE kind ' . $operator . ' :album + ORDER BY id_parent, ' . $order . ' + LIMIT :offset, :limit', [ - 'order' => $order . ($direction === 'up' ? ' ASC' : ' DESC'), 'offset' => $offset, 'limit' => $limit, 'album' => 'Album', - 'operator' => $isAlbum ? '=' : '!=', ]); $albums_by_parent = []; - while ($row = $db->fetch_assoc($res)) + while ($row = $db->fetchAssoc($res)) { if (!isset($albums_by_parent[$row['id_parent']])) $albums_by_parent[$row['id_parent']] = []; From 9989ba1fa70b2ff77b451c11957722c459d5b689 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Tue, 13 May 2025 22:51:12 +0200 Subject: [PATCH 2/6] CachedPDOIterator: introduce rewindable PDOStatement iterator --- models/CachedPDOIterator.php | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 models/CachedPDOIterator.php diff --git a/models/CachedPDOIterator.php b/models/CachedPDOIterator.php new file mode 100644 index 0000000..99297f8 --- /dev/null +++ b/models/CachedPDOIterator.php @@ -0,0 +1,56 @@ +index === null) + { + parent::rewind(); + } + $this->index = 0; + } + + public function current(): mixed + { + if ($this->offsetExists($this->index)) + { + return $this->offsetGet($this->index); + } + return parent::current(); + } + + public function key(): mixed + { + return $this->index; + } + + public function next(): void + { + $this->index++; + if (!$this->offsetExists($this->index)) + { + parent::next(); + } + } + + public function valid(): bool + { + return $this->offsetExists($this->index) || parent::valid(); + } +} From 4b26c677bb7b6182e921c844badc278cd985e978 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Tue, 13 May 2025 23:29:43 +0200 Subject: [PATCH 3/6] AssetIterator: rewrite to standard Iterator interface --- controllers/ViewPhotoAlbum.php | 6 -- controllers/ViewTimeline.php | 6 -- models/Asset.php | 2 +- models/AssetIterator.php | 116 +++++++++++++------------ models/PhotoMosaic.php | 22 ++--- templates/FeaturedThumbnailManager.php | 10 +-- 6 files changed, 75 insertions(+), 87 deletions(-) diff --git a/controllers/ViewPhotoAlbum.php b/controllers/ViewPhotoAlbum.php index 1d2272a..a491005 100644 --- a/controllers/ViewPhotoAlbum.php +++ b/controllers/ViewPhotoAlbum.php @@ -289,10 +289,4 @@ class ViewPhotoAlbum extends HTMLController $description = !empty($tag->description) ? $tag->description : ''; return new AlbumHeaderBox($tag->tag, $description, $back_link, $back_link_title); } - - public function __destruct() - { - if (isset($this->iterator)) - $this->iterator->clean(); - } } diff --git a/controllers/ViewTimeline.php b/controllers/ViewTimeline.php index d5dd9d3..33d933c 100644 --- a/controllers/ViewTimeline.php +++ b/controllers/ViewTimeline.php @@ -54,10 +54,4 @@ class ViewTimeline extends HTMLController // Set the canonical url. $this->page->setCanonicalUrl(BASEURL . '/timeline/'); } - - public function __destruct() - { - if (isset($this->iterator)) - $this->iterator->clean(); - } } diff --git a/models/Asset.php b/models/Asset.php index e8cdedd..a4f8353 100644 --- a/models/Asset.php +++ b/models/Asset.php @@ -24,7 +24,7 @@ class Asset protected $tags; protected $thumbnails; - protected function __construct(array $data) + public function __construct(array $data) { foreach ($data as $attribute => $value) { diff --git a/models/AssetIterator.php b/models/AssetIterator.php index 5690fb6..9088719 100644 --- a/models/AssetIterator.php +++ b/models/AssetIterator.php @@ -1,42 +1,50 @@ db = Registry::get('db'); $this->direction = $direction; - $this->res_assets = $res_assets; - $this->res_meta = $res_meta; - $this->res_thumbs = $res_thumbs; $this->return_format = $return_format; + $this->rowCount = $stmt_assets->rowCount(); + + $this->assets_iterator = new CachedPDOIterator($stmt_assets); + $this->assets_iterator->rewind(); + + $this->meta_iterator = new CachedPDOIterator($stmt_meta); + $this->thumbs_iterator = new CachedPDOIterator($stmt_thumbs); } - public function next() + public static function all() { - $row = $this->db->fetchAssoc($this->res_assets); + return self::getByOptions(); + } - // No more rows? + public function current(): mixed + { + $row = $this->assets_iterator->current(); if (!$row) - return false; + return $row; - // Looks up metadata. + // Collect metadata $row['meta'] = []; - while ($meta = $this->db->fetchAssoc($this->res_meta)) + $this->meta_iterator->rewind(); + foreach ($this->meta_iterator as $meta) { if ($meta['id_asset'] != $row['id_asset']) continue; @@ -44,54 +52,23 @@ class AssetIterator extends Asset $row['meta'][$meta['variable']] = $meta['value']; } - // Reset internal pointer for next asset. - $this->db->data_seek($this->res_meta, 0); - - // Looks up thumbnails. + // Collect thumbnails $row['thumbnails'] = []; - while ($thumbs = $this->db->fetchAssoc($this->res_thumbs)) + $this->thumbs_iterator->rewind(); + foreach ($this->thumbs_iterator as $thumb) { - if ($thumbs['id_asset'] != $row['id_asset']) + if ($thumb['id_asset'] != $row['id_asset']) continue; - $row['thumbnails'][$thumbs['selector']] = $thumbs['filename']; + $row['thumbnails'][$thumb['selector']] = $thumb['filename']; } - // Reset internal pointer for next asset. - $this->db->data_seek($this->res_thumbs, 0); - if ($this->return_format === 'object') return new Asset($row); else return $row; } - public function reset() - { - $this->db->data_seek($this->res_assets, 0); - $this->db->data_seek($this->res_meta, 0); - $this->db->data_seek($this->res_thumbs, 0); - } - - public function clean() - { - if (!$this->res_assets) - return; - - $this->db->free($this->res_assets); - $this->res_assets = null; - } - - public function num() - { - return $this->db->rowCount($this->res_assets); - } - - public static function all() - { - return self::getByOptions(); - } - public static function getByOptions(array $options = [], $return_count = false, $return_format = 'object') { $params = [ @@ -210,13 +187,38 @@ class AssetIterator extends Asset return $iterator; } - public function isAscending() + public function key(): mixed + { + return $this->assets_iterator->key(); + } + + public function isAscending(): bool { return $this->direction === 'asc'; } - public function isDescending() + public function isDescending(): bool { return $this->direction === 'desc'; } + + public function next(): void + { + $this->assets_iterator->next(); + } + + public function num(): int + { + return $this->rowCount; + } + + public function rewind(): void + { + $this->assets_iterator->rewind(); + } + + public function valid(): bool + { + return $this->assets_iterator->valid(); + } } diff --git a/models/PhotoMosaic.php b/models/PhotoMosaic.php index 8326e1d..4859ef4 100644 --- a/models/PhotoMosaic.php +++ b/models/PhotoMosaic.php @@ -8,11 +8,11 @@ class PhotoMosaic { - private $descending; + private bool $descending; private AssetIterator $iterator; - private $layouts; - private $processedImages = 0; - private $queue = []; + private array $layouts; + private int $processedImages = 0; + private array $queue = []; const IMAGE_MASK_ALL = Image::TYPE_PORTRAIT | Image::TYPE_LANDSCAPE | Image::TYPE_PANORAMA; const NUM_DAYS_CUTOFF = 7; @@ -25,11 +25,6 @@ class PhotoMosaic $this->descending = $iterator->isDescending(); } - public function __destruct() - { - $this->iterator->clean(); - } - private function availableLayouts() { static $layouts = [ @@ -86,9 +81,14 @@ class PhotoMosaic } } - // Check whatever's next up! - while (($asset = $this->iterator->next()) && ($image = $asset->getImage())) + // Check whatever's up next! + // NB: not is not a `foreach` so as to not reset the iterator implicitly + while ($this->iterator->valid()) { + $asset = $this->iterator->current(); + $image = $asset->getImage(); + $this->iterator->next(); + // Give up on the recordset once dates are too far apart if (isset($refDate) && abs(self::daysApart($image->getDateCaptured(), $refDate)) > self::NUM_DAYS_CUTOFF) { diff --git a/templates/FeaturedThumbnailManager.php b/templates/FeaturedThumbnailManager.php index ffbe1c0..49e58de 100644 --- a/templates/FeaturedThumbnailManager.php +++ b/templates/FeaturedThumbnailManager.php @@ -8,12 +8,12 @@ class FeaturedThumbnailManager extends SubTemplate { - private $assets; + private $iterator; private $currentThumbnailId; - public function __construct(AssetIterator $assets, $currentThumbnailId) + public function __construct(AssetIterator $iterator, $currentThumbnailId) { - $this->assets = $assets; + $this->iterator = $iterator; $this->currentThumbnailId = $currentThumbnailId; } @@ -25,7 +25,7 @@ class FeaturedThumbnailManager extends SubTemplate

Select thumbnail

    '; - while ($asset = $this->assets->next()) + foreach ($this->iterator as $asset) { $image = $asset->getImage(); echo ' @@ -36,8 +36,6 @@ class FeaturedThumbnailManager extends SubTemplate '; } - $this->assets->clean(); - echo '
From 219260c57fe680468ca19a24910d4a8b1cd05634 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Fri, 16 May 2025 11:53:59 +0200 Subject: [PATCH 4/6] Member: set empty reset key for new users --- models/Member.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/Member.php b/models/Member.php index 36ccacd..0417a0e 100644 --- a/models/Member.php +++ b/models/Member.php @@ -77,6 +77,7 @@ class Member extends User 'creation_time' => time(), 'ip_address' => isset($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : '', 'is_admin' => empty($data['is_admin']) ? 0 : 1, + 'reset_key' => '', ]; if ($error) @@ -92,6 +93,7 @@ class Member extends User 'creation_time' => 'int', 'ip_address' => 'string-45', 'is_admin' => 'int', + 'reset_key' => 'string-16' ], $new_user, ['id_user']); if (!$bool) From 3dfda456816cab93a8d8518e0891d2b403ac8509 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Fri, 16 May 2025 11:54:05 +0200 Subject: [PATCH 5/6] GenericTable: better handling of null values for timestamps --- controllers/ManageUsers.php | 1 + models/GenericTable.php | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/controllers/ManageUsers.php b/controllers/ManageUsers.php index 8473596..9e80cb9 100644 --- a/controllers/ManageUsers.php +++ b/controllers/ManageUsers.php @@ -62,6 +62,7 @@ class ManageUsers extends HTMLController 'type' => 'timestamp', 'pattern' => 'long', 'value' => 'last_action_time', + 'if_null' => 'n/a', ], 'header' => 'Last activity', 'is_sortable' => true, diff --git a/models/GenericTable.php b/models/GenericTable.php index ec76a1a..7291461 100644 --- a/models/GenericTable.php +++ b/models/GenericTable.php @@ -226,8 +226,8 @@ class GenericTable else $pattern = $options['pattern']; - assert(isset($rowData[$options['value']])); - if (!is_numeric($rowData[$options['value']])) + assert(array_key_exists($options['value'], $rowData)); + if (isset($rowData[$options['value']]) && !is_numeric($rowData[$options['value']])) $timestamp = strtotime($rowData[$options['value']]); else $timestamp = (int) $rowData[$options['value']]; From 26d8063c4506c9c51c7225ac261ba31bc573ee36 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Fri, 16 May 2025 11:57:07 +0200 Subject: [PATCH 6/6] Asset/Thumbnail: replace 'NULL' placeholder strings with actual null values --- models/Asset.php | 14 +++++++------- models/Thumbnail.php | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/models/Asset.php b/models/Asset.php index a4f8353..b008bf5 100644 --- a/models/Asset.php +++ b/models/Asset.php @@ -32,7 +32,7 @@ class Asset $this->$attribute = $value; } - if (isset($data['date_captured']) && $data['date_captured'] !== 'NULL' && !is_object($data['date_captured'])) + if (isset($data['date_captured']) && $data['date_captured'] !== null && !is_object($data['date_captured'])) $this->date_captured = new DateTime($data['date_captured']); } @@ -273,9 +273,9 @@ class Asset 'title' => $title, 'slug' => $slug, 'mimetype' => $mimetype, - 'image_width' => isset($image_width) ? $image_width : 'NULL', - 'image_height' => isset($image_height) ? $image_height : 'NULL', - 'date_captured' => isset($date_captured) ? $date_captured : 'NULL', + 'image_width' => isset($image_width) ? $image_width : null, + 'image_height' => isset($image_height) ? $image_height : null, + 'date_captured' => isset($date_captured) ? $date_captured : null, 'priority' => isset($priority) ? (int) $priority : 0, ]); @@ -504,9 +504,9 @@ class Asset [ 'id_asset' => $this->id_asset, 'mimetype' => $this->mimetype, - 'image_width' => isset($this->image_width) ? $this->image_width : 'NULL', - 'image_height' => isset($this->image_height) ? $this->image_height : 'NULL', - 'date_captured' => isset($this->date_captured) ? $this->date_captured : 'NULL', + 'image_width' => isset($this->image_width) ? $this->image_width : null, + 'image_height' => isset($this->image_height) ? $this->image_height : null, + 'date_captured' => isset($this->date_captured) ? $this->date_captured : null, 'priority' => $this->priority, ]); } diff --git a/models/Thumbnail.php b/models/Thumbnail.php index 8168ee2..2e9b3d6 100644 --- a/models/Thumbnail.php +++ b/models/Thumbnail.php @@ -335,7 +335,7 @@ class Thumbnail if ($success) { $thumb_selector = $this->width . 'x' . $this->height . $this->filename_suffix; - $this->thumbnails[$thumb_selector] = $filename !== 'NULL' ? $filename : null; + $this->thumbnails[$thumb_selector] = $filename ?? null; // For consistency, write new thumbnail filename to parent Image object. // TODO: there could still be an inconsistency if multiple objects exists for the same image asset. @@ -349,7 +349,7 @@ class Thumbnail private function markAsQueued() { - $this->updateDb('NULL'); + $this->updateDb(null); } private function markAsGenerated($filename)