From e125f1f7094a8cd9b14e380d43e713721b6684e5 Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 13 Jun 2024 13:27:03 +0700 Subject: [PATCH 1/4] [ticket/17338] Prefer user_last_active to display user last activity info PHPBB-17338 --- phpBB/includes/acp/acp_users.php | 2 +- phpBB/includes/functions_display.php | 2 +- phpBB/memberlist.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index 611e72a9b80..80089c2717d 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -1088,7 +1088,7 @@ function main($id, $mode) $s_action_options .= ''; } - $last_active = (!empty($user_row['session_time'])) ? $user_row['session_time'] : $user_row['user_last_active']; + $last_active = $user_row['user_last_active'] ?: ($user_row['session_time'] ?? 0); $inactive_reason = ''; if ($user_row['user_type'] == USER_INACTIVE) diff --git a/phpBB/includes/functions_display.php b/phpBB/includes/functions_display.php index d995676e8fb..26f2d43cb5c 100644 --- a/phpBB/includes/functions_display.php +++ b/phpBB/includes/functions_display.php @@ -1603,7 +1603,7 @@ function phpbb_show_profile($data, $user_notes_enabled = false, $warn_user_enabl if ($data['user_allow_viewonline'] || $auth->acl_get('u_viewonline')) { - $last_active = (!empty($data['session_time'])) ? $data['session_time'] : $data['user_last_active']; + $last_active = $data['user_last_active'] ?: ($data['session_time'] ?? 0); } else { diff --git a/phpBB/memberlist.php b/phpBB/memberlist.php index dbdfe80618e..c846bdde055 100644 --- a/phpBB/memberlist.php +++ b/phpBB/memberlist.php @@ -1722,7 +1722,7 @@ { $row['session_time'] = $session_ary[$row['user_id']]['session_time'] ?? 0; $row['session_viewonline'] = $session_ary[$row['user_id']]['session_viewonline'] ?? 0; - $row['last_visit'] = (!empty($row['session_time'])) ? $row['session_time'] : $row['user_last_active']; + $row['last_visit'] = $row['user_last_active'] ?: $row['session_time']; $id_cache[$row['user_id']] = $row; } From db9874546b95f17254ca944b050f4ea595d688b2 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 14 Jun 2024 10:28:25 +0700 Subject: [PATCH 2/4] [ticket/17338] Add user_last_active to session_gc() PHPBB-17338 --- phpBB/phpbb/session.php | 10 ++++++---- tests/session/fixtures/sessions_garbage.xml | 3 +++ tests/session/garbage_collection_test.php | 4 ++++ tests/test_framework/phpbb_session_test_case.php | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php index af39ecf2fa0..46e82d03baf 100644 --- a/phpBB/phpbb/session.php +++ b/phpBB/phpbb/session.php @@ -987,7 +987,7 @@ function session_gc() // For SQLite versions 3.8.3+ which support Common Table Expressions (CTE) $sql = "WITH s3 (session_page, session_user_id, session_time) AS ($sql_select) UPDATE " . USERS_TABLE . ' - SET (user_lastpage, user_lastvisit) = (SELECT session_page, session_time FROM s3 WHERE session_user_id = user_id) + SET (user_lastpage, user_lastvisit, user_last_active) = (SELECT session_page, session_time, session_time FROM s3 WHERE session_user_id = user_id) WHERE EXISTS (SELECT session_user_id FROM s3 WHERE session_user_id = user_id)'; $db->sql_query($sql); @@ -1000,7 +1000,9 @@ function session_gc() while ($row = $db->sql_fetchrow($result)) { $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_lastvisit = ' . (int) $row['recent_time'] . ", user_lastpage = '" . $db->sql_escape($row['session_page']) . "' + SET user_lastvisit = ' . (int) $row['recent_time'] . ', + user_last_active = ' . (int) $row['recent_time'] . ", + user_lastpage = '" . $db->sql_escape($row['session_page']) . "' WHERE user_id = " . (int) $row['session_user_id']; $db->sql_query($sql); } @@ -1010,14 +1012,14 @@ function session_gc() case 'mysqli': $sql = 'UPDATE ' . USERS_TABLE . " u, ($sql_select) s3 - SET u.user_lastvisit = s3.recent_time, u.user_lastpage = s3.session_page + SET u.user_lastvisit = s3.recent_time, u.user_last_active = s3.recent_time, u.user_lastpage = s3.session_page WHERE u.user_id = s3.session_user_id"; $db->sql_query($sql); break; default: $sql = 'UPDATE ' . USERS_TABLE . " - SET user_lastvisit = s3.recent_time, user_lastpage = s3.session_page + SET user_lastvisit = s3.recent_time, user_last_active = s3.recent_time, user_lastpage = s3.session_page FROM ($sql_select) s3 WHERE user_id = s3.session_user_id"; $db->sql_query($sql); diff --git a/tests/session/fixtures/sessions_garbage.xml b/tests/session/fixtures/sessions_garbage.xml index 59a2dc2ebe7..16971428a70 100644 --- a/tests/session/fixtures/sessions_garbage.xml +++ b/tests/session/fixtures/sessions_garbage.xml @@ -7,6 +7,7 @@ user_sig user_lastpage user_lastvisit + user_last_active 4 bar @@ -14,6 +15,7 @@ oldpage_user_bar.php 1400000000 + 1300000999 5 @@ -22,6 +24,7 @@ oldpage_user_foo.php 1400000000 + 1300000998 diff --git a/tests/session/garbage_collection_test.php b/tests/session/garbage_collection_test.php index 9080478a285..32c4db3f85d 100644 --- a/tests/session/garbage_collection_test.php +++ b/tests/session/garbage_collection_test.php @@ -65,11 +65,13 @@ public function test_session_gc() [ 'username_clean' => 'bar', 'user_lastvisit' => 1400000000, + 'user_last_active' => 1300000999, 'user_lastpage' => 'oldpage_user_bar.php', ], [ 'username_clean' => 'foo', 'user_lastvisit' => 1400000000, + 'user_last_active' => 1300000998, 'user_lastpage' => 'oldpage_user_foo.php', ], ], @@ -89,11 +91,13 @@ public function test_session_gc() [ 'username_clean' => 'bar', 'user_lastvisit' => '1500000000', + 'user_last_active' => '1500000000', 'user_lastpage' => 'newpage_user_bar.php', ], [ 'username_clean' => 'foo', 'user_lastvisit' => '1500000000', + 'user_last_active' => '1500000000', 'user_lastpage' => 'newpage_user_foo.php', ], ], diff --git a/tests/test_framework/phpbb_session_test_case.php b/tests/test_framework/phpbb_session_test_case.php index deb76d4e5e5..15b5d44eb53 100644 --- a/tests/test_framework/phpbb_session_test_case.php +++ b/tests/test_framework/phpbb_session_test_case.php @@ -50,7 +50,7 @@ protected function setUp(): void protected function check_user_session_data($expected_session_data, $message) { - $sql= 'SELECT username_clean, user_lastvisit, user_lastpage + $sql= 'SELECT username_clean, user_lastvisit, user_last_active, user_lastpage FROM ' . USERS_TABLE . ' ORDER BY user_id'; From e21a8e02cdc21c5e6c70727114eab4006014dedd Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 14 Jun 2024 15:22:20 +0700 Subject: [PATCH 3/4] [ticket/17338] Update user_last_active on session removal and login keys reset PHPBB-17338 --- phpBB/phpbb/session.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php index 46e82d03baf..a1d7df511a5 100644 --- a/phpBB/phpbb/session.php +++ b/phpBB/phpbb/session.php @@ -903,7 +903,8 @@ function session_kill($new_session = true) } $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_lastvisit = ' . (int) $this->data['session_time'] . ' + SET user_lastvisit = ' . (int) $this->data['session_time'] . ', + user_last_active = ' . (int) $this->data['session_time'] . ' WHERE user_id = ' . (int) $this->data['user_id']; $db->sql_query($sql); @@ -1652,7 +1653,9 @@ function reset_login_keys($user_id = false) if ($row) { $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_lastvisit = ' . (int) $row['session_time'] . ", user_lastpage = '" . $db->sql_escape($row['session_page']) . "' + SET user_lastvisit = ' . (int) $row['session_time'] . ', + user_last_active = ' . (int) $row['session_time'] . ", + user_lastpage = '" . $db->sql_escape($row['session_page']) . "' WHERE user_id = " . (int) $user_id; $db->sql_query($sql); } From 4003f54d0bda589830efbae47a5173125a603a1b Mon Sep 17 00:00:00 2001 From: rxu Date: Mon, 17 Jun 2024 11:31:27 +0700 Subject: [PATCH 4/4] [ticket/17338] Do not update user_last_active to outdated session_time value PHPBB-17338 --- phpBB/phpbb/session.php | 25 ++++++++----------- tests/session/fixtures/sessions_garbage.xml | 3 --- tests/session/garbage_collection_test.php | 4 --- .../phpbb_session_test_case.php | 2 +- 4 files changed, 11 insertions(+), 23 deletions(-) diff --git a/phpBB/phpbb/session.php b/phpBB/phpbb/session.php index a1d7df511a5..aa4841316fa 100644 --- a/phpBB/phpbb/session.php +++ b/phpBB/phpbb/session.php @@ -441,7 +441,7 @@ function session_begin($update_session_page = true) $this->check_ban_for_current_session($config); // Update user last active time accordingly, but in a minute or so - if ((int) $this->data['session_time'] - (int) $this->data['user_last_active'] > 60) + if ($this->time_now - (int) $this->data['user_last_active'] > 60) { $this->update_last_active_time(); } @@ -903,8 +903,7 @@ function session_kill($new_session = true) } $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_lastvisit = ' . (int) $this->data['session_time'] . ', - user_last_active = ' . (int) $this->data['session_time'] . ' + SET user_lastvisit = ' . (int) $this->data['session_time'] . ' WHERE user_id = ' . (int) $this->data['user_id']; $db->sql_query($sql); @@ -988,7 +987,7 @@ function session_gc() // For SQLite versions 3.8.3+ which support Common Table Expressions (CTE) $sql = "WITH s3 (session_page, session_user_id, session_time) AS ($sql_select) UPDATE " . USERS_TABLE . ' - SET (user_lastpage, user_lastvisit, user_last_active) = (SELECT session_page, session_time, session_time FROM s3 WHERE session_user_id = user_id) + SET (user_lastpage, user_lastvisit) = (SELECT session_page, session_time FROM s3 WHERE session_user_id = user_id) WHERE EXISTS (SELECT session_user_id FROM s3 WHERE session_user_id = user_id)'; $db->sql_query($sql); @@ -1001,9 +1000,7 @@ function session_gc() while ($row = $db->sql_fetchrow($result)) { $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_lastvisit = ' . (int) $row['recent_time'] . ', - user_last_active = ' . (int) $row['recent_time'] . ", - user_lastpage = '" . $db->sql_escape($row['session_page']) . "' + SET user_lastvisit = ' . (int) $row['recent_time'] . ", user_lastpage = '" . $db->sql_escape($row['session_page']) . "' WHERE user_id = " . (int) $row['session_user_id']; $db->sql_query($sql); } @@ -1013,14 +1010,14 @@ function session_gc() case 'mysqli': $sql = 'UPDATE ' . USERS_TABLE . " u, ($sql_select) s3 - SET u.user_lastvisit = s3.recent_time, u.user_last_active = s3.recent_time, u.user_lastpage = s3.session_page + SET u.user_lastvisit = s3.recent_time, u.user_lastpage = s3.session_page WHERE u.user_id = s3.session_user_id"; $db->sql_query($sql); break; default: $sql = 'UPDATE ' . USERS_TABLE . " - SET user_lastvisit = s3.recent_time, user_last_active = s3.recent_time, user_lastpage = s3.session_page + SET user_lastvisit = s3.recent_time, user_lastpage = s3.session_page FROM ($sql_select) s3 WHERE user_id = s3.session_user_id"; $db->sql_query($sql); @@ -1653,9 +1650,7 @@ function reset_login_keys($user_id = false) if ($row) { $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_lastvisit = ' . (int) $row['session_time'] . ', - user_last_active = ' . (int) $row['session_time'] . ", - user_lastpage = '" . $db->sql_escape($row['session_page']) . "' + SET user_lastvisit = ' . (int) $row['session_time'] . ", user_lastpage = '" . $db->sql_escape($row['session_page']) . "' WHERE user_id = " . (int) $user_id; $db->sql_query($sql); } @@ -1817,7 +1812,7 @@ public function update_user_lastvisit() { $sql = 'UPDATE ' . USERS_TABLE . ' SET user_lastvisit = ' . (int) $this->data['session_time'] . ', - user_last_active = ' . (int) $this->data['session_time'] . ' + user_last_active = ' . $this->time_now . ' WHERE user_id = ' . (int) $this->data['user_id']; $db->sql_query($sql); } @@ -1832,10 +1827,10 @@ public function update_last_active_time() { global $db; - if (isset($this->data['session_time'], $this->data['user_id'])) + if (isset($this->time_now, $this->data['user_id'])) { $sql = 'UPDATE ' . USERS_TABLE . ' - SET user_last_active = ' . (int) $this->data['session_time'] . ' + SET user_last_active = ' . $this->time_now . ' WHERE user_id = ' . (int) $this->data['user_id']; $db->sql_query($sql); } diff --git a/tests/session/fixtures/sessions_garbage.xml b/tests/session/fixtures/sessions_garbage.xml index 16971428a70..59a2dc2ebe7 100644 --- a/tests/session/fixtures/sessions_garbage.xml +++ b/tests/session/fixtures/sessions_garbage.xml @@ -7,7 +7,6 @@ user_sig user_lastpage user_lastvisit - user_last_active 4 bar @@ -15,7 +14,6 @@ oldpage_user_bar.php 1400000000 - 1300000999 5 @@ -24,7 +22,6 @@ oldpage_user_foo.php 1400000000 - 1300000998
diff --git a/tests/session/garbage_collection_test.php b/tests/session/garbage_collection_test.php index 32c4db3f85d..9080478a285 100644 --- a/tests/session/garbage_collection_test.php +++ b/tests/session/garbage_collection_test.php @@ -65,13 +65,11 @@ public function test_session_gc() [ 'username_clean' => 'bar', 'user_lastvisit' => 1400000000, - 'user_last_active' => 1300000999, 'user_lastpage' => 'oldpage_user_bar.php', ], [ 'username_clean' => 'foo', 'user_lastvisit' => 1400000000, - 'user_last_active' => 1300000998, 'user_lastpage' => 'oldpage_user_foo.php', ], ], @@ -91,13 +89,11 @@ public function test_session_gc() [ 'username_clean' => 'bar', 'user_lastvisit' => '1500000000', - 'user_last_active' => '1500000000', 'user_lastpage' => 'newpage_user_bar.php', ], [ 'username_clean' => 'foo', 'user_lastvisit' => '1500000000', - 'user_last_active' => '1500000000', 'user_lastpage' => 'newpage_user_foo.php', ], ], diff --git a/tests/test_framework/phpbb_session_test_case.php b/tests/test_framework/phpbb_session_test_case.php index 15b5d44eb53..deb76d4e5e5 100644 --- a/tests/test_framework/phpbb_session_test_case.php +++ b/tests/test_framework/phpbb_session_test_case.php @@ -50,7 +50,7 @@ protected function setUp(): void protected function check_user_session_data($expected_session_data, $message) { - $sql= 'SELECT username_clean, user_lastvisit, user_last_active, user_lastpage + $sql= 'SELECT username_clean, user_lastvisit, user_lastpage FROM ' . USERS_TABLE . ' ORDER BY user_id';