From 7adee3c50d3f0c280458b703c2597fbfa0059fef Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 8 Jun 2023 00:05:48 +0700 Subject: [PATCH 1/3] [ticket/17142] Fix DBMS+cache related PHP warnings while installing PHPBB3-17142 --- phpBB/phpbb/cache/driver/base.php | 22 ++++++++++++++++ phpBB/phpbb/cache/driver/driver_interface.php | 9 +++++++ phpBB/phpbb/db/driver/driver.php | 21 ++++++++++++++++ phpBB/phpbb/db/driver/driver_interface.php | 9 +++++++ phpBB/phpbb/db/driver/mssql_odbc.php | 20 +++++++++------ phpBB/phpbb/db/driver/mssqlnative.php | 20 +++++++++------ phpBB/phpbb/db/driver/mysqli.php | 15 ++++++----- phpBB/phpbb/db/driver/oracle.php | 25 +++++++++++-------- phpBB/phpbb/db/driver/postgres.php | 21 ---------------- phpBB/phpbb/db/driver/sqlite3.php | 10 +++++--- 10 files changed, 115 insertions(+), 57 deletions(-) diff --git a/phpBB/phpbb/cache/driver/base.php b/phpBB/phpbb/cache/driver/base.php index 3eca521148..2ba2812875 100644 --- a/phpBB/phpbb/cache/driver/base.php +++ b/phpBB/phpbb/cache/driver/base.php @@ -115,6 +115,7 @@ abstract class base implements \phpbb\cache\driver\driver_interface */ function sql_exists($query_id) { + $query_id = $this->clean_query_id($query_id); return isset($this->sql_rowset[$query_id]); } @@ -123,6 +124,7 @@ abstract class base implements \phpbb\cache\driver\driver_interface */ function sql_fetchrow($query_id) { + $query_id = $this->clean_query_id($query_id); if ($this->sql_row_pointer[$query_id] < count($this->sql_rowset[$query_id])) { return $this->sql_rowset[$query_id][$this->sql_row_pointer[$query_id]++]; @@ -136,6 +138,7 @@ abstract class base implements \phpbb\cache\driver\driver_interface */ function sql_fetchfield($query_id, $field) { + $query_id = $this->clean_query_id($query_id); if ($this->sql_row_pointer[$query_id] < count($this->sql_rowset[$query_id])) { return (isset($this->sql_rowset[$query_id][$this->sql_row_pointer[$query_id]][$field])) ? $this->sql_rowset[$query_id][$this->sql_row_pointer[$query_id]++][$field] : false; @@ -149,6 +152,7 @@ abstract class base implements \phpbb\cache\driver\driver_interface */ function sql_rowseek($rownum, $query_id) { + $query_id = $this->clean_query_id($query_id); if ($rownum >= count($this->sql_rowset[$query_id])) { return false; @@ -163,6 +167,7 @@ abstract class base implements \phpbb\cache\driver\driver_interface */ function sql_freeresult($query_id) { + $query_id = $this->clean_query_id($query_id); if (!isset($this->sql_rowset[$query_id])) { return false; @@ -231,4 +236,21 @@ abstract class base implements \phpbb\cache\driver\driver_interface @rmdir($dir); } + + /** + * {@inheritDoc} + */ + public function clean_query_id($query_id) + { + // Some DBMS functions accept/return objects and/or resources instead of integer identifier + // Attempting to cast object to int will throw error, hence correctly handle all cases + if (is_resource($query_id)) + { + return function_exists('get_resource_id') ? get_resource_id($query_id) : (int) $query_id; + } + else + { + return is_object($query_id) ? spl_object_id($query_id) : $query_id; + } + } } diff --git a/phpBB/phpbb/cache/driver/driver_interface.php b/phpBB/phpbb/cache/driver/driver_interface.php index 9ac9ca0c59..087a812ca1 100644 --- a/phpBB/phpbb/cache/driver/driver_interface.php +++ b/phpBB/phpbb/cache/driver/driver_interface.php @@ -164,4 +164,13 @@ interface driver_interface * @return bool */ public function sql_freeresult($query_id); + + /** + * Ensure query ID can be used by cache + * + * @param object|resource|int|string $query_id Mixed type query id + * + * @return int|string Query id in string or integer format + */ + public function clean_query_id($query_id); } diff --git a/phpBB/phpbb/db/driver/driver.php b/phpBB/phpbb/db/driver/driver.php index 690cfbf1e9..f64847db83 100644 --- a/phpBB/phpbb/db/driver/driver.php +++ b/phpBB/phpbb/db/driver/driver.php @@ -1245,4 +1245,25 @@ abstract class driver implements driver_interface return $rows_total; } + + /** + * Ensure query ID can be used by cache + * + * @param resource|int|string $query_id Mixed type query id + * + * @return int|string Query id in string or integer format + */ + public function clean_query_id($query_id) + { + // Some DBMS functions accept/return objects and/or resources instead if identifiers + // Attempting to use objects/resources as array keys will throw error, hence correctly handle all cases + if (is_resource($query_id)) + { + return function_exists('get_resource_id') ? get_resource_id($query_id) : (int) $query_id; + } + else + { + return is_object($query_id) ? spl_object_id($query_id) : $query_id; + } + } } diff --git a/phpBB/phpbb/db/driver/driver_interface.php b/phpBB/phpbb/db/driver/driver_interface.php index 73aabaf4a0..290a830ee9 100644 --- a/phpBB/phpbb/db/driver/driver_interface.php +++ b/phpBB/phpbb/db/driver/driver_interface.php @@ -494,4 +494,13 @@ interface driver_interface * @return string Quoted version of $msg */ public function sql_quote($msg); + + /** + * Ensure query ID can be used by cache + * + * @param resource|int|string $query_id Mixed type query id + * + * @return int|string Query id in string or integer format + */ + public function clean_query_id($query_id); } diff --git a/phpBB/phpbb/db/driver/mssql_odbc.php b/phpBB/phpbb/db/driver/mssql_odbc.php index ac66d94d0d..cbc3a06a37 100644 --- a/phpBB/phpbb/db/driver/mssql_odbc.php +++ b/phpBB/phpbb/db/driver/mssql_odbc.php @@ -185,14 +185,16 @@ class mssql_odbc extends \phpbb\db\driver\mssql_base return false; } + $safe_query_id = $this->clean_query_id($this->query_result); + if ($cache && $cache_ttl) { - $this->open_queries[(int) $this->query_result] = $this->query_result; + $this->open_queries[$safe_query_id] = $this->query_result; $this->query_result = $cache->sql_save($this, $query, $this->query_result, $cache_ttl); } else if (strpos($query, 'SELECT') === 0) { - $this->open_queries[(int) $this->query_result] = $this->query_result; + $this->open_queries[$safe_query_id] = $this->query_result; } } else if ($this->debug_sql_explain) @@ -260,9 +262,10 @@ class mssql_odbc extends \phpbb\db\driver\mssql_base $query_id = $this->query_result; } - if ($cache && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_fetchrow($query_id); + return $cache->sql_fetchrow($safe_query_id); } return ($query_id) ? odbc_fetch_array($query_id) : false; @@ -301,14 +304,15 @@ class mssql_odbc extends \phpbb\db\driver\mssql_base $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_freeresult($query_id); + return $cache->sql_freeresult($safe_query_id); } - if (isset($this->open_queries[(int) $query_id])) + if (isset($this->open_queries[$safe_query_id])) { - unset($this->open_queries[(int) $query_id]); + unset($this->open_queries[$safe_query_id]); return odbc_free_result($query_id); } diff --git a/phpBB/phpbb/db/driver/mssqlnative.php b/phpBB/phpbb/db/driver/mssqlnative.php index 357047ace0..f806553ad5 100644 --- a/phpBB/phpbb/db/driver/mssqlnative.php +++ b/phpBB/phpbb/db/driver/mssqlnative.php @@ -159,14 +159,16 @@ class mssqlnative extends \phpbb\db\driver\mssql_base return false; } + $safe_query_id = $this->clean_query_id($this->query_result); + if ($cache && $cache_ttl) { - $this->open_queries[(int) $this->query_result] = $this->query_result; + $this->open_queries[$safe_query_id] = $this->query_result; $this->query_result = $cache->sql_save($this, $query, $this->query_result, $cache_ttl); } else if (strpos($query, 'SELECT') === 0) { - $this->open_queries[(int) $this->query_result] = $this->query_result; + $this->open_queries[$safe_query_id] = $this->query_result; } } else if ($this->debug_sql_explain) @@ -242,9 +244,10 @@ class mssqlnative extends \phpbb\db\driver\mssql_base $query_id = $this->query_result; } - if ($cache && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_fetchrow($query_id); + return $cache->sql_fetchrow($safe_query_id); } if (!$query_id) @@ -302,14 +305,15 @@ class mssqlnative extends \phpbb\db\driver\mssql_base $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_freeresult($query_id); + return $cache->sql_freeresult($safe_query_id); } - if (isset($this->open_queries[(int) $query_id])) + if (isset($this->open_queries[$safe_query_id])) { - unset($this->open_queries[(int) $query_id]); + unset($this->open_queries[$safe_query_id]); return sqlsrv_free_stmt($query_id); } diff --git a/phpBB/phpbb/db/driver/mysqli.php b/phpBB/phpbb/db/driver/mysqli.php index d7a929315b..d2e2569d8e 100644 --- a/phpBB/phpbb/db/driver/mysqli.php +++ b/phpBB/phpbb/db/driver/mysqli.php @@ -254,9 +254,10 @@ class mysqli extends \phpbb\db\driver\mysql_base $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_fetchrow($query_id); + return $cache->sql_fetchrow($safe_query_id); } if ($query_id) @@ -280,9 +281,10 @@ class mysqli extends \phpbb\db\driver\mysql_base $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_rowseek($rownum, $query_id); + return $cache->sql_rowseek($rownum, $safe_query_id); } return ($query_id) ? @mysqli_data_seek($query_id, $rownum) : false; @@ -308,9 +310,10 @@ class mysqli extends \phpbb\db\driver\mysql_base $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_freeresult($query_id); + return $cache->sql_freeresult($safe_query_id); } if (!$query_id) diff --git a/phpBB/phpbb/db/driver/oracle.php b/phpBB/phpbb/db/driver/oracle.php index dc5eb994ec..eb9fbfb07c 100644 --- a/phpBB/phpbb/db/driver/oracle.php +++ b/phpBB/phpbb/db/driver/oracle.php @@ -441,14 +441,16 @@ class oracle extends \phpbb\db\driver\driver return false; } + $safe_query_id = $this->clean_query_id($this->query_result); + if ($cache && $cache_ttl) { - $this->open_queries[(int) $this->query_result] = $this->query_result; + $this->open_queries[$safe_query_id] = $this->query_result; $this->query_result = $cache->sql_save($this, $query, $this->query_result, $cache_ttl); } else if (strpos($query, 'SELECT') === 0) { - $this->open_queries[(int) $this->query_result] = $this->query_result; + $this->open_queries[$safe_query_id] = $this->query_result; } } else if ($this->debug_sql_explain) @@ -496,9 +498,10 @@ class oracle extends \phpbb\db\driver\driver $query_id = $this->query_result; } - if ($cache && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_fetchrow($query_id); + return $cache->sql_fetchrow($safe_query_id); } if ($query_id) @@ -544,9 +547,10 @@ class oracle extends \phpbb\db\driver\driver $query_id = $this->query_result; } - if ($cache && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_rowseek($rownum, $query_id); + return $cache->sql_rowseek($rownum, $safe_query_id); } if (!$query_id) @@ -619,14 +623,15 @@ class oracle extends \phpbb\db\driver\driver $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_freeresult($query_id); + return $cache->sql_freeresult($safe_query_id); } - if (isset($this->open_queries[(int) $query_id])) + if (isset($this->open_queries[$safe_query_id])) { - unset($this->open_queries[(int) $query_id]); + unset($this->open_queries[$safe_query_id]); return oci_free_statement($query_id); } diff --git a/phpBB/phpbb/db/driver/postgres.php b/phpBB/phpbb/db/driver/postgres.php index 02ffa4e3e7..77eb915e81 100644 --- a/phpBB/phpbb/db/driver/postgres.php +++ b/phpBB/phpbb/db/driver/postgres.php @@ -547,25 +547,4 @@ class postgres extends \phpbb\db\driver\driver { return '"' . $msg . '"'; } - - /** - * Ensure query ID can be used by cache - * - * @param resource|int|string $query_id Mixed type query id - * - * @return int|string Query id in string or integer format - */ - private function clean_query_id($query_id) - { - // As of PHP 8.1 PgSQL functions accept/return \PgSQL\* objects instead of "pgsql *" resources - // Attempting to cast object to int will throw error, hence correctly handle all cases - if (is_resource($query_id)) - { - return function_exists('get_resource_id') ? get_resource_id($query_id) : (int) $query_id; - } - else - { - return is_object($query_id) ? spl_object_id($query_id) : $query_id; - } - } } diff --git a/phpBB/phpbb/db/driver/sqlite3.php b/phpBB/phpbb/db/driver/sqlite3.php index 93a1c36ee9..4d79573d86 100644 --- a/phpBB/phpbb/db/driver/sqlite3.php +++ b/phpBB/phpbb/db/driver/sqlite3.php @@ -233,9 +233,10 @@ class sqlite3 extends \phpbb\db\driver\driver $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_fetchrow($query_id); + return $cache->sql_fetchrow($safe_query_id); } return is_object($query_id) ? @$query_id->fetchArray(SQLITE3_ASSOC) : false; @@ -261,9 +262,10 @@ class sqlite3 extends \phpbb\db\driver\driver $query_id = $this->query_result; } - if ($cache && !is_object($query_id) && $cache->sql_exists($query_id)) + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && $cache->sql_exists($safe_query_id)) { - return $cache->sql_freeresult($query_id); + return $cache->sql_freeresult($safe_query_id); } if ($query_id) From d8d67b5f17ec676882529d54ebf4785aa4a0b00d Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 8 Jun 2023 01:13:18 +0700 Subject: [PATCH 2/3] [ticket/17142] Fix tests' mock cache class PHPBB3-17142 --- tests/mock/cache.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/mock/cache.php b/tests/mock/cache.php index 2306fd9009..cc50d0b81b 100644 --- a/tests/mock/cache.php +++ b/tests/mock/cache.php @@ -168,4 +168,8 @@ class phpbb_mock_cache implements \phpbb\cache\driver\driver_interface { return isset($this->data['_bots']) ? $this->data['_bots'] : array(); } + + public function clean_query_id($query_id) + { + } } From 1eb81e9b2a0fa9e2c8428f1aaec4962d669a9cea Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 8 Jun 2023 09:26:18 +0700 Subject: [PATCH 3/3] [ticket/17142] Fix class inheritance issues PHPBB3-17142 --- phpBB/phpbb/db/driver/driver.php | 6 +----- phpBB/phpbb/db/driver/factory.php | 8 ++++++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/db/driver/driver.php b/phpBB/phpbb/db/driver/driver.php index f64847db83..533a1dcf84 100644 --- a/phpBB/phpbb/db/driver/driver.php +++ b/phpBB/phpbb/db/driver/driver.php @@ -1247,11 +1247,7 @@ abstract class driver implements driver_interface } /** - * Ensure query ID can be used by cache - * - * @param resource|int|string $query_id Mixed type query id - * - * @return int|string Query id in string or integer format + * {@inheritDoc} */ public function clean_query_id($query_id) { diff --git a/phpBB/phpbb/db/driver/factory.php b/phpBB/phpbb/db/driver/factory.php index 2541c28481..4f2efcf57d 100644 --- a/phpBB/phpbb/db/driver/factory.php +++ b/phpBB/phpbb/db/driver/factory.php @@ -472,4 +472,12 @@ class factory implements driver_interface { return $this->get_driver()->sql_quote($msg); } + + /** + * {@inheritDoc} + */ + public function clean_query_id($query_id) + { + return $this->get_driver()->clean_query_id($query_id); + } }