From 645e662b110b0d5c246d32ac5368c3bf6eabf86b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 26 Mar 2021 22:54:19 +0100 Subject: [PATCH 1/6] [ticket/16740] Add method for ensuring resource is not passed to cache PHPBB3-16740 --- phpBB/phpbb/db/driver/postgres.php | 63 ++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/phpBB/phpbb/db/driver/postgres.php b/phpBB/phpbb/db/driver/postgres.php index 52a5b6b546..b004ecee30 100644 --- a/phpBB/phpbb/db/driver/postgres.php +++ b/phpBB/phpbb/db/driver/postgres.php @@ -277,9 +277,10 @@ class postgres 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); } return ($query_id) ? pg_fetch_assoc($query_id, null) : false; @@ -297,14 +298,47 @@ class postgres 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); } return ($query_id) ? @pg_result_seek($query_id, $rownum) : false; } + /** + * {@inheritDoc} + */ + function sql_fetchfield($field, $rownum = false, $query_id = false) + { + global $cache; + + if ($query_id === false) + { + $query_id = $this->query_result; + } + + if ($query_id) + { + if ($rownum !== false) + { + $this->sql_rowseek($rownum, $query_id); + } + + $safe_query_id = $this->clean_query_id($query_id); + if ($cache && !is_object($query_id) && $cache->sql_exists($safe_query_id)) + { + return $cache->sql_fetchfield($safe_query_id, $field); + } + + $row = $this->sql_fetchrow($query_id); + return (isset($row[$field])) ? $row[$field] : false; + } + + return false; + } + /** * {@inheritDoc} */ @@ -346,14 +380,15 @@ class postgres 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 && !is_object($query_id) && $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[(int) $safe_query_id])) { - unset($this->open_queries[(int) $query_id]); + unset($this->open_queries[(int) $safe_query_id]); return pg_free_result($query_id); } @@ -505,4 +540,16 @@ 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) + { + return is_resource($query_id) ? (int) $query_id : $query_id; + } } From 03824189e4c32c85ddbcdf98016f2ce25a26ac74 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 26 Mar 2021 22:56:57 +0100 Subject: [PATCH 2/6] [ticket/16740] Add more postgres tests to github actions PHPBB3-16740 --- .github/workflows/tests.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c72abf2f36..e14579d620 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -242,6 +242,16 @@ jobs: db: "postgres:12" - php: '7.1' db: "postgres:13" + - php: '7.2' + db: "postgres:13" + - php: '7.3' + db: "postgres:13" + - php: '7.4' + db: "postgres:13" + - php: '8.0' + db: "postgres:12" + - php: '8.0' + db: "postgres:13" name: PHP ${{ matrix.php }} - ${{ matrix.db }} From c99bfe116fef648b17796367abcbf4e2bc4b53b5 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 28 Mar 2021 09:52:55 +0200 Subject: [PATCH 3/6] [ticket/16740] Improve open queries handling & do not close freed connection PHPBB3-16740 --- phpBB/phpbb/db/driver/postgres.php | 9 +++++++-- tests/test_framework/phpbb_database_test_case.php | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/db/driver/postgres.php b/phpBB/phpbb/db/driver/postgres.php index b004ecee30..cd17f3294d 100644 --- a/phpBB/phpbb/db/driver/postgres.php +++ b/phpBB/phpbb/db/driver/postgres.php @@ -386,9 +386,9 @@ class postgres extends \phpbb\db\driver\driver return $cache->sql_freeresult($safe_query_id); } - if (isset($this->open_queries[(int) $safe_query_id])) + if (isset($this->open_queries[$safe_query_id])) { - unset($this->open_queries[(int) $safe_query_id]); + unset($this->open_queries[$safe_query_id]); return pg_free_result($query_id); } @@ -466,6 +466,11 @@ class postgres extends \phpbb\db\driver\driver */ function _sql_close() { + // Released resources are already closed, return true in this case + if (get_resource_type($this->db_connect_id) === 'Unknown') + { + return true; + } return @pg_close($this->db_connect_id); } diff --git a/tests/test_framework/phpbb_database_test_case.php b/tests/test_framework/phpbb_database_test_case.php index ebaf573753..c46bbe11c3 100644 --- a/tests/test_framework/phpbb_database_test_case.php +++ b/tests/test_framework/phpbb_database_test_case.php @@ -29,7 +29,7 @@ abstract class phpbb_database_test_case extends TestCase static protected $install_schema_file; - static protected $phpunit_version; + static protected $phpunit_version; public function __construct($name = NULL, array $data = [], $dataName = '') { From 1e6ed3f0f5b109fe75dfd16242946c4d59397ed4 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 28 Mar 2021 09:59:36 +0200 Subject: [PATCH 4/6] [ticket/16740] Disable PHP 8.1 tests PHPBB3-16740 --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e14579d620..e99074c3c5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -123,8 +123,8 @@ jobs: db: "mysql:8.0" - php: '8.0' db: "mysql:5.7" - - php: '8.1' - db: "mysql:5.7" + #- php: '8.1' + # db: "mysql:5.7" name: PHP ${{ matrix.php }} - ${{ matrix.db_alias != '' && matrix.db_alias || matrix.db }} From e8afa2901340ac8be496bbc3b0013432f92b541c Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 28 Mar 2021 11:16:11 +0200 Subject: [PATCH 5/6] [ticket/16740] Implement sql_table_exists for psql w/out debug spam Failed pg_query() results in PHP notices now ... PHPBB3-16740 --- phpBB/phpbb/db/tools/postgres.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/phpBB/phpbb/db/tools/postgres.php b/phpBB/phpbb/db/tools/postgres.php index 7cb024d4f6..573b40fe5d 100644 --- a/phpBB/phpbb/db/tools/postgres.php +++ b/phpBB/phpbb/db/tools/postgres.php @@ -96,6 +96,24 @@ class postgres extends tools return $tables; } + /** + * {@inheritDoc} + */ + function sql_table_exists($table_name) + { + $sql = "SELECT CAST(EXISTS( + SELECT FROM information_schema.tables + WHERE table_schema = 'public' + AND table_name = '" . $this->db->sql_escape($table_name) . "' + ) AS INTEGER)"; + $result = $this->db->sql_query_limit($sql, 1); + $row = $this->db->sql_fetchrow($result); + $table_exists = (booL) $row['exists']; + $this->db->sql_freeresult($result); + + return $table_exists; + } + /** * {@inheritDoc} */ From 4ce8224c2cbe7d8ddd01fa0b0f25bdb858d3ac1f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 31 Mar 2021 21:22:14 +0200 Subject: [PATCH 6/6] [ticket/16740] Use is_resource() to check whether connect id is still valid PHPBB3-16740 --- phpBB/phpbb/db/driver/postgres.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/db/driver/postgres.php b/phpBB/phpbb/db/driver/postgres.php index cd17f3294d..7541d1e6b6 100644 --- a/phpBB/phpbb/db/driver/postgres.php +++ b/phpBB/phpbb/db/driver/postgres.php @@ -467,7 +467,7 @@ class postgres extends \phpbb\db\driver\driver function _sql_close() { // Released resources are already closed, return true in this case - if (get_resource_type($this->db_connect_id) === 'Unknown') + if (!is_resource($this->db_connect_id)) { return true; }