From f5c5d7d1e65b5df9f982683dbcbd43321eeb4edd Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 23 May 2023 18:59:25 +0700 Subject: [PATCH 1/5] [ticket/17107] Fix viewonline page locations for posting modes PHPBB3-17107 --- .../default/container/services_content.yml | 1 + phpBB/phpbb/viewonline_helper.php | 50 +++++++- phpBB/viewonline.php | 8 +- tests/functional/viewonline_test.php | 118 ++++++++++++++++++ 4 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 tests/functional/viewonline_test.php diff --git a/phpBB/config/default/container/services_content.yml b/phpBB/config/default/container/services_content.yml index 6717c20337..8b240067e4 100644 --- a/phpBB/config/default/container/services_content.yml +++ b/phpBB/config/default/container/services_content.yml @@ -71,3 +71,4 @@ services: class: phpbb\viewonline_helper arguments: - '@filesystem' + - '@dbal.conn' diff --git a/phpBB/phpbb/viewonline_helper.php b/phpBB/phpbb/viewonline_helper.php index 89915f2228..9b8c4c50ea 100644 --- a/phpBB/phpbb/viewonline_helper.php +++ b/phpBB/phpbb/viewonline_helper.php @@ -21,12 +21,60 @@ class viewonline_helper /** @var \phpbb\filesystem\filesystem_interface */ protected $filesystem; + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + /** * @param \phpbb\filesystem\filesystem_interface $filesystem phpBB's filesystem service + * @param \phpbb\db\driver\driver_interface $db */ - public function __construct(\phpbb\filesystem\filesystem_interface $filesystem) + public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, \phpbb\db\driver\driver_interface $db) { $this->filesystem = $filesystem; + $this->db = $db; + } + + /** + * Get forum IDs for topics + * + * Retrieve forum IDs and add the data into the session data array + * Array structure matches sql_fethrowset() result array + * + * @param string $session_data_rowset Users' session data array + * + * @return void + */ + public function get_forum_ids(&$session_data_rowset) + { + $topic_ids = $match = []; + foreach ($session_data_rowset as $number => $row) + { + if ($row['session_forum_id'] == 0 && preg_match('#t=([0-9]+)#', $row['session_page'], $match)) + { + $topic_ids[$number] = (int) $match[1]; + } + } + + if (count($topic_ids = array_unique($topic_ids))) + { + $sql_ary = [ + 'SELECT' => 't.topic_id, t.forum_id', + 'FROM' => [ + TOPICS_TABLE => 't', + ], + 'WHERE' => $this->db->sql_in_set('t.topic_id', $topic_ids), + 'ORDER_BY' => 't.topic_id', + ]; + $result = $this->db->sql_query($this->db->sql_build_query('SELECT', $sql_ary)); + $forum_ids_rowset = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + + foreach ($forum_ids_rowset as $forum_ids_row) + { + $session_data_row_number = array_search((int) $forum_ids_row['topic_id'], $topic_ids); + $session_data_rowset[$session_data_row_number]['session_forum_id'] = (int) $forum_ids_row['forum_id']; + } + } } /** diff --git a/phpBB/viewonline.php b/phpBB/viewonline.php index 1b28750a0b..c9c7e730a5 100644 --- a/phpBB/viewonline.php +++ b/phpBB/viewonline.php @@ -180,6 +180,8 @@ $vars = array('sql_ary', 'show_guests', 'guest_counter', 'forum_data'); extract($phpbb_dispatcher->trigger_event('core.viewonline_modify_sql', compact($vars))); $result = $db->sql_query($db->sql_build_query('SELECT', $sql_ary)); +$session_data_rowset = $db->sql_fetchrowset($result); +$db->sql_freeresult($result); $prev_id = $prev_ip = $user_list = array(); $logged_visible_online = $logged_hidden_online = $counter = 0; @@ -190,7 +192,10 @@ $controller_helper = $phpbb_container->get('controller.helper'); /** @var \phpbb\group\helper $group_helper */ $group_helper = $phpbb_container->get('group_helper'); -while ($row = $db->sql_fetchrow($result)) +// Get forum IDs for session pages which have only 't' parameter +$viewonline_helper->get_forum_ids($session_data_rowset); + +foreach ($session_data_rowset as $row) { if ($row['user_id'] != ANONYMOUS && !isset($prev_id[$row['user_id']])) { @@ -438,7 +443,6 @@ while ($row = $db->sql_fetchrow($result)) $template->assign_block_vars('user_row', $template_row); } -$db->sql_freeresult($result); unset($prev_id, $prev_ip); $order_legend = ($config['legend_sort_groupname']) ? 'group_name' : 'group_legend'; diff --git a/tests/functional/viewonline_test.php b/tests/functional/viewonline_test.php new file mode 100644 index 0000000000..a6aed91661 --- /dev/null +++ b/tests/functional/viewonline_test.php @@ -0,0 +1,118 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +/** +* @group functional +*/ +class phpbb_functional_viewonline_test extends phpbb_functional_test_case +{ + + protected function get_forum_name_by_topic_id($topic_id) + { + $db = $this->get_db(); + + // Forum info + $sql = 'SELECT f.forum_name + FROM ' . FORUMS_TABLE . ' f,' . TOPICS_TABLE . ' t + WHERE t.forum_id = f.forum_id + AND t.topic_id = ' . (int) $topic_id; + $result = $db->sql_query($sql); + $forum_name = $db->sql_fetchfield('forum_name'); + $db->sql_freeresult($result, 1800); // cache for 30 minutes + + return $forum_name; + } + + protected function get_forum_name_by_forum_id($forum_id) + { + $db = $this->get_db(); + + // Forum info + $sql = 'SELECT forum_name + FROM ' . FORUMS_TABLE . ' + WHERE forum_id = ' . (int) $topic_id; + $result = $db->sql_query($sql); + $forum_name = $db->sql_fetchfield('forum_name'); + $db->sql_freeresult($result, 1800); // cache for 30 minutes + + return $forum_name; + } + + public function test_viewonline_reply_mode() + { + $this->create_user('viewonline-test-user'); + $this->logout(); + $this->login('viewonline-test-user'); + $crawler = self::request('GET', 'posting.php?mode=reply&t=1&sid=' . $this->sid); + $this->assertContainsLang('POST_REPLY', $crawler->text()); + + // Log in as another user + $this->logout(); + $this->login(); + $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); + $this->assertContainsLang('FORUM_LOCATION', $crawler->text()); + + // Make sure posting reply page is in the list + $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + $this->assertStringContainsString($this->lang('REPLYING_MESSAGE', $this->get_forum_name_by_topic_id(1)), $crawler->text()); + } + + public function test_viewonline_posting_mode() + { + $this->logout(); + $this->login('viewonline-test-user'); + $crawler = self::request('GET', 'posting.php?mode=post&f=1&sid=' . $this->sid); + $this->assertContainsLang('POST_TOPIC', $crawler->text()); + + // Log in as another user + $this->logout(); + $this->login(); + $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); + + // Make sure posting message page is in the list + $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + $this->assertStringContainsString($this->lang('POSTING_MESSAGE', $this->get_forum_name_by_forum_id(2)), $crawler->text()); + } + + public function test_viewonline_reading_topic() + { + $this->logout(); + $this->login('viewonline-test-user'); + $crawler = self::request('GET', 'viewtopic.php?t=1&sid=' . $this->sid); + + // Log in as another user + $this->logout(); + $this->login(); + $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); + + // Make sure the page is in the list + $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + $this->assertStringContainsString($this->lang('READING_TOPIC', $this->get_forum_name_by_topic_id(1)), $crawler->text()); + } + + public function test_viewonline_reading_forum() + { + $this->logout(); + $this->login('viewonline-test-user'); + $crawler = self::request('GET', 'viewforum.php?f=2&sid=' . $this->sid); + + // Log in as another user + $this->logout(); + $this->login(); + $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); + + // Make sure the page is in the list + $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + $this->assertStringContainsString($this->lang('READING_FORUM', $this->get_forum_name_by_forum_id(2)), $crawler->text()); + } +} From 130e7f7e8a1d743a0c8173b30f896889060bb5a3 Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 23 May 2023 19:26:45 +0700 Subject: [PATCH 2/5] [ticket/17107] Fix viewonline helper test PHPBB3-17107 --- tests/viewonline/helper_test.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/viewonline/helper_test.php b/tests/viewonline/helper_test.php index 32bda75533..1a070ceb4b 100644 --- a/tests/viewonline/helper_test.php +++ b/tests/viewonline/helper_test.php @@ -17,7 +17,11 @@ class phpbb_viewonline_helper_test extends phpbb_test_case { parent::setUp(); - $this->viewonline_helper = new \phpbb\viewonline_helper(new \phpbb\filesystem\filesystem()); + $db = $this->getMockBuilder('\phpbb\db\driver\mysqli') + ->disableOriginalConstructor() + ->getMock(); + + $this->viewonline_helper = new \phpbb\viewonline_helper(new \phpbb\filesystem\filesystem(), $db); } public function session_pages_data() From 60d267893c43ac01e2bc0b941c839b234a871f40 Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 23 May 2023 19:57:31 +0700 Subject: [PATCH 3/5] [ticket/17107] Fix viewonline functional test PHPBB3-17107 --- tests/functional/viewonline_test.php | 80 +++++++++++++--------------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/tests/functional/viewonline_test.php b/tests/functional/viewonline_test.php index a6aed91661..871d43792d 100644 --- a/tests/functional/viewonline_test.php +++ b/tests/functional/viewonline_test.php @@ -16,7 +16,6 @@ */ class phpbb_functional_viewonline_test extends phpbb_functional_test_case { - protected function get_forum_name_by_topic_id($topic_id) { $db = $this->get_db(); @@ -40,7 +39,7 @@ class phpbb_functional_viewonline_test extends phpbb_functional_test_case // Forum info $sql = 'SELECT forum_name FROM ' . FORUMS_TABLE . ' - WHERE forum_id = ' . (int) $topic_id; + WHERE forum_id = ' . (int) $forum_id; $result = $db->sql_query($sql); $forum_name = $db->sql_fetchfield('forum_name'); $db->sql_freeresult($result, 1800); // cache for 30 minutes @@ -48,71 +47,66 @@ class phpbb_functional_viewonline_test extends phpbb_functional_test_case return $forum_name; } - public function test_viewonline_reply_mode() + public function test_viewonline() { - $this->create_user('viewonline-test-user'); - $this->logout(); - $this->login('viewonline-test-user'); + $this->create_user('viewonline-test-user1'); + + // Log in as test user + self::$client->restart(); + $this->login('viewonline-test-user1'); $crawler = self::request('GET', 'posting.php?mode=reply&t=1&sid=' . $this->sid); $this->assertContainsLang('POST_REPLY', $crawler->text()); - // Log in as another user - $this->logout(); + self::$client->restart(); $this->login(); + // PHP goes faster than DBMS, make sure session data got written to the database + sleep(1); $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); - $this->assertContainsLang('FORUM_LOCATION', $crawler->text()); - // Make sure posting reply page is in the list - $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + $this->assertStringContainsString('viewonline-test-user1', $crawler->text()); $this->assertStringContainsString($this->lang('REPLYING_MESSAGE', $this->get_forum_name_by_topic_id(1)), $crawler->text()); - } - public function test_viewonline_posting_mode() - { - $this->logout(); - $this->login('viewonline-test-user'); - $crawler = self::request('GET', 'posting.php?mode=post&f=1&sid=' . $this->sid); + // Log in as test user + self::$client->restart(); + $this->login('viewonline-test-user1'); + $crawler = self::request('GET', 'posting.php?mode=post&f=2&sid=' . $this->sid); $this->assertContainsLang('POST_TOPIC', $crawler->text()); - // Log in as another user - $this->logout(); + self::$client->restart(); $this->login(); + // PHP goes faster than DBMS, make sure session data got written to the database + sleep(1); $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); - // Make sure posting message page is in the list - $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + $this->assertStringContainsString('viewonline-test-user1', $crawler->text()); $this->assertStringContainsString($this->lang('POSTING_MESSAGE', $this->get_forum_name_by_forum_id(2)), $crawler->text()); - } - - public function test_viewonline_reading_topic() - { - $this->logout(); - $this->login('viewonline-test-user'); - $crawler = self::request('GET', 'viewtopic.php?t=1&sid=' . $this->sid); + // Log in as test user + self::$client->restart(); + $this->login('viewonline-test-user1'); + self::request('GET', 'viewtopic.php?t=1&sid=' . $this->sid); // Log in as another user - $this->logout(); + self::$client->restart(); $this->login(); + // PHP goes faster than DBMS, make sure session data got written to the database + sleep(1); $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); - - // Make sure the page is in the list - $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + // Make sure reading topic page is in the list + $this->assertStringContainsString('viewonline-test-user1', $crawler->text()); $this->assertStringContainsString($this->lang('READING_TOPIC', $this->get_forum_name_by_topic_id(1)), $crawler->text()); - } - - public function test_viewonline_reading_forum() - { - $this->logout(); - $this->login('viewonline-test-user'); - $crawler = self::request('GET', 'viewforum.php?f=2&sid=' . $this->sid); + // Log in as test user + self::$client->restart(); + $this->login('viewonline-test-user1'); + self::request('GET', 'viewforum.php?f=2&sid=' . $this->sid); // Log in as another user - $this->logout(); + self::$client->restart(); $this->login(); + // PHP goes faster than DBMS, make sure session data got written to the database + sleep(1); $crawler = self::request('GET', 'viewonline.php?sid=' . $this->sid); - - // Make sure the page is in the list - $this->assertStringContainsString('viewonline-test-user', $crawler->text()); + // Make sure reading forum page is in the list + $this->assertStringContainsString('viewonline-test-user1', $crawler->text()); $this->assertStringContainsString($this->lang('READING_FORUM', $this->get_forum_name_by_forum_id(2)), $crawler->text()); } } From dfe6421da8fb30f633c8700cdd72fa59674b4d47 Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 24 May 2023 20:27:53 +0700 Subject: [PATCH 4/5] [ticket/17107] Fix docblock issues PHPBB3-17107 --- phpBB/phpbb/viewonline_helper.php | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/phpBB/phpbb/viewonline_helper.php b/phpBB/phpbb/viewonline_helper.php index 9b8c4c50ea..fa7312a41f 100644 --- a/phpBB/phpbb/viewonline_helper.php +++ b/phpBB/phpbb/viewonline_helper.php @@ -14,8 +14,8 @@ namespace phpbb; /** -* Class to handle viewonline related tasks -*/ + * Class to handle viewonline related tasks + */ class viewonline_helper { /** @var \phpbb\filesystem\filesystem_interface */ @@ -25,9 +25,9 @@ class viewonline_helper protected $db; /** - * @param \phpbb\filesystem\filesystem_interface $filesystem phpBB's filesystem service - * @param \phpbb\db\driver\driver_interface $db - */ + * @param \phpbb\filesystem\filesystem_interface $filesystem phpBB's filesystem service + * @param \phpbb\db\driver\driver_interface $db + */ public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, \phpbb\db\driver\driver_interface $db) { $this->filesystem = $filesystem; @@ -40,7 +40,7 @@ class viewonline_helper * Retrieve forum IDs and add the data into the session data array * Array structure matches sql_fethrowset() result array * - * @param string $session_data_rowset Users' session data array + * @param array $session_data_rowset Users' session data array * * @return void */ @@ -78,11 +78,11 @@ class viewonline_helper } /** - * Get user page - * - * @param string $session_page User's session page - * @return array Match array filled by preg_match() - */ + * Get user page + * + * @param string $session_page User's session page + * @return array Match array filled by preg_match() + */ public function get_user_page($session_page) { $session_page = $this->filesystem->clean_path($session_page); From 4f9bd296a75fabb43803071c99e56e90491778f5 Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 14 Jun 2023 21:13:49 +0700 Subject: [PATCH 5/5] [ticket/17107] Add method parameter and result type declarations PHPBB3-17107 --- phpBB/phpbb/viewonline_helper.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/phpBB/phpbb/viewonline_helper.php b/phpBB/phpbb/viewonline_helper.php index fa7312a41f..92eca6145f 100644 --- a/phpBB/phpbb/viewonline_helper.php +++ b/phpBB/phpbb/viewonline_helper.php @@ -41,10 +41,9 @@ class viewonline_helper * Array structure matches sql_fethrowset() result array * * @param array $session_data_rowset Users' session data array - * * @return void */ - public function get_forum_ids(&$session_data_rowset) + public function get_forum_ids(array &$session_data_rowset): void { $topic_ids = $match = []; foreach ($session_data_rowset as $number => $row)