From cf651ef81d611865a06d93c9db7c4dfcd680405c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 17 Mar 2012 23:50:28 +0100 Subject: [PATCH 01/56] [ticket/10714] Implement a class to add logs to the database. PHPBB3-10714 --- phpBB/includes/log/interface.php | 55 ++++++++++ phpBB/includes/log/log.php | 177 +++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 phpBB/includes/log/interface.php create mode 100644 phpBB/includes/log/log.php diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php new file mode 100644 index 0000000000..897b8a8211 --- /dev/null +++ b/phpBB/includes/log/interface.php @@ -0,0 +1,55 @@ +log_table = $log_table; + $this->enable(); + } + + /** + * This function returns the state of the log-system. + * + * @return bool True if log is enabled + */ + public function is_enabled() + { + return $this->enabled; + } + + /** + * This function allows disable the log-system. When add_log is called, the log will not be added to the database. + */ + public function disable() + { + $this->enabled = false; + } + + /** + * This function allows re-enable the log-system. + */ + public function enable() + { + $this->enabled = true; + } + + /** + * Adds a log to the database + * + * @param string $mode The mode defines which log_type is used and in which log the entry is displayed. + * @param int $user_id User ID of the user + * @param string $log_ip IP address of the user + * @param string $log_operation Name of the operation + * @param int $log_time Timestamp when the log was added. + * @param array $additional_data More arguments can be added, depending on the log_type + * + * @return int|bool Returns the log_id, if the entry was added to the database, false otherwise. + */ + public function add($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = array()) + { + if (!$this->is_enabled()) + { + return false; + } + + global $db; + /** + * @todo: enable when events are merged + * + global $db, $phpbb_dispatcher; + */ + + if ($log_time == false) + { + $log_time = time(); + } + + $sql_ary = array( + 'user_id' => $user_id, + 'log_ip' => $log_ip, + 'log_time' => $log_time, + 'log_operation' => $log_operation, + ); + + switch ($mode) + { + case 'admin': + $sql_ary += array( + 'log_type' => LOG_ADMIN, + 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + ); + break; + + case 'mod': + $sql_ary += array( + 'log_type' => LOG_MOD, + 'forum_id' => intval(array_shift($additional_data)), + 'topic_id' => intval(array_shift($additional_data)), + 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + ); + break; + + case 'user': + $sql_ary += array( + 'log_type' => LOG_USERS, + 'reportee_id' => intval(array_shift($additional_data)), + 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + ); + break; + + case 'critical': + $sql_ary += array( + 'log_type' => LOG_CRITICAL, + 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + ); + break; + + default: + /** + * @todo: enable when events are merged + * + if ($phpbb_dispatcher != null) + { + $vars = array('mode', 'user_id', 'log_ip', 'log_time', 'additional_data', 'sql_ary'); + $event = new phpbb_event_data(compact($vars)); + $phpbb_dispatcher->dispatch('core.add_log_case', $event); + extract($event->get_data_filtered($vars)); + } + */ + + // We didn't find a log_type, so we don't save it in the database. + if (!isset($sql_ary['log_type'])) + { + return false; + } + } + + /** + * @todo: enable when events are merged + * + if ($phpbb_dispatcher != null) + { + $vars = array('mode', 'user_id', 'log_ip', 'log_time', 'additional_data', 'sql_ary'); + $event = new phpbb_event_data(compact($vars)); + $phpbb_dispatcher->dispatch('core.add_log', $event); + extract($event->get_data_filtered($vars)); + } + */ + + $db->sql_query('INSERT INTO ' . $this->log_table . ' ' . $db->sql_build_array('INSERT', $sql_ary)); + + return $db->sql_nextid(); + } +} From 3fbac076ceb4773aa3c985d24eeaf306aa0b6a42 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 17 Mar 2012 23:52:01 +0100 Subject: [PATCH 02/56] [ticket/10714] Use new phpbb_log class in add_log function PHPBB3-10714 --- phpBB/includes/functions.php | 93 ++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index ecec1e5e4a..9a1485f37a 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3357,65 +3357,74 @@ function parse_cfg_file($filename, $lines = false) */ function add_log() { - global $db, $user; + // This is all just an ugly hack to add "Dependency Injection" to a function + // the only real code is the function call which maps this function to a method. + static $static_log = null; - // In phpBB 3.1.x i want to have logging in a class to be able to control it - // For now, we need a quite hakish approach to circumvent logging for some actions - // @todo implement cleanly - if (!empty($GLOBALS['skip_add_log'])) + $args = func_get_args(); + $log = (isset($args[0])) ? $args[0] : false; + + if ($log instanceof phpbb_log_interface) + { + $static_log = $log; + return true; + } + else if ($log === false) { return false; } - $args = func_get_args(); + $tmp_log = $static_log; - $mode = array_shift($args); - $reportee_id = ($mode == 'user') ? intval(array_shift($args)) : ''; - $forum_id = ($mode == 'mod') ? intval(array_shift($args)) : ''; - $topic_id = ($mode == 'mod') ? intval(array_shift($args)) : ''; - $action = array_shift($args); - $data = (!sizeof($args)) ? '' : serialize($args); + // no log class set, create a temporary one ourselves to keep backwards compatability + if ($tmp_log === null) + { + $tmp_log = new phpbb_log(LOG_TABLE); + } - $sql_ary = array( - 'user_id' => (empty($user->data)) ? ANONYMOUS : $user->data['user_id'], - 'log_ip' => $user->ip, - 'log_time' => time(), - 'log_operation' => $action, - 'log_data' => $data, - ); + $mode = array_shift($args); + // This looks kind of dirty, but add_log has some additional data before the log_operation + $additional_data = array(); switch ($mode) { case 'admin': - $sql_ary['log_type'] = LOG_ADMIN; - break; - - case 'mod': - $sql_ary += array( - 'log_type' => LOG_MOD, - 'forum_id' => $forum_id, - 'topic_id' => $topic_id - ); - break; - - case 'user': - $sql_ary += array( - 'log_type' => LOG_USERS, - 'reportee_id' => $reportee_id - ); - break; - case 'critical': - $sql_ary['log_type'] = LOG_CRITICAL; break; - + case 'mod': + // forum_id + $additional_data[] = array_shift($args); + // topic_id + $additional_data[] = array_shift($args); + break; + case 'user': + // reportee_id + $additional_data[] = array_shift($args); + break; default: - return false; + /** + * @todo: enable when events are merged + * + global $phpbb_dispatcher; + + if ($phpbb_dispatcher != null) + { + $vars = array('mode', 'args', 'additional_data'); + $event = new phpbb_event_data(compact($vars)); + $phpbb_dispatcher->dispatch('core.function_add_log', $event); + extract($event->get_data_filtered($vars)); + } + */ } + $log_operation = array_shift($args); + $additional_data = array_merge($additional_data, $args); - $db->sql_query('INSERT INTO ' . LOG_TABLE . ' ' . $db->sql_build_array('INSERT', $sql_ary)); + global $user; - return $db->sql_nextid(); + $user_id = (empty($user->data)) ? ANONYMOUS : $user->data['user_id']; + $user_ip = (empty($user->ip)) ? '' : $user->ip; + + return $tmp_log->add($mode, $user_id, $user_ip, $log_operation, time(), $additional_data); } /** From 34ce2561a0242c9066702e5fa9c92d0a6c77c2d2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 17 Mar 2012 23:52:47 +0100 Subject: [PATCH 03/56] [ticket/10714] Remove the dirty global hack to disable the log. PHPBB3-10714 --- phpBB/includes/functions_user.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 9e33a5122e..4074eaa2f2 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -299,8 +299,10 @@ function user_add($user_row, $cp_data = false) if ($add_group_id) { - // Because these actions only fill the log unneccessarily we skip the add_log() entry with a little hack. :/ - $GLOBALS['skip_add_log'] = true; + global $phpbb_log; + + // Because these actions only fill the log unneccessarily we skip the add_log() entry. + $phpbb_log->disable(); // Add user to "newly registered users" group and set to default group if admin specified so. if ($config['new_member_group_default']) @@ -313,7 +315,7 @@ function user_add($user_row, $cp_data = false) group_user_add($add_group_id, $user_id); } - unset($GLOBALS['skip_add_log']); + $phpbb_log->enable(); } } From 87eec7cfb66f6072344680743b04bf0186e8ca17 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 00:07:09 +0100 Subject: [PATCH 04/56] [ticket/10714] Create a phpbb_log object and inject it into add_log PHPBB3-10714 --- phpBB/common.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpBB/common.php b/phpBB/common.php index c7c5859c25..bbcf8b894f 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -139,6 +139,10 @@ foreach ($cache->obtain_hooks() as $hook) @include($phpbb_root_path . 'includes/hooks/' . $hook . '.' . $phpEx); } +// make sure add_log uses this log instance +$phpbb_log = new phpbb_log(LOG_TABLE); +add_log($phpbb_log); // "dependency injection" for a function + if (!$config['use_system_cron']) { $cron = new phpbb_cron_manager(new phpbb_cron_task_provider($phpbb_extension_manager), $cache->get_driver()); From 7e80e4004e92ddcf3ad147d05d43bb411010e9e0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 12:07:41 +0100 Subject: [PATCH 05/56] [ticket/10714] Add unit tests for add_log function PHPBB3-10714 --- tests/log/fixtures/empty_log.xml | 15 +++ tests/log/function_add_log_test.php | 151 ++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 tests/log/fixtures/empty_log.xml create mode 100644 tests/log/function_add_log_test.php diff --git a/tests/log/fixtures/empty_log.xml b/tests/log/fixtures/empty_log.xml new file mode 100644 index 0000000000..261b6a622a --- /dev/null +++ b/tests/log/fixtures/empty_log.xml @@ -0,0 +1,15 @@ + + + + log_id + log_type + user_id + forum_id + topic_id + reportee_id + log_ip + log_time + log_operation + log_data +
+ diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php new file mode 100644 index 0000000000..05fae0f916 --- /dev/null +++ b/tests/log/function_add_log_test.php @@ -0,0 +1,151 @@ +createXMLDataSet(dirname(__FILE__) . '/fixtures/empty_log.xml'); + } + + public static function test_add_log_function_critical_data() + { + return array( + array( + array( + array( + 'user_id' => 2, + 'log_type' => LOG_CRITICAL, + 'log_operation' => 'LOG_NO_ADDITIONAL', + 'log_data' => '', + 'reportee_id' => 0, + 'forum_id' => 0, + 'topic_id' => 0, + ), + ), + 2, 'critical', 'LOG_NO_ADDITIONAL', + ), + array( + array( + array( + 'user_id' => 2, + 'log_type' => LOG_CRITICAL, + 'log_operation' => 'LOG_ONE_ADDITIONAL', + 'log_data' => 'a:1:{i:0;s:9:"argument1";}', + 'reportee_id' => 0, + 'forum_id' => 0, + 'topic_id' => 0, + ), + ), + 2, 'critical', 'LOG_ONE_ADDITIONAL', 'argument1', + ), + array( + array( + array( + 'user_id' => ANONYMOUS, + 'log_type' => LOG_ADMIN, + 'log_operation' => 'LOG_TWO_ADDITIONAL', + 'log_data' => 'a:2:{i:0;s:9:"argument1";i:1;s:9:"argument2";}', + 'reportee_id' => 0, + 'forum_id' => 0, + 'topic_id' => 0, + ), + ), + false, 'admin', 'LOG_TWO_ADDITIONAL', 'argument1', 'argument2', + ), + array( + array( + array( + 'user_id' => ANONYMOUS, + 'log_type' => LOG_USERS, + 'log_operation' => 'LOG_USERS_ADDITIONAL', + 'log_data' => 'a:1:{i:0;s:9:"argument2";}', + 'reportee_id' => 2, + 'forum_id' => 0, + 'topic_id' => 0, + ), + ), + false, 'user', 2, 'LOG_USERS_ADDITIONAL', 'argument2', + ), + array( + array( + array( + 'user_id' => ANONYMOUS, + 'log_type' => LOG_MOD, + 'log_operation' => 'LOG_MOD_TOPIC_AND_FORUM', + 'log_data' => '', + 'reportee_id' => 0, + 'forum_id' => 12, + 'topic_id' => 34, + ), + ), + false, 'mod', 12, 34, 'LOG_MOD_TOPIC_AND_FORUM', + ), + array( + array( + array( + 'user_id' => ANONYMOUS, + 'log_type' => LOG_MOD, + 'log_operation' => 'LOG_MOD_ADDITIONAL', + 'log_data' => 'a:1:{i:0;s:9:"argument3";}', + 'reportee_id' => 0, + 'forum_id' => 56, + 'topic_id' => 78, + ), + ), + false, 'mod', 56, 78, 'LOG_MOD_ADDITIONAL', 'argument3', + ), + array( + array( + ), + false, 'mode_does_not_exist', 'LOG_MOD_ADDITIONAL', 'argument1', + ), + ); + } + + /** + * @dataProvider test_add_log_function_critical_data + */ + public function test_add_log_function_critical($expected, $user_id, $mode, $required1, $additional1 = null, $additional2 = null, $additional3 = null) + { + global $db, $user; + + $db = $this->new_dbal(); + + $user->ip = 'user_ip'; + if ($user_id) + { + $user->data['user_id'] = $user_id; + } + + if ($additional3 != null) + { + add_log($mode, $required1, $additional1, $additional2, $additional3); + } + else if ($additional2 != null) + { + add_log($mode, $required1, $additional1, $additional2); + } + else if ($additional1 != null) + { + add_log($mode, $required1, $additional1); + } + else + { + add_log($mode, $required1); + } + + $result = $db->sql_query('SELECT user_id, log_type, log_operation, log_data, reportee_id, forum_id, topic_id + FROM ' . LOG_TABLE); + + $this->assertEquals($expected, $db->sql_fetchrowset($result)); + } +} From 72d875ebdee08f8c6af7c016b15d3e89442ed0e1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 12:23:32 +0100 Subject: [PATCH 06/56] [ticket/10714] Add unit tests for log class PHPBB3-10714 --- tests/log/add_test.php | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/log/add_test.php diff --git a/tests/log/add_test.php b/tests/log/add_test.php new file mode 100644 index 0000000000..a2b763f2b7 --- /dev/null +++ b/tests/log/add_test.php @@ -0,0 +1,56 @@ +createXMLDataSet(dirname(__FILE__) . '/fixtures/empty_log.xml'); + } + + public function test_log_enabled() + { + $log = new phpbb_log(LOG_TABLE); + $this->assertTrue($log->is_enabled()); + + $log->disable(); + $this->assertFalse($log->is_enabled()); + + $log->enable(); + $this->assertTrue($log->is_enabled()); + } + + public function test_log_add() + { + global $db; + + $db = $this->new_dbal(); + + $mode = 'critical'; + $user_id = ANONYMOUS; + $log_ip = 'user_ip'; + $log_time = time(); + $log_operation = 'LOG_OPERATION'; + $additional_data = array(); + + // Add an entry successful + $log = new phpbb_log(LOG_TABLE); + $this->assertEquals(1, $log->add($mode, $user_id, $log_ip, $log_operation, $log_time)); + + // Disable logging + $log->disable(); + $this->assertFalse($log->add($mode, $user_id, $log_ip, $log_operation, $log_time)); + $log->enable(); + + // Invalid mode specified + $this->assertFalse($log->add('mode_does_not_exist', $user_id, $log_ip, $log_operation, $log_time)); + } +} From 31e18f31a6139cddb32520aa6ed020dc8f80f70a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 13:40:56 +0100 Subject: [PATCH 07/56] [ticket/10714] Serialize the log_data in the testinsteadof hardcoding it PHPBB3-10714 --- tests/log/function_add_log_test.php | 107 +++++++++++++++------------- 1 file changed, 57 insertions(+), 50 deletions(-) diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index 05fae0f916..407aeb9ad1 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -21,85 +21,82 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case return array( array( array( - array( - 'user_id' => 2, - 'log_type' => LOG_CRITICAL, - 'log_operation' => 'LOG_NO_ADDITIONAL', - 'log_data' => '', - 'reportee_id' => 0, - 'forum_id' => 0, - 'topic_id' => 0, - ), + 'user_id' => 2, + 'log_type' => LOG_CRITICAL, + 'log_operation' => 'LOG_NO_ADDITIONAL', + 'log_data' => '', + 'reportee_id' => 0, + 'forum_id' => 0, + 'topic_id' => 0, ), 2, 'critical', 'LOG_NO_ADDITIONAL', ), array( array( - array( - 'user_id' => 2, - 'log_type' => LOG_CRITICAL, - 'log_operation' => 'LOG_ONE_ADDITIONAL', - 'log_data' => 'a:1:{i:0;s:9:"argument1";}', - 'reportee_id' => 0, - 'forum_id' => 0, - 'topic_id' => 0, + 'user_id' => 2, + 'log_type' => LOG_CRITICAL, + 'log_operation' => 'LOG_ONE_ADDITIONAL', + 'log_data' => array( + 'argument1', ), + 'reportee_id' => 0, + 'forum_id' => 0, + 'topic_id' => 0, ), 2, 'critical', 'LOG_ONE_ADDITIONAL', 'argument1', ), array( array( - array( - 'user_id' => ANONYMOUS, - 'log_type' => LOG_ADMIN, - 'log_operation' => 'LOG_TWO_ADDITIONAL', - 'log_data' => 'a:2:{i:0;s:9:"argument1";i:1;s:9:"argument2";}', - 'reportee_id' => 0, - 'forum_id' => 0, - 'topic_id' => 0, + 'user_id' => ANONYMOUS, + 'log_type' => LOG_ADMIN, + 'log_operation' => 'LOG_TWO_ADDITIONAL', + 'log_data' => array( + 'argument1', + 'argument2', ), + 'reportee_id' => 0, + 'forum_id' => 0, + 'topic_id' => 0, ), false, 'admin', 'LOG_TWO_ADDITIONAL', 'argument1', 'argument2', ), array( array( - array( - 'user_id' => ANONYMOUS, - 'log_type' => LOG_USERS, - 'log_operation' => 'LOG_USERS_ADDITIONAL', - 'log_data' => 'a:1:{i:0;s:9:"argument2";}', - 'reportee_id' => 2, - 'forum_id' => 0, - 'topic_id' => 0, + 'user_id' => ANONYMOUS, + 'log_type' => LOG_USERS, + 'log_operation' => 'LOG_USERS_ADDITIONAL', + 'log_data' => array( + 'argument2', ), + 'reportee_id' => 2, + 'forum_id' => 0, + 'topic_id' => 0, ), false, 'user', 2, 'LOG_USERS_ADDITIONAL', 'argument2', ), array( array( - array( - 'user_id' => ANONYMOUS, - 'log_type' => LOG_MOD, - 'log_operation' => 'LOG_MOD_TOPIC_AND_FORUM', - 'log_data' => '', - 'reportee_id' => 0, - 'forum_id' => 12, - 'topic_id' => 34, - ), + 'user_id' => ANONYMOUS, + 'log_type' => LOG_MOD, + 'log_operation' => 'LOG_MOD_TOPIC_AND_FORUM', + 'log_data' => '', + 'reportee_id' => 0, + 'forum_id' => 12, + 'topic_id' => 34, ), false, 'mod', 12, 34, 'LOG_MOD_TOPIC_AND_FORUM', ), array( array( - array( - 'user_id' => ANONYMOUS, - 'log_type' => LOG_MOD, - 'log_operation' => 'LOG_MOD_ADDITIONAL', - 'log_data' => 'a:1:{i:0;s:9:"argument3";}', - 'reportee_id' => 0, - 'forum_id' => 56, - 'topic_id' => 78, + 'user_id' => ANONYMOUS, + 'log_type' => LOG_MOD, + 'log_operation' => 'LOG_MOD_ADDITIONAL', + 'log_data' => array( + 'argument3', ), + 'reportee_id' => 0, + 'forum_id' => 56, + 'topic_id' => 78, ), false, 'mod', 56, 78, 'LOG_MOD_ADDITIONAL', 'argument3', ), @@ -118,6 +115,16 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case { global $db, $user; + if ($expected) + { + // Serialize the log data if we have some + if (is_array($expected['log_data'])) + { + $expected['log_data'] = serialize($expected['log_data']); + } + $expected = array($expected); + } + $db = $this->new_dbal(); $user->ip = 'user_ip'; From 1539ad7ebe1493e4c486181f65976c93dbb95c29 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 13:42:08 +0100 Subject: [PATCH 08/56] [ticket/10714] Add @return null to doc blocks PHPBB3-10714 --- phpBB/includes/log/interface.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index 897b8a8211..7eda4b9710 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -31,11 +31,15 @@ interface phpbb_log_interface /** * This function allows disable the log-system. When add_log is called, the log will not be added to the database. + * + * @return null */ public function disable(); /** * This function allows re-enable the log-system. + * + * @return null */ public function enable(); From cff15ec307d76b004b6a825fb51b5dd3c8da58f2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 13:47:24 +0100 Subject: [PATCH 09/56] [ticket/10714] Use keys for the log data instead of requiring a special order PHPBB3-10714 --- phpBB/includes/functions.php | 9 +++------ phpBB/includes/log/log.php | 13 ++++++++++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 9a1485f37a..3e3d796ba2 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3392,14 +3392,11 @@ function add_log() case 'critical': break; case 'mod': - // forum_id - $additional_data[] = array_shift($args); - // topic_id - $additional_data[] = array_shift($args); + $additional_data['forum_id'] = array_shift($args); + $additional_data['topic_id'] = array_shift($args); break; case 'user': - // reportee_id - $additional_data[] = array_shift($args); + $additional_data['reportee_id'] = array_shift($args); break; default: /** diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 89dc22593e..2523b97dbe 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -115,18 +115,25 @@ class phpbb_log implements phpbb_log_interface break; case 'mod': + $forum_id = (int) $additional_data['forum_id']; + unset($additional_data['forum_id']); + $topic_id = (int) $additional_data['topic_id']; + unset($additional_data['topic_id']); $sql_ary += array( 'log_type' => LOG_MOD, - 'forum_id' => intval(array_shift($additional_data)), - 'topic_id' => intval(array_shift($additional_data)), + 'forum_id' => $forum_id, + 'topic_id' => $topic_id, 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), ); break; case 'user': + $reportee_id = (int) $additional_data['reportee_id']; + unset($additional_data['reportee_id']); + $sql_ary += array( 'log_type' => LOG_USERS, - 'reportee_id' => intval(array_shift($additional_data)), + 'reportee_id' => $reportee_id, 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), ); break; From b9b08cf765d7feb2865477bb82ab58e8cfb0c156 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 13:54:33 +0100 Subject: [PATCH 10/56] [ticket/10714] Add return null to phpbb_log and add param to constructor PHPBB3-10714 --- phpBB/includes/log/log.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 2523b97dbe..67336d232a 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -34,6 +34,8 @@ class phpbb_log implements phpbb_log_interface /** * Constructor + * + * @param string $log_table The table we use to store our logs */ public function __construct($log_table) { @@ -53,6 +55,8 @@ class phpbb_log implements phpbb_log_interface /** * This function allows disable the log-system. When add_log is called, the log will not be added to the database. + * + * @return null */ public function disable() { @@ -61,6 +65,8 @@ class phpbb_log implements phpbb_log_interface /** * This function allows re-enable the log-system. + * + * @return null */ public function enable() { From 61cbabb120dfca6d924fbb08645f6dfbbcc5c1ec Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 13:59:32 +0100 Subject: [PATCH 11/56] [ticket/10714] Add missing log_operation to events in phpbb_log PHPBB3-10714 --- phpBB/includes/log/log.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 67336d232a..db774e48d5 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -157,7 +157,7 @@ class phpbb_log implements phpbb_log_interface * if ($phpbb_dispatcher != null) { - $vars = array('mode', 'user_id', 'log_ip', 'log_time', 'additional_data', 'sql_ary'); + $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); $event = new phpbb_event_data(compact($vars)); $phpbb_dispatcher->dispatch('core.add_log_case', $event); extract($event->get_data_filtered($vars)); @@ -176,7 +176,7 @@ class phpbb_log implements phpbb_log_interface * if ($phpbb_dispatcher != null) { - $vars = array('mode', 'user_id', 'log_ip', 'log_time', 'additional_data', 'sql_ary'); + $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); $event = new phpbb_event_data(compact($vars)); $phpbb_dispatcher->dispatch('core.add_log', $event); extract($event->get_data_filtered($vars)); From a0b35f8e4e94d1301421670cf35406b974510ed0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 18 Mar 2012 21:56:15 +0100 Subject: [PATCH 12/56] [ticket/10714] Use {@inheritDoc} instead of repeating the doc-block PHPBB3-10714 --- phpBB/includes/log/log.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index db774e48d5..aa60c453e4 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -76,14 +76,7 @@ class phpbb_log implements phpbb_log_interface /** * Adds a log to the database * - * @param string $mode The mode defines which log_type is used and in which log the entry is displayed. - * @param int $user_id User ID of the user - * @param string $log_ip IP address of the user - * @param string $log_operation Name of the operation - * @param int $log_time Timestamp when the log was added. - * @param array $additional_data More arguments can be added, depending on the log_type - * - * @return int|bool Returns the log_id, if the entry was added to the database, false otherwise. + * {@inheritDoc} */ public function add($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = array()) { From ea652f0ec9e7046f329e692fc355e64836f9bf9d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 21 Mar 2012 13:33:25 +0100 Subject: [PATCH 13/56] [ticket/10714] Rename add_log_function test PHPBB3-10714 --- tests/log/function_add_log_test.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index 407aeb9ad1..1f54f66d2d 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -16,7 +16,7 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/empty_log.xml'); } - public static function test_add_log_function_critical_data() + public static function test_add_log_function_data() { return array( array( @@ -109,9 +109,9 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case } /** - * @dataProvider test_add_log_function_critical_data + * @dataProvider test_add_log_function_data */ - public function test_add_log_function_critical($expected, $user_id, $mode, $required1, $additional1 = null, $additional2 = null, $additional3 = null) + public function test_add_log_function($expected, $user_id, $mode, $required1, $additional1 = null, $additional2 = null, $additional3 = null) { global $db, $user; From 920cb1a0de10febca3c78ef13286a49838656ab2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 21 Mar 2012 13:38:02 +0100 Subject: [PATCH 14/56] [ticket/10714] Add unit tests for view_log function PHPBB3-10714 --- tests/log/fixtures/full_log.xml | 166 +++++++++++++ tests/log/function_view_log_test.php | 341 +++++++++++++++++++++++++++ 2 files changed, 507 insertions(+) create mode 100644 tests/log/fixtures/full_log.xml create mode 100644 tests/log/function_view_log_test.php diff --git a/tests/log/fixtures/full_log.xml b/tests/log/fixtures/full_log.xml new file mode 100644 index 0000000000..2ce2643d26 --- /dev/null +++ b/tests/log/fixtures/full_log.xml @@ -0,0 +1,166 @@ + + + + log_id + log_type + user_id + forum_id + topic_id + reportee_id + log_ip + log_time + log_operation + log_data + + 1 + 0 + 1 + 0 + 0 + 0 + 127.0.0.1 + 1 + LOG_INSTALL_INSTALLED + a:1:{i:0;s:9:"3.1.0-dev";} + + + 2 + 0 + 1 + 0 + 0 + 0 + 127.0.0.1 + 1 + LOG_KEY_NOT_EXISTS + a:1:{i:0;s:15:"additional_data";} + + + 3 + 2 + 1 + 0 + 0 + 0 + 127.0.0.1 + 1 + LOG_CRITICAL + a:1:{i:0;s:13:"critical data";} + + + 4 + 1 + 1 + 12 + 34 + 0 + 127.0.0.1 + 1 + LOG_MOD + + + + 5 + 1 + 1 + 12 + 45 + 0 + 127.0.0.1 + 1 + LOG_MOD + + + + 6 + 1 + 1 + 23 + 56 + 0 + 127.0.0.1 + 1 + LOG_MOD + + + + 7 + 1 + 1 + 12 + 45 + 0 + 127.0.0.1 + 1 + LOG_MOD2 + + + + 8 + 3 + 1 + 0 + 0 + 2 + 127.0.0.1 + 1 + LOG_USER + a:1:{i:0;s:5:"admin";} + + + 9 + 3 + 1 + 0 + 0 + 1 + 127.0.0.1 + 1 + LOG_USER + a:1:{i:0;s:5:"guest";} + +
+ + user_id + username + username_clean + user_permissions + user_sig + user_occ + user_interests + + 1 + Anonymous + Anonymous + + + + + + + 2 + admin + admin + + + + + +
+ + topic_id + forum_id + + 34 + 12 + + + 45 + 12 + + + 56 + 23 + +
+ diff --git a/tests/log/function_view_log_test.php b/tests/log/function_view_log_test.php new file mode 100644 index 0000000000..7c44413330 --- /dev/null +++ b/tests/log/function_view_log_test.php @@ -0,0 +1,341 @@ +createXMLDataSet(dirname(__FILE__) . '/fixtures/full_log.xml'); + } + + public static function test_view_log_function_data() + { + global $phpEx; + + $expected_data_sets = array( + 1 => array( + 'id' => 1, + + 'reportee_id' => 0, + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 0, + 'topic_id' => 0, + + 'viewforum' => '', + 'action' => 'installed: 3.1.0-dev', + ), + 2 => array( + 'id' => 2, + + 'reportee_id' => 0, + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 0, + 'topic_id' => 0, + + 'viewforum' => '', + 'action' => '{LOG KEY NOT EXISTS}
additional_data', + ), + 3 => array( + 'id' => 3, + + 'reportee_id' => 0, + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 0, + 'topic_id' => 0, + + 'viewforum' => '', + 'action' => '{LOG CRITICAL}
critical data', + ), + 4 => array( + 'id' => 4, + + 'reportee_id' => 0, + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 12, + 'topic_id' => 34, + + 'viewforum' => '', + 'action' => '{LOG MOD}', + 'viewtopic' => '', + 'viewlogs' => '', + ), + 5 => array( + 'id' => 5, + + 'reportee_id' => 0, + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 12, + 'topic_id' => 45, + + 'viewforum' => '', + 'action' => '{LOG MOD}', + 'viewtopic' => '', + 'viewlogs' => '', + ), + 6 => array( + 'id' => 6, + + 'reportee_id' => 0, + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 23, + 'topic_id' => 56, + + 'viewforum' => append_sid("phpBB/viewforum.$phpEx", 'f=23'), + 'action' => '{LOG MOD}', + 'viewtopic' => append_sid("phpBB/viewtopic.$phpEx", 'f=23&t=56'), + 'viewlogs' => append_sid("phpBB/mcp.$phpEx", 'i=logs&mode=topic_logs&t=56'), + ), + 7 => array( + 'id' => 7, + + 'reportee_id' => 0, + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 12, + 'topic_id' => 45, + + 'viewforum' => '', + 'action' => '{LOG MOD2}', + 'viewtopic' => '', + 'viewlogs' => '', + ), + 8 => array( + 'id' => 8, + + 'reportee_id' => 2, + 'reportee_username' => 'admin', + 'reportee_username_full'=> 'admin', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 0, + 'topic_id' => 0, + + 'viewforum' => '', + 'action' => '{LOG USER}
admin', + ), + 9 => array( + 'id' => 9, + + 'reportee_id' => 1, + 'reportee_username' => 'Anonymous', + 'reportee_username_full'=> 'Anonymous', + + 'user_id' => 1, + 'username' => 'Anonymous', + 'username_full' => 'Anonymous', + + 'ip' => '127.0.0.1', + 'time' => 1, + 'forum_id' => 0, + 'topic_id' => 0, + + 'viewforum' => '', + 'action' => '{LOG USER}
guest', + ), + ); + + $test_cases = array( + array( + 'expected' => array(1, 2), + 'expected_returned' => 0, + false, + 'admin', + ), + array( + 'expected' => array(1), + 'expected_returned' => 0, + false, + 'admin', 1, + ), + array( + 'expected' => array(2), + 'expected_returned' => 1, + false, + 'admin', 1, 1, + ), + array( + 'expected' => array(2), + 'expected_returned' => 1, + 0, + 'admin', 1, 1, + ), + array( + 'expected' => array(2), + 'expected_returned' => 1, + 0, + 'admin', 1, 5, + ), + array( + 'expected' => array(3), + 'expected_returned' => 0, + false, + 'critical', + ), + array( + 'expected' => array(), + 'expected_returned' => null, + false, + 'mode_does_not_exist', + ), + array( + 'expected' => array(4, 5, 7), + 'expected_returned' => 0, + 0, + 'mod', 5, 0, 12, + ), + array( + 'expected' => array(5, 7), + 'expected_returned' => 0, + 0, + 'mod', 5, 0, 12, 45, + ), + array( + 'expected' => array(6), + 'expected_returned' => 0, + 0, + 'mod', 5, 0, 23, + ), + array( + 'expected' => array(8), + 'expected_returned' => 0, + 0, + 'user', 5, 0, 0, 0, 2, + ), + array( + 'expected' => array(8, 9), + 'expected_returned' => 0, + 0, + 'users', + ), + ); + + foreach ($test_cases as $case => $case_data) + { + foreach ($case_data['expected'] as $data_set => $expected) + { + $test_cases[$case]['expected'][$data_set] = $expected_data_sets[$expected]; + } + } + + return $test_cases; + } + + /** + * @dataProvider test_view_log_function_data + */ + public function test_view_log_function($expected, $expected_returned, $log_count, $mode, $limit = 5, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_id ASC', $keywords = '') + { + global $cache, $db, $user, $auth; + + $db = $this->new_dbal(); + $cache = new phpbb_mock_cache; + + // Create auth mock + $auth = $this->getMock('auth'); + $acl_get_map = array( + array('f_read', 23, true), + array('m_', 23, true), + ); + $acl_gets_map = array( + array('a_', 'm_', 23, true), + ); + + $auth->expects($this->any()) + ->method('acl_get') + ->with($this->stringContains('_'), + $this->anything()) + ->will($this->returnValueMap($acl_get_map)); + $auth->expects($this->any()) + ->method('acl_gets') + ->with($this->stringContains('_'), + $this->anything()) + ->will($this->returnValueMap($acl_gets_map)); + + $user = new phpbb_mock_user; + $user->optionset('viewcensors', false); + // Test sprintf() of the data into the action + $user->lang = array( + 'LOG_INSTALL_INSTALLED' => 'installed: %s', + ); + + $log = array(); + $this->assertEquals($expected_returned, view_log($mode, $log, $log_count, $limit, $offset, $forum_id, $topic_id, $user_id, $limit_days, $sort_by, $keywords)); + + $this->assertEquals($expected, $log); + } +} From 91384d8395166ec21995103410e35f7ba28ac830 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 15:05:02 +0100 Subject: [PATCH 15/56] [ticket/10714] Add casts to integer values. PHPBB3-10714 --- phpBB/includes/functions_admin.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/functions_admin.php b/phpBB/includes/functions_admin.php index 5e2ee8c8f6..e05ed3cdde 100644 --- a/phpBB/includes/functions_admin.php +++ b/phpBB/includes/functions_admin.php @@ -2603,6 +2603,7 @@ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id $log = array(); while ($row = $db->sql_fetchrow($result)) { + $row['forum_id'] = (int) $row['forum_id']; if ($row['topic_id']) { $topic_id_list[] = $row['topic_id']; @@ -2614,20 +2615,20 @@ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id } $log[$i] = array( - 'id' => $row['log_id'], + 'id' => (int) $row['log_id'], - 'reportee_id' => $row['reportee_id'], + 'reportee_id' => (int) $row['reportee_id'], 'reportee_username' => '', 'reportee_username_full'=> '', - 'user_id' => $row['user_id'], + 'user_id' => (int) $row['user_id'], 'username' => $row['username'], 'username_full' => get_username_string('full', $row['user_id'], $row['username'], $row['user_colour'], false, $profile_url), 'ip' => $row['log_ip'], - 'time' => $row['log_time'], - 'forum_id' => $row['forum_id'], - 'topic_id' => $row['topic_id'], + 'time' => (int) $row['log_time'], + 'forum_id' => (int) $row['forum_id'], + 'topic_id' => (int) $row['topic_id'], 'viewforum' => ($row['forum_id'] && $auth->acl_get('f_read', $row['forum_id'])) ? append_sid("{$phpbb_root_path}viewforum.$phpEx", 'f=' . $row['forum_id']) : false, 'action' => (isset($user->lang[$row['log_operation']])) ? $user->lang[$row['log_operation']] : '{' . ucfirst(str_replace('_', ' ', $row['log_operation'])) . '}', @@ -2689,6 +2690,7 @@ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id while ($row = $db->sql_fetchrow($result)) { + $row['forum_id'] = (int) $row['forum_id']; if ($auth->acl_get('f_read', $row['forum_id'])) { $is_auth[$row['topic_id']] = $row['forum_id']; From f5063a6eda49d2a35b2aed486f86cde76e0f04a8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 15:06:13 +0100 Subject: [PATCH 16/56] [ticket/10714] Add incorrect offset calculation in view_log function PHPBB3-10714 --- phpBB/includes/functions_admin.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions_admin.php b/phpBB/includes/functions_admin.php index e05ed3cdde..fd1f5568ab 100644 --- a/phpBB/includes/functions_admin.php +++ b/phpBB/includes/functions_admin.php @@ -2584,9 +2584,13 @@ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id return 0; } - if ($offset >= $log_count) + if ($log_count) { - $offset = ($offset - $limit < 0) ? 0 : $offset - $limit; + // Return the user to the last page that is valid + while ($offset >= $log_count) + { + $offset = ($offset - $limit < 0) ? 0 : $offset - $limit; + } } $sql = "SELECT l.*, u.username, u.username_clean, u.user_colour From 9248b9b25fdc3c05cc9fb1e99f607817f8ec7bcb Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 15:06:32 +0100 Subject: [PATCH 17/56] [ticket/10714] Add doc block for view_log function PHPBB3-10714 --- phpBB/includes/functions_admin.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/functions_admin.php b/phpBB/includes/functions_admin.php index fd1f5568ab..49c34f7fff 100644 --- a/phpBB/includes/functions_admin.php +++ b/phpBB/includes/functions_admin.php @@ -2470,7 +2470,21 @@ function cache_moderators() /** * View log -* If $log_count is set to false, we will skip counting all entries in the database. +* +* @param string $mode The mode defines which log_type is used and in which log the entry is displayed. +* @param array &$log The result array with the logs +* @param mixed &$log_count If $log_count is set to false, we will skip counting all entries in the database. +* Otherwise an integer with the number of total matching entries is returned. +* @param int $limit Limit the number of entries that are returned +* @param int $offset Offset when fetching the log entries, f.e. on paginations +* @param mixed $forum_id Restrict the log entries to the given forum_id (can also be an array of forum_ids) +* @param int $topic_id Restrict the log entries to the given topic_id +* @param int $user_id Restrict the log entries to the given user_id +* @param int $log_time Only get log entries newer than the given timestamp +* @param string $sort_by SQL order option, e.g. 'l.log_time DESC' +* @param string $keywords Will only return log entries that have the keywords in log_operation or log_data +* +* @return int Returns the offset of the last valid page, if the specified offset was invalid (too high) */ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_time DESC', $keywords = '') { From 55b94af82ecb7e73535bfbed6c278f1d992efecb Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 16:27:52 +0100 Subject: [PATCH 18/56] [ticket/10714] Implement get_logs() based on view_log() I moved some stuff into its own function to make the code a bit clearer. PHPBB3-10714 --- phpBB/includes/log/interface.php | 64 +++++ phpBB/includes/log/log.php | 406 +++++++++++++++++++++++++++++++ 2 files changed, 470 insertions(+) diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index 7eda4b9710..fde718b71a 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -56,4 +56,68 @@ interface phpbb_log_interface * @return int|bool Returns the log_id, if the entry was added to the database, false otherwise. */ public function add($mode, $user_id, $log_ip, $log_operation, $log_time, $additional_data); + + /** + * Grab the logs from the database + * + * @param string $mode The mode defines which log_type is used and in which log the entry is displayed. + * @param bool $count_logs Shall we count all matching log entries? + * @param int $limit Limit the number of entries that are returned + * @param int $offset Offset when fetching the log entries, f.e. on paginations + * @param mixed $forum_id Restrict the log entries to the given forum_id (can also be an array of forum_ids) + * @param int $topic_id Restrict the log entries to the given topic_id + * @param int $user_id Restrict the log entries to the given user_id + * @param int $log_time Only get log entries newer than the given timestamp + * @param string $sort_by SQL order option, e.g. 'l.log_time DESC' + * @param string $keywords Will only return log entries that have the keywords in log_operation or log_data + * + * @return array The result array with the logs + */ + public function get_logs($mode, $count_logs = true, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $log_time = 0, $sort_by = 'l.log_time DESC', $keywords = ''); + + /** + * Generates a sql condition out of the specified keywords + * + * @param string $keywords The keywords the user specified to search for + * + * @return string Returns the SQL condition searching for the keywords + */ + static public function generate_sql_keyword($keywords); + + /** + * Determinate whether the user is allowed to read and/or moderate the forum of the topic + * + * @param array $topic_ids Array with the topic ids + * + * @return array Returns an array with two keys 'm_' and 'read_f' which are also an array of topic_id => forum_id sets when the permissions are given. Sample: + * array( + * 'permission' => array( + * topic_id => forum_id + * ), + * ), + */ + static public function get_topic_auth($topic_ids); + + /** + * Get the data for all reportee form the database + * + * @param array $reportee_ids Array with the user ids of the reportees + * + * @return array Returns an array with the reportee data + */ + static public function get_reportee_data($reportee_ids); + + /** + * Get total log count + * + * @return int Returns the number of matching logs from the last call to get_logs() + */ + public function get_log_count(); + + /** + * Get offset of the last valid page + * + * @return int Returns the offset of the last valid page from the last call to get_logs() + */ + public function get_valid_offset(); } diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index aa60c453e4..14f8bfd534 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -27,6 +27,16 @@ class phpbb_log implements phpbb_log_interface */ private $enabled; + /** + * Keeps the total log count of the last call to get_logs() + */ + private $logs_total; + + /** + * Keeps the offset of the last valid page of the last call to get_logs() + */ + private $logs_offset; + /** * The table we use to store our logs. */ @@ -180,4 +190,400 @@ class phpbb_log implements phpbb_log_interface return $db->sql_nextid(); } + + /** + * Grab the logs from the database + * + * {@inheritDoc} + */ + public function get_logs($mode, $count_logs = true, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $log_time = 0, $sort_by = 'l.log_time DESC', $keywords = '') + { + global $db, $user, $auth, $phpEx, $phpbb_root_path, $phpbb_admin_path; + + $this->logs_total = 0; + $this->logs_offset = $offset; + + $topic_id_list = $reportee_id_list = array(); + + $profile_url = (defined('IN_ADMIN')) ? append_sid("{$phpbb_admin_path}index.$phpEx", 'i=users&mode=overview') : append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile'); + + switch ($mode) + { + case 'admin': + $log_type = LOG_ADMIN; + $sql_additional = ''; + break; + + case 'mod': + $log_type = LOG_MOD; + $sql_additional = ''; + + if ($topic_id) + { + $sql_additional = 'AND l.topic_id = ' . (int) $topic_id; + } + else if (is_array($forum_id)) + { + $sql_additional = 'AND ' . $db->sql_in_set('l.forum_id', array_map('intval', $forum_id)); + } + else if ($forum_id) + { + $sql_additional = 'AND l.forum_id = ' . (int) $forum_id; + } + break; + + case 'user': + $log_type = LOG_USERS; + $sql_additional = 'AND l.reportee_id = ' . (int) $user_id; + break; + + case 'users': + $log_type = LOG_USERS; + $sql_additional = ''; + break; + + case 'critical': + $log_type = LOG_CRITICAL; + $sql_additional = ''; + break; + + default: + $log_type = null; + $sql_additional = ''; + /** + * @todo: enable when events are merged + * + if ($phpbb_dispatcher != null) + { + $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); + $event = new phpbb_event_data(compact($vars)); + $phpbb_dispatcher->dispatch('core.get_logs_switch_mode', $event); + extract($event->get_data_filtered($vars)); + } + */ + + if (!isset($log_type)) + { + $this->logs_offset = 0; + return array(); + } + } + + /** + * @todo: enable when events are merged + * + if ($phpbb_dispatcher != null) + { + $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); + $event = new phpbb_event_data(compact($vars)); + $phpbb_dispatcher->dispatch('core.get_logs_after_get_type', $event); + extract($event->get_data_filtered($vars)); + } + */ + + $sql_keywords = ''; + if (!empty($keywords)) + { + // Get the SQL condition for our keywords + $sql_keywords = self::generate_sql_keyword($keywords); + } + + if ($count_logs) + { + $sql = 'SELECT COUNT(l.log_id) AS total_entries + FROM ' . LOG_TABLE . ' l, ' . USERS_TABLE . " u + WHERE l.log_type = $log_type + AND l.user_id = u.user_id + AND l.log_time >= $log_time + $sql_keywords + $sql_additional"; + $result = $db->sql_query($sql); + $this->logs_total = (int) $db->sql_fetchfield('total_entries'); + $db->sql_freeresult($result); + + if ($this->logs_total == 0) + { + // Save the queries, because there are no logs to display + $this->logs_offset = 0; + return array(); + } + + // Return the user to the last page that is valid + while ($this->logs_offset >= $this->logs_total) + { + $this->logs_offset = ($this->logs_offset - $limit < 0) ? 0 : $this->logs_offset - $limit; + } + } + + $sql = "SELECT l.*, u.username, u.username_clean, u.user_colour + FROM " . LOG_TABLE . " l, " . USERS_TABLE . " u + WHERE l.log_type = $log_type + AND u.user_id = l.user_id + " . (($log_time) ? "AND l.log_time >= $log_time" : '') . " + $sql_keywords + $sql_additional + ORDER BY $sort_by"; + $result = $db->sql_query_limit($sql, $limit, $this->logs_offset); + + $i = 0; + $log = array(); + while ($row = $db->sql_fetchrow($result)) + { + $row['forum_id'] = (int) $row['forum_id']; + if ($row['topic_id']) + { + $topic_id_list[] = (int) $row['topic_id']; + } + + if ($row['reportee_id']) + { + $reportee_id_list[] = (int) $row['reportee_id']; + } + + $log_entry_data = array( + 'id' => (int) $row['log_id'], + + 'reportee_id' => (int) $row['reportee_id'], + 'reportee_username' => '', + 'reportee_username_full'=> '', + + 'user_id' => (int) $row['user_id'], + 'username' => $row['username'], + 'username_full' => get_username_string('full', $row['user_id'], $row['username'], $row['user_colour'], false, $profile_url), + + 'ip' => $row['log_ip'], + 'time' => (int) $row['log_time'], + 'forum_id' => (int) $row['forum_id'], + 'topic_id' => (int) $row['topic_id'], + + 'viewforum' => ($row['forum_id'] && $auth->acl_get('f_read', $row['forum_id'])) ? append_sid("{$phpbb_root_path}viewforum.$phpEx", 'f=' . $row['forum_id']) : false, + 'action' => (isset($user->lang[$row['log_operation']])) ? $user->lang[$row['log_operation']] : '{' . ucfirst(str_replace('_', ' ', $row['log_operation'])) . '}', + ); + + /** + * @todo: enable when events are merged + * + if ($phpbb_dispatcher != null) + { + $vars = array('log_entry_data', 'row'); + $event = new phpbb_event_data(compact($vars)); + $phpbb_dispatcher->dispatch('core.get_logs_entry_data', $event); + extract($event->get_data_filtered($vars)); + } + */ + + $log[$i] = $log_entry_data; + + if (!empty($row['log_data'])) + { + $log_data_ary = @unserialize($row['log_data']); + $log_data_ary = ($log_data_ary === false) ? array() : $log_data_ary; + + if (isset($user->lang[$row['log_operation']])) + { + // Check if there are more occurrences of % than arguments, if there are we fill out the arguments array + // It doesn't matter if we add more arguments than placeholders + if ((substr_count($log[$i]['action'], '%') - sizeof($log_data_ary)) > 0) + { + $log_data_ary = array_merge($log_data_ary, array_fill(0, substr_count($log[$i]['action'], '%') - sizeof($log_data_ary), '')); + } + + $log[$i]['action'] = vsprintf($log[$i]['action'], $log_data_ary); + + // If within the admin panel we do not censor text out + if (defined('IN_ADMIN')) + { + $log[$i]['action'] = bbcode_nl2br($log[$i]['action']); + } + else + { + $log[$i]['action'] = bbcode_nl2br(censor_text($log[$i]['action'])); + } + } + else if (!empty($log_data_ary)) + { + $log[$i]['action'] .= '
' . implode('', $log_data_ary); + } + + /* Apply make_clickable... has to be seen if it is for good. :/ + // Seems to be not for the moment, reconsider later... + $log[$i]['action'] = make_clickable($log[$i]['action']); + */ + } + + $i++; + } + $db->sql_freeresult($result); + + /** + * @todo: enable when events are merged + * + if ($phpbb_dispatcher != null) + { + $vars = array('log', 'topic_id_list', 'reportee_id_list'); + $event = new phpbb_event_data(compact($vars)); + $phpbb_dispatcher->dispatch('core.get_logs_additional_data', $event); + extract($event->get_data_filtered($vars)); + } + */ + + if (sizeof($topic_id_list)) + { + $topic_auth = self::get_topic_auth($topic_id_list); + + foreach ($log as $key => $row) + { + $log[$key]['viewtopic'] = (isset($topic_auth['f_read'][$row['topic_id']])) ? append_sid("{$phpbb_root_path}viewtopic.$phpEx", 'f=' . $topic_auth['f_read'][$row['topic_id']] . '&t=' . $row['topic_id']) : false; + $log[$key]['viewlogs'] = (isset($topic_auth['m_'][$row['topic_id']])) ? append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=logs&mode=topic_logs&t=' . $row['topic_id'], true, $user->session_id) : false; + } + } + + if (sizeof($reportee_id_list)) + { + $reportee_data_list = self::get_reportee_data($reportee_id_list); + + foreach ($log as $key => $row) + { + if (!isset($reportee_data_list[$row['reportee_id']])) + { + continue; + } + + $log[$key]['reportee_username'] = $reportee_data_list[$row['reportee_id']]['username']; + $log[$key]['reportee_username_full'] = get_username_string('full', $row['reportee_id'], $reportee_data_list[$row['reportee_id']]['username'], $reportee_data_list[$row['reportee_id']]['user_colour'], false, $profile_url); + } + } + + return $log; + } + + /** + * Generates a sql condition out of the specified keywords + * + * {@inheritDoc} + */ + static public function generate_sql_keyword($keywords) + { + global $db, $user; + + // Use no preg_quote for $keywords because this would lead to sole backslashes being added + // We also use an OR connection here for spaces and the | string. Currently, regex is not supported for searching (but may come later). + $keywords = preg_split('#[\s|]+#u', utf8_strtolower($keywords), 0, PREG_SPLIT_NO_EMPTY); + $sql_keywords = ''; + + if (!empty($keywords)) + { + $keywords_pattern = array(); + + // Build pattern and keywords... + for ($i = 0, $num_keywords = sizeof($keywords); $i < $num_keywords; $i++) + { + $keywords_pattern[] = preg_quote($keywords[$i], '#'); + $keywords[$i] = $db->sql_like_expression($db->any_char . $keywords[$i] . $db->any_char); + } + + $keywords_pattern = '#' . implode('|', $keywords_pattern) . '#ui'; + + $operations = array(); + foreach ($user->lang as $key => $value) + { + if (substr($key, 0, 4) == 'LOG_' && preg_match($keywords_pattern, $value)) + { + $operations[] = $key; + } + } + + $sql_keywords = 'AND ('; + if (!empty($operations)) + { + $sql_keywords .= $db->sql_in_set('l.log_operation', $operations) . ' OR '; + } + $sql_keywords .= 'LOWER(l.log_data) ' . implode(' OR LOWER(l.log_data) ', $keywords) . ')'; + } + + return $sql_keywords; + } + + /** + * Determinate whether the user is allowed to read and/or moderate the forum of the topic + * + * {@inheritDoc} + */ + static public function get_topic_auth($topic_ids) + { + global $auth, $db; + + $forum_auth = array('f_read' => array(), 'm_' => array()); + $topic_ids = array_unique($topic_ids); + + $sql = 'SELECT topic_id, forum_id + FROM ' . TOPICS_TABLE . ' + WHERE ' . $db->sql_in_set('topic_id', array_map('intval', $topic_ids)); + $result = $db->sql_query($sql); + + while ($row = $db->sql_fetchrow($result)) + { + $row['topic_id'] = (int) $row['topic_id']; + $row['forum_id'] = (int) $row['forum_id']; + + if ($auth->acl_get('f_read', $row['forum_id'])) + { + $forum_auth['f_read'][$row['topic_id']] = $row['forum_id']; + } + + if ($auth->acl_gets('a_', 'm_', $row['forum_id'])) + { + $forum_auth['m_'][$row['topic_id']] = $row['forum_id']; + } + } + $db->sql_freeresult($result); + + return $forum_auth; + } + + /** + * Get the data for all reportee form the database + * + * {@inheritDoc} + */ + static public function get_reportee_data($reportee_ids) + { + global $db; + + $reportee_ids = array_unique($reportee_ids); + $reportee_data_list = array(); + + $sql = 'SELECT user_id, username, user_colour + FROM ' . USERS_TABLE . ' + WHERE ' . $db->sql_in_set('user_id', $reportee_ids); + $result = $db->sql_query($sql); + + while ($row = $db->sql_fetchrow($result)) + { + $reportee_data_list[$row['user_id']] = $row; + } + $db->sql_freeresult($result); + + return $reportee_data_list; + } + + /** + * Get total log count + * + * @return int Returns the number of matching logs from the last call to get_logs() + */ + public function get_log_count() + { + return ($this->logs_total) ? $this->logs_total : 0; + } + + /** + * Get offset of the last valid log page + * + * @return int Returns the offset of the last valid page from the last call to get_logs() + */ + public function get_valid_offset() + { + return ($this->logs_offset) ? $this->logs_offset : 0; + } } From 97290647fae683ecce842541a682e3403b7717ee Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 16:39:03 +0100 Subject: [PATCH 19/56] [ticket/10714] Use phpbb_log class in view_log() PHPBB3-10714 --- phpBB/includes/functions_admin.php | 277 ++--------------------------- phpBB/includes/log/log.php | 105 +++++++---- 2 files changed, 87 insertions(+), 295 deletions(-) diff --git a/phpBB/includes/functions_admin.php b/phpBB/includes/functions_admin.php index 49c34f7fff..e7aed85e15 100644 --- a/phpBB/includes/functions_admin.php +++ b/phpBB/includes/functions_admin.php @@ -2488,275 +2488,34 @@ function cache_moderators() */ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_time DESC', $keywords = '') { - global $db, $user, $auth, $phpEx, $phpbb_root_path, $phpbb_admin_path; + // This is all just an ugly hack to add "Dependency Injection" to a function + // the only real code is the function call which maps this function to a method. + static $static_log = null; - $topic_id_list = $reportee_id_list = $is_auth = $is_mod = array(); - - $profile_url = (defined('IN_ADMIN')) ? append_sid("{$phpbb_admin_path}index.$phpEx", 'i=users&mode=overview') : append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile'); - - switch ($mode) + if ($mode instanceof phpbb_log_interface) { - case 'admin': - $log_type = LOG_ADMIN; - $sql_forum = ''; - break; - - case 'mod': - $log_type = LOG_MOD; - $sql_forum = ''; - - if ($topic_id) - { - $sql_forum = 'AND l.topic_id = ' . (int) $topic_id; - } - else if (is_array($forum_id)) - { - $sql_forum = 'AND ' . $db->sql_in_set('l.forum_id', array_map('intval', $forum_id)); - } - else if ($forum_id) - { - $sql_forum = 'AND l.forum_id = ' . (int) $forum_id; - } - break; - - case 'user': - $log_type = LOG_USERS; - $sql_forum = 'AND l.reportee_id = ' . (int) $user_id; - break; - - case 'users': - $log_type = LOG_USERS; - $sql_forum = ''; - break; - - case 'critical': - $log_type = LOG_CRITICAL; - $sql_forum = ''; - break; - - default: - return; + $static_log = $mode; + return true; + } + else if ($mode === false) + { + return false; } - // Use no preg_quote for $keywords because this would lead to sole backslashes being added - // We also use an OR connection here for spaces and the | string. Currently, regex is not supported for searching (but may come later). - $keywords = preg_split('#[\s|]+#u', utf8_strtolower($keywords), 0, PREG_SPLIT_NO_EMPTY); - $sql_keywords = ''; + $tmp_log = $static_log; - if (!empty($keywords)) + // no log class set, create a temporary one ourselves to keep backwards compatability + if ($tmp_log === null) { - $keywords_pattern = array(); - - // Build pattern and keywords... - for ($i = 0, $num_keywords = sizeof($keywords); $i < $num_keywords; $i++) - { - $keywords_pattern[] = preg_quote($keywords[$i], '#'); - $keywords[$i] = $db->sql_like_expression($db->any_char . $keywords[$i] . $db->any_char); - } - - $keywords_pattern = '#' . implode('|', $keywords_pattern) . '#ui'; - - $operations = array(); - foreach ($user->lang as $key => $value) - { - if (substr($key, 0, 4) == 'LOG_' && preg_match($keywords_pattern, $value)) - { - $operations[] = $key; - } - } - - $sql_keywords = 'AND ('; - if (!empty($operations)) - { - $sql_keywords .= $db->sql_in_set('l.log_operation', $operations) . ' OR '; - } - $sql_lower = $db->sql_lower_text('l.log_data'); - $sql_keywords .= "$sql_lower " . implode(" OR $sql_lower ", $keywords) . ')'; + $tmp_log = new phpbb_log(LOG_TABLE); } - if ($log_count !== false) - { - $sql = 'SELECT COUNT(l.log_id) AS total_entries - FROM ' . LOG_TABLE . ' l, ' . USERS_TABLE . " u - WHERE l.log_type = $log_type - AND l.user_id = u.user_id - AND l.log_time >= $limit_days - $sql_keywords - $sql_forum"; - $result = $db->sql_query($sql); - $log_count = (int) $db->sql_fetchfield('total_entries'); - $db->sql_freeresult($result); - } + $count_logs = ($log_count !== false); - // $log_count may be false here if false was passed in for it, - // because in this case we did not run the COUNT() query above. - // If we ran the COUNT() query and it returned zero rows, return; - // otherwise query for logs below. - if ($log_count === 0) - { - // Save the queries, because there are no logs to display - return 0; - } + $log = $tmp_log->get_logs($mode, $count_logs, $limit, $offset, $forum_id, $topic_id, $user_id, $limit_days, $sort_by, $keywords); + $log_count = $tmp_log->get_log_count(); - if ($log_count) - { - // Return the user to the last page that is valid - while ($offset >= $log_count) - { - $offset = ($offset - $limit < 0) ? 0 : $offset - $limit; - } - } - - $sql = "SELECT l.*, u.username, u.username_clean, u.user_colour - FROM " . LOG_TABLE . " l, " . USERS_TABLE . " u - WHERE l.log_type = $log_type - AND u.user_id = l.user_id - " . (($limit_days) ? "AND l.log_time >= $limit_days" : '') . " - $sql_keywords - $sql_forum - ORDER BY $sort_by"; - $result = $db->sql_query_limit($sql, $limit, $offset); - - $i = 0; - $log = array(); - while ($row = $db->sql_fetchrow($result)) - { - $row['forum_id'] = (int) $row['forum_id']; - if ($row['topic_id']) - { - $topic_id_list[] = $row['topic_id']; - } - - if ($row['reportee_id']) - { - $reportee_id_list[] = $row['reportee_id']; - } - - $log[$i] = array( - 'id' => (int) $row['log_id'], - - 'reportee_id' => (int) $row['reportee_id'], - 'reportee_username' => '', - 'reportee_username_full'=> '', - - 'user_id' => (int) $row['user_id'], - 'username' => $row['username'], - 'username_full' => get_username_string('full', $row['user_id'], $row['username'], $row['user_colour'], false, $profile_url), - - 'ip' => $row['log_ip'], - 'time' => (int) $row['log_time'], - 'forum_id' => (int) $row['forum_id'], - 'topic_id' => (int) $row['topic_id'], - - 'viewforum' => ($row['forum_id'] && $auth->acl_get('f_read', $row['forum_id'])) ? append_sid("{$phpbb_root_path}viewforum.$phpEx", 'f=' . $row['forum_id']) : false, - 'action' => (isset($user->lang[$row['log_operation']])) ? $user->lang[$row['log_operation']] : '{' . ucfirst(str_replace('_', ' ', $row['log_operation'])) . '}', - ); - - if (!empty($row['log_data'])) - { - $log_data_ary = @unserialize($row['log_data']); - $log_data_ary = ($log_data_ary === false) ? array() : $log_data_ary; - - if (isset($user->lang[$row['log_operation']])) - { - // Check if there are more occurrences of % than arguments, if there are we fill out the arguments array - // It doesn't matter if we add more arguments than placeholders - if ((substr_count($log[$i]['action'], '%') - sizeof($log_data_ary)) > 0) - { - $log_data_ary = array_merge($log_data_ary, array_fill(0, substr_count($log[$i]['action'], '%') - sizeof($log_data_ary), '')); - } - - $log[$i]['action'] = vsprintf($log[$i]['action'], $log_data_ary); - - // If within the admin panel we do not censor text out - if (defined('IN_ADMIN')) - { - $log[$i]['action'] = bbcode_nl2br($log[$i]['action']); - } - else - { - $log[$i]['action'] = bbcode_nl2br(censor_text($log[$i]['action'])); - } - } - else if (!empty($log_data_ary)) - { - $log[$i]['action'] .= '
' . implode('', $log_data_ary); - } - - /* Apply make_clickable... has to be seen if it is for good. :/ - // Seems to be not for the moment, reconsider later... - $log[$i]['action'] = make_clickable($log[$i]['action']); - */ - } - - $i++; - } - $db->sql_freeresult($result); - - if (sizeof($topic_id_list)) - { - $topic_id_list = array_unique($topic_id_list); - - // This query is not really needed if move_topics() updates the forum_id field, - // although it's also used to determine if the topic still exists in the database - $sql = 'SELECT topic_id, forum_id - FROM ' . TOPICS_TABLE . ' - WHERE ' . $db->sql_in_set('topic_id', array_map('intval', $topic_id_list)); - $result = $db->sql_query($sql); - - $default_forum_id = 0; - - while ($row = $db->sql_fetchrow($result)) - { - $row['forum_id'] = (int) $row['forum_id']; - if ($auth->acl_get('f_read', $row['forum_id'])) - { - $is_auth[$row['topic_id']] = $row['forum_id']; - } - - if ($auth->acl_gets('a_', 'm_', $row['forum_id'])) - { - $is_mod[$row['topic_id']] = $row['forum_id']; - } - } - $db->sql_freeresult($result); - - foreach ($log as $key => $row) - { - $log[$key]['viewtopic'] = (isset($is_auth[$row['topic_id']])) ? append_sid("{$phpbb_root_path}viewtopic.$phpEx", 'f=' . $is_auth[$row['topic_id']] . '&t=' . $row['topic_id']) : false; - $log[$key]['viewlogs'] = (isset($is_mod[$row['topic_id']])) ? append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=logs&mode=topic_logs&t=' . $row['topic_id'], true, $user->session_id) : false; - } - } - - if (sizeof($reportee_id_list)) - { - $reportee_id_list = array_unique($reportee_id_list); - $reportee_names_list = array(); - - $sql = 'SELECT user_id, username, user_colour - FROM ' . USERS_TABLE . ' - WHERE ' . $db->sql_in_set('user_id', $reportee_id_list); - $result = $db->sql_query($sql); - - while ($row = $db->sql_fetchrow($result)) - { - $reportee_names_list[$row['user_id']] = $row; - } - $db->sql_freeresult($result); - - foreach ($log as $key => $row) - { - if (!isset($reportee_names_list[$row['reportee_id']])) - { - continue; - } - - $log[$key]['reportee_username'] = $reportee_names_list[$row['reportee_id']]['username']; - $log[$key]['reportee_username_full'] = get_username_string('full', $row['reportee_id'], $reportee_names_list[$row['reportee_id']]['username'], $reportee_names_list[$row['reportee_id']]['user_colour'], false, $profile_url); - } - } - - return $offset; + return $tmp_log->get_valid_offset(); } /** diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 14f8bfd534..5d81dd8495 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -25,7 +25,7 @@ class phpbb_log implements phpbb_log_interface /** * Keeps the status of the log-system. Is the log enabled or disabled? */ - private $enabled; + private $disabled_logs; /** * Keeps the total log count of the last call to get_logs() @@ -56,31 +56,70 @@ class phpbb_log implements phpbb_log_interface /** * This function returns the state of the log-system. * - * @return bool True if log is enabled + * @param string $type The log type we want to check. Empty to get global log status. + * + * @return bool True if log for the type is enabled */ - public function is_enabled() + public function is_enabled($type = '') { - return $this->enabled; + if ($type == '' || $type == 'all') + { + return !isset($this->disabled_logs['all']); + } + return !isset($this->disabled_logs[$type]) && !isset($this->disabled_logs['all']); } /** * This function allows disable the log-system. When add_log is called, the log will not be added to the database. * + * @param mixed $type The log type we want to enable. Empty to disable all logs. + * Can also be an array of types + * * @return null */ - public function disable() + public function disable($type = '') { - $this->enabled = false; + if (is_array($type)) + { + foreach ($type as $disable_type) + { + $this->disable($disable_type); + } + return; + } + + if ($type == '' || $type == 'all') + { + $this->disabled_logs['all'] = true; + return; + } + $this->disabled_logs[$type] = true; } /** * This function allows re-enable the log-system. * + * @param mixed $type The log type we want to enable. Empty to enable all logs. + * * @return null */ - public function enable() + public function enable($type = '') { - $this->enabled = true; + if (is_array($type)) + { + foreach ($type as $enable_type) + { + $this->enable($enable_type); + } + return; + } + + if ($type == '' || $type == 'all') + { + $this->disabled_logs = array(); + return; + } + unset($this->disabled_logs[$type]); } /** @@ -90,7 +129,7 @@ class phpbb_log implements phpbb_log_interface */ public function add($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = array()) { - if (!$this->is_enabled()) + if (!$this->is_enabled($mode)) { return false; } @@ -119,7 +158,7 @@ class phpbb_log implements phpbb_log_interface case 'admin': $sql_ary += array( 'log_type' => LOG_ADMIN, - 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + 'log_data' => (!empty($additional_data)) ? serialize($additional_data) : '', ); break; @@ -132,7 +171,7 @@ class phpbb_log implements phpbb_log_interface 'log_type' => LOG_MOD, 'forum_id' => $forum_id, 'topic_id' => $topic_id, - 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + 'log_data' => (!empty($additional_data)) ? serialize($additional_data) : '', ); break; @@ -143,14 +182,14 @@ class phpbb_log implements phpbb_log_interface $sql_ary += array( 'log_type' => LOG_USERS, 'reportee_id' => $reportee_id, - 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + 'log_data' => (!empty($additional_data)) ? serialize($additional_data) : '', ); break; case 'critical': $sql_ary += array( 'log_type' => LOG_CRITICAL, - 'log_data' => (!sizeof($additional_data)) ? '' : serialize($additional_data), + 'log_data' => (!empty($additional_data)) ? serialize($additional_data) : '', ); break; @@ -161,9 +200,7 @@ class phpbb_log implements phpbb_log_interface if ($phpbb_dispatcher != null) { $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); - $event = new phpbb_event_data(compact($vars)); - $phpbb_dispatcher->dispatch('core.add_log_case', $event); - extract($event->get_data_filtered($vars)); + extract($phpbb_dispatcher->trigger_event('core.add_log_case', $vars, $vars)); } */ @@ -180,9 +217,7 @@ class phpbb_log implements phpbb_log_interface if ($phpbb_dispatcher != null) { $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); - $event = new phpbb_event_data(compact($vars)); - $phpbb_dispatcher->dispatch('core.add_log', $event); - extract($event->get_data_filtered($vars)); + extract($phpbb_dispatcher->trigger_event('core.add_log', $vars, $vars)); } */ @@ -199,6 +234,11 @@ class phpbb_log implements phpbb_log_interface public function get_logs($mode, $count_logs = true, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $log_time = 0, $sort_by = 'l.log_time DESC', $keywords = '') { global $db, $user, $auth, $phpEx, $phpbb_root_path, $phpbb_admin_path; + /** + * @todo: enable when events are merged + * + global $db, $user, $auth, $phpEx, $phpbb_root_path, $phpbb_admin_path, $phpbb_dispatcher; + */ $this->logs_total = 0; $this->logs_offset = $offset; @@ -256,9 +296,7 @@ class phpbb_log implements phpbb_log_interface if ($phpbb_dispatcher != null) { $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); - $event = new phpbb_event_data(compact($vars)); - $phpbb_dispatcher->dispatch('core.get_logs_switch_mode', $event); - extract($event->get_data_filtered($vars)); + extract($phpbb_dispatcher->trigger_event('core.get_logs_switch_mode', $vars, $vars)); } */ @@ -275,9 +313,7 @@ class phpbb_log implements phpbb_log_interface if ($phpbb_dispatcher != null) { $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); - $event = new phpbb_event_data(compact($vars)); - $phpbb_dispatcher->dispatch('core.get_logs_after_get_type', $event); - extract($event->get_data_filtered($vars)); + extract($phpbb_dispatcher->trigger_event('core.get_logs_after_get_type', $vars, $vars)); } */ @@ -311,12 +347,12 @@ class phpbb_log implements phpbb_log_interface // Return the user to the last page that is valid while ($this->logs_offset >= $this->logs_total) { - $this->logs_offset = ($this->logs_offset - $limit < 0) ? 0 : $this->logs_offset - $limit; + $this->logs_offset = max(0, $this->logs_offset - $limit); } } - $sql = "SELECT l.*, u.username, u.username_clean, u.user_colour - FROM " . LOG_TABLE . " l, " . USERS_TABLE . " u + $sql = 'SELECT l.*, u.username, u.username_clean, u.user_colour + FROM ' . LOG_TABLE . ' l, ' . USERS_TABLE . " u WHERE l.log_type = $log_type AND u.user_id = l.user_id " . (($log_time) ? "AND l.log_time >= $log_time" : '') . " @@ -366,9 +402,7 @@ class phpbb_log implements phpbb_log_interface if ($phpbb_dispatcher != null) { $vars = array('log_entry_data', 'row'); - $event = new phpbb_event_data(compact($vars)); - $phpbb_dispatcher->dispatch('core.get_logs_entry_data', $event); - extract($event->get_data_filtered($vars)); + extract($phpbb_dispatcher->trigger_event('core.get_logs_entry_data', $vars, $vars)); } */ @@ -377,7 +411,7 @@ class phpbb_log implements phpbb_log_interface if (!empty($row['log_data'])) { $log_data_ary = @unserialize($row['log_data']); - $log_data_ary = ($log_data_ary === false) ? array() : $log_data_ary; + $log_data_ary = ($log_data_ary !== false) ? $log_data_ary : array(); if (isset($user->lang[$row['log_operation']])) { @@ -421,9 +455,7 @@ class phpbb_log implements phpbb_log_interface if ($phpbb_dispatcher != null) { $vars = array('log', 'topic_id_list', 'reportee_id_list'); - $event = new phpbb_event_data(compact($vars)); - $phpbb_dispatcher->dispatch('core.get_logs_additional_data', $event); - extract($event->get_data_filtered($vars)); + extract($phpbb_dispatcher->trigger_event('core.get_logs_additional_data', $vars, $vars)); } */ @@ -498,7 +530,8 @@ class phpbb_log implements phpbb_log_interface { $sql_keywords .= $db->sql_in_set('l.log_operation', $operations) . ' OR '; } - $sql_keywords .= 'LOWER(l.log_data) ' . implode(' OR LOWER(l.log_data) ', $keywords) . ')'; + $sql_lower = $db->sql_lower_text('l.log_data'); + $sql_keywords .= " $sql_lower " . implode(" OR $sql_lower ", $keywords) . ')'; } return $sql_keywords; From 325827c40f778ef7efd3c195706cb0b1fb28805b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 16:43:26 +0100 Subject: [PATCH 20/56] [ticket/10714] Inject the global $phpbb_log into view_log() PHPBB3-10714 --- phpBB/common.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phpBB/common.php b/phpBB/common.php index bbcf8b894f..799c1162b1 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -76,6 +76,7 @@ if (!empty($load_extensions) && function_exists('dl')) require($phpbb_root_path . 'includes/class_loader.' . $phpEx); require($phpbb_root_path . 'includes/functions.' . $phpEx); +require($phpbb_root_path . 'includes/functions_admin.' . $phpEx); require($phpbb_root_path . 'includes/functions_content.' . $phpEx); require($phpbb_root_path . 'includes/constants.' . $phpEx); @@ -142,6 +143,10 @@ foreach ($cache->obtain_hooks() as $hook) // make sure add_log uses this log instance $phpbb_log = new phpbb_log(LOG_TABLE); add_log($phpbb_log); // "dependency injection" for a function +// Parameter 2 and 3 are passed by reference, so we need to create a variable for it. +$tmp_var = ''; +view_log($phpbb_log, $tmp_var, $tmp_var); // "dependency injection" for a function +unset($tmp_var); if (!$config['use_system_cron']) { From 1e00c697b766d1a8695c8e058334efe1dd3dbb7e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 17:02:56 +0100 Subject: [PATCH 21/56] [ticket/10714] Add docblock for the test cases PHPBB3-10714 --- tests/log/function_add_log_test.php | 29 ++++++++++++++++ tests/log/function_view_log_test.php | 50 ++++++++++++++-------------- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index 1f54f66d2d..e7b3c4335f 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -19,6 +19,35 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case public static function test_add_log_function_data() { return array( + /** + * Case documentation + array( + // Row that is in the database afterwards + array( + 'user_id' => ANONYMOUS, + 'log_type' => LOG_MOD, + 'log_operation' => 'LOG_MOD_ADDITIONAL', + // log_data will be serialized + 'log_data' => array( + 'argument3', + ), + 'reportee_id' => 0, + 'forum_id' => 56, + 'topic_id' => 78, + ), + // user_id Can also be false, than ANONYMOUS is used + false, + // log_mode Used to determinate the log_type + 'mod', + // Followed by some additional arguments + // forum_id, topic_id and reportee_id are specified before log_operation + // The rest is specified afterwards. + 56, + 78, + 'LOG_MOD_ADDITIONAL', // log_operation + 'argument3', + ), + */ array( array( 'user_id' => 2, diff --git a/tests/log/function_view_log_test.php b/tests/log/function_view_log_test.php index 7c44413330..78bc2843a8 100644 --- a/tests/log/function_view_log_test.php +++ b/tests/log/function_view_log_test.php @@ -210,77 +210,77 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case ); $test_cases = array( + /** + * Case documentation + array( + // Array of datasets that should be in $log after running the function + 'expected' => array(5, 7), + // Offset that will be returned form the function + 'expected_returned' => 0, + // view_log parameters (see includes/functions_admin.php for docblock) + // $log is ommited! + 'mod', 5, 0, 12, 45, + ), + */ array( 'expected' => array(1, 2), 'expected_returned' => 0, - false, - 'admin', + 'admin', false, ), array( 'expected' => array(1), 'expected_returned' => 0, - false, - 'admin', 1, + 'admin', false, 1, ), array( 'expected' => array(2), 'expected_returned' => 1, - false, - 'admin', 1, 1, + 'admin', false, 1, 1, ), array( 'expected' => array(2), 'expected_returned' => 1, - 0, - 'admin', 1, 1, + 'admin', 0, 1, 1, ), array( 'expected' => array(2), 'expected_returned' => 1, - 0, - 'admin', 1, 5, + 'admin', 0, 1, 5, ), array( 'expected' => array(3), 'expected_returned' => 0, - false, - 'critical', + 'critical', false, ), array( 'expected' => array(), 'expected_returned' => null, - false, - 'mode_does_not_exist', + 'mode_does_not_exist', false, ), array( 'expected' => array(4, 5, 7), 'expected_returned' => 0, - 0, - 'mod', 5, 0, 12, + 'mod', 0, 5, 0, 12, ), array( 'expected' => array(5, 7), 'expected_returned' => 0, - 0, - 'mod', 5, 0, 12, 45, + 'mod', 0, 5, 0, 12, 45, ), array( 'expected' => array(6), 'expected_returned' => 0, - 0, - 'mod', 5, 0, 23, + 'mod', 0, 5, 0, 23, ), array( 'expected' => array(8), 'expected_returned' => 0, - 0, - 'user', 5, 0, 0, 0, 2, + 'user', 0, 5, 0, 0, 0, 2, ), array( 'expected' => array(8, 9), 'expected_returned' => 0, - 0, - 'users', + 'users', 0, ), ); @@ -298,7 +298,7 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case /** * @dataProvider test_view_log_function_data */ - public function test_view_log_function($expected, $expected_returned, $log_count, $mode, $limit = 5, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_id ASC', $keywords = '') + public function test_view_log_function($expected, $expected_returned, $mode, $log_count, $limit = 5, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_id ASC', $keywords = '') { global $cache, $db, $user, $auth; From 2c7f498c1b43cfb96f868e9b0f9b80ad5ec626a8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sat, 24 Mar 2012 17:13:17 +0100 Subject: [PATCH 22/56] [ticket/10714] Change $phpbb_dispatcher calls to the new layout PHPBB3-10714 --- phpBB/includes/functions.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 3e3d796ba2..c5a1543277 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3407,9 +3407,7 @@ function add_log() if ($phpbb_dispatcher != null) { $vars = array('mode', 'args', 'additional_data'); - $event = new phpbb_event_data(compact($vars)); - $phpbb_dispatcher->dispatch('core.function_add_log', $event); - extract($event->get_data_filtered($vars)); + extract($phpbb_dispatcher->trigger_event('core.function_add_log', $vars, $vars)); } */ } From 3170845a5011ea76af7f4f8359acafb43ad7e19e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 28 Mar 2012 15:48:45 +0200 Subject: [PATCH 23/56] [ticket/10714] Refactor disable mechanism to only disable certain types Only disable admin log when adding multiple users, so critical errors are still logged. PHPBB3-10714 --- phpBB/includes/functions_user.php | 4 ++-- phpBB/includes/log/interface.php | 16 ++++++++++++---- tests/log/add_test.php | 28 +++++++++++++++++++++++----- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 4074eaa2f2..d1f1544fcd 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -302,7 +302,7 @@ function user_add($user_row, $cp_data = false) global $phpbb_log; // Because these actions only fill the log unneccessarily we skip the add_log() entry. - $phpbb_log->disable(); + $phpbb_log->disable('admin'); // Add user to "newly registered users" group and set to default group if admin specified so. if ($config['new_member_group_default']) @@ -315,7 +315,7 @@ function user_add($user_row, $cp_data = false) group_user_add($add_group_id, $user_id); } - $phpbb_log->enable(); + $phpbb_log->enable('admin'); } } diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index fde718b71a..feeab585bf 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -25,23 +25,31 @@ interface phpbb_log_interface /** * This function returns the state of the log-system. * - * @return bool True if log is enabled + * @param string $type The log type we want to check. Empty to get global log status. + * + * @return bool True if log for the type is enabled */ - public function is_enabled(); + public function is_enabled($type = ''); /** * This function allows disable the log-system. When add_log is called, the log will not be added to the database. * + * @param mixed $type The log type we want to disable. Empty to disable all logs. + * Can also be an array of types + * * @return null */ - public function disable(); + public function disable($type = ''); /** * This function allows re-enable the log-system. * + * @param mixed $type The log type we want to enable. Empty to enable all logs. + * Can also be an array of types + * * @return null */ - public function enable(); + public function enable($type = ''); /** * Adds a log to the database diff --git a/tests/log/add_test.php b/tests/log/add_test.php index a2b763f2b7..faf5953836 100644 --- a/tests/log/add_test.php +++ b/tests/log/add_test.php @@ -19,13 +19,21 @@ class phpbb_log_add_test extends phpbb_database_test_case public function test_log_enabled() { $log = new phpbb_log(LOG_TABLE); - $this->assertTrue($log->is_enabled()); + $this->assertTrue($log->is_enabled(), 'Initialise failed'); $log->disable(); - $this->assertFalse($log->is_enabled()); + $this->assertFalse($log->is_enabled(), 'Disable all failed'); $log->enable(); - $this->assertTrue($log->is_enabled()); + $this->assertTrue($log->is_enabled(), 'Enable all failed'); + + $log->disable('admin'); + $this->assertFalse($log->is_enabled('admin'), 'Disable admin failed'); + $this->assertTrue($log->is_enabled('user'), 'User should be enabled, is disabled'); + $this->assertTrue($log->is_enabled(), 'Disable admin disabled all'); + + $log->enable('admin'); + $this->assertTrue($log->is_enabled('admin'), 'Enable admin failed'); } public function test_log_add() @@ -45,9 +53,19 @@ class phpbb_log_add_test extends phpbb_database_test_case $log = new phpbb_log(LOG_TABLE); $this->assertEquals(1, $log->add($mode, $user_id, $log_ip, $log_operation, $log_time)); - // Disable logging + // Disable logging for all types $log->disable(); - $this->assertFalse($log->add($mode, $user_id, $log_ip, $log_operation, $log_time)); + $this->assertFalse($log->add($mode, $user_id, $log_ip, $log_operation, $log_time), 'Disable for all types failed'); + $log->enable(); + + // Disable logging for same type + $log->disable('critical'); + $this->assertFalse($log->add($mode, $user_id, $log_ip, $log_operation, $log_time), 'Disable for same type failed'); + $log->enable(); + + // Disable logging for different type + $log->disable('admin'); + $this->assertEquals(2, $log->add($mode, $user_id, $log_ip, $log_operation, $log_time), 'Disable for different types failed'); $log->enable(); // Invalid mode specified From 0fcbb40a0e1affcfa07a6d51ce273b73b3a95359 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 12:40:55 +0200 Subject: [PATCH 24/56] [ticket/10714] Enable core.add_log event and remove superseded add_log_case PHPBB3-10714 --- phpBB/includes/log/log.php | 46 ++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 5d81dd8495..752ed21955 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -134,12 +134,7 @@ class phpbb_log implements phpbb_log_interface return false; } - global $db; - /** - * @todo: enable when events are merged - * global $db, $phpbb_dispatcher; - */ if ($log_time == false) { @@ -192,34 +187,37 @@ class phpbb_log implements phpbb_log_interface 'log_data' => (!empty($additional_data)) ? serialize($additional_data) : '', ); break; - - default: - /** - * @todo: enable when events are merged - * - if ($phpbb_dispatcher != null) - { - $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); - extract($phpbb_dispatcher->trigger_event('core.add_log_case', $vars, $vars)); - } - */ - - // We didn't find a log_type, so we don't save it in the database. - if (!isset($sql_ary['log_type'])) - { - return false; - } } /** - * @todo: enable when events are merged + * Allow to modify log data before we add them to the database * + * NOTE: if sql_ary does not contain a log_type value, the entry will + * not be stored in the database. So ensure to set it, if needed. + * + * @event core.add_log + * @var string mode Mode of the entry we log + * @var int user_id ID of the user who triggered the log + * @var string log_ip IP of the user who triggered the log + * @var string log_operation Language key of the log operation + * @var int log_time Timestamp, when the log was added + * @var array additional_data Array with additional log data + * @var array sql_ary Array with log data we insert into the + * database. If sql_ary[log_type] is not set, + * we won't add the entry to the database. + * @since 3.1-A1 + */ if ($phpbb_dispatcher != null) { $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); extract($phpbb_dispatcher->trigger_event('core.add_log', $vars, $vars)); } - */ + + // We didn't find a log_type, so we don't save it in the database. + if (!isset($sql_ary['log_type'])) + { + return false; + } $db->sql_query('INSERT INTO ' . $this->log_table . ' ' . $db->sql_build_array('INSERT', $sql_ary)); From bd6dfee23e0d3f11ff028d4376e73c9c1e770f2a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 13:06:43 +0200 Subject: [PATCH 25/56] [ticket/10714] Add event core.get_logs_modify_type core.get_logs_switch_mode is superseded by this one and therefor removed PHPBB3-10714 --- phpBB/includes/log/log.php | 53 +++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 752ed21955..97d0aac623 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -231,12 +231,7 @@ class phpbb_log implements phpbb_log_interface */ public function get_logs($mode, $count_logs = true, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $log_time = 0, $sort_by = 'l.log_time DESC', $keywords = '') { - global $db, $user, $auth, $phpEx, $phpbb_root_path, $phpbb_admin_path; - /** - * @todo: enable when events are merged - * global $db, $user, $auth, $phpEx, $phpbb_root_path, $phpbb_admin_path, $phpbb_dispatcher; - */ $this->logs_total = 0; $this->logs_offset = $offset; @@ -288,32 +283,44 @@ class phpbb_log implements phpbb_log_interface default: $log_type = null; $sql_additional = ''; - /** - * @todo: enable when events are merged - * - if ($phpbb_dispatcher != null) - { - $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_switch_mode', $vars, $vars)); - } - */ - - if (!isset($log_type)) - { - $this->logs_offset = 0; - return array(); - } } /** - * @todo: enable when events are merged + * Overwrite log type and limitations before we count and get the logs * + * NOTE: if log_type is not set, no entries will be returned. + * + * @event core.get_logs_modify_type + * @var string mode Mode of the entries we display + * @var bool count_logs Do we count all matching entries? + * @var int limit Limit the number of entries + * @var int offset Offset when fetching the entries + * @var mixed forum_id Limit entries to the forum_id, + * can also be an array of forum_ids + * @var int topic_id Limit entries to the topic_id + * @var int user_id Limit entries to the user_id + * @var int log_time Limit maximum age of log entries + * @var string sort_by SQL order option + * @var string keywords Will only return entries that have the + * keywords in log_operation or log_data + * @var string profile_url URL to the users profile + * @var int log_type Limit logs to a certain type. If log_type + * is not set, no entries will be returned. + * @var string sql_additional Additional conditions for the entries, + * e.g.: 'AND l.forum_id = 1' + * @since 3.1-A1 + */ if ($phpbb_dispatcher != null) { $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_after_get_type', $vars, $vars)); + extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_type', $vars)); + } + + if (!isset($log_type)) + { + $this->logs_offset = 0; + return array(); } - */ $sql_keywords = ''; if (!empty($keywords)) From 0bb4af90a4d9a1fedb254a1b6e9702726cfcf091 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 13:07:11 +0200 Subject: [PATCH 26/56] [ticket/10714] Fix core.add_log event PHPBB3-10714 --- phpBB/includes/log/log.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 97d0aac623..92730aa7da 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -210,7 +210,7 @@ class phpbb_log implements phpbb_log_interface if ($phpbb_dispatcher != null) { $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); - extract($phpbb_dispatcher->trigger_event('core.add_log', $vars, $vars)); + extract($phpbb_dispatcher->trigger_event('core.add_log', $vars)); } // We didn't find a log_type, so we don't save it in the database. From cf095dd393a6540d092c6308bc03aab824376562 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 13:12:50 +0200 Subject: [PATCH 27/56] [ticket/10714] Enable event core.get_logs_modify_entry_data PHPBB3-10714 --- phpBB/includes/log/log.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 92730aa7da..c95b334cad 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -402,14 +402,18 @@ class phpbb_log implements phpbb_log_interface ); /** - * @todo: enable when events are merged + * Modify the entry's data before it is returned * + * @event core.get_logs_modify_entry_data + * @var array row Entry data from the database + * @var array log_entry_data Entry's data which is returned + * @since 3.1-A1 + */ if ($phpbb_dispatcher != null) { - $vars = array('log_entry_data', 'row'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_entry_data', $vars, $vars)); + $vars = array('row', 'log_entry_data'); + extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_entry_data', $vars)); } - */ $log[$i] = $log_entry_data; From 701052481542cad1ab46fb5e48cf5bbd139030b8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 13:19:13 +0200 Subject: [PATCH 28/56] [ticket/10714] Enable event core.get_logs_get_additional_data PHPBB3-10714 --- phpBB/includes/log/log.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index c95b334cad..ef74f6aaf4 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -459,14 +459,21 @@ class phpbb_log implements phpbb_log_interface $db->sql_freeresult($result); /** - * @todo: enable when events are merged + * Get some additional data after we got all log entries * + * @event core.get_logs_get_additional_data + * @var array log Array with all our log entries + * @var array topic_id_list Array of topic ids, for which we + * get the permission data + * @var array reportee_id_list Array of additional user IDs we + * get the username strings for + * @since 3.1-A1 + */ if ($phpbb_dispatcher != null) { $vars = array('log', 'topic_id_list', 'reportee_id_list'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_additional_data', $vars, $vars)); + extract($phpbb_dispatcher->trigger_event('core.get_logs_get_additional_data', $vars)); } - */ if (sizeof($topic_id_list)) { From 151346c6e053e925b5cfcf2845eca532509bbc80 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 13:25:26 +0200 Subject: [PATCH 29/56] [ticket/10714] Remove event core.function_add_log, add_log should be used instead PHPBB3-10714 --- phpBB/includes/functions.php | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index c5a1543277..b5d0c4d62f 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3398,18 +3398,6 @@ function add_log() case 'user': $additional_data['reportee_id'] = array_shift($args); break; - default: - /** - * @todo: enable when events are merged - * - global $phpbb_dispatcher; - - if ($phpbb_dispatcher != null) - { - $vars = array('mode', 'args', 'additional_data'); - extract($phpbb_dispatcher->trigger_event('core.function_add_log', $vars, $vars)); - } - */ } $log_operation = array_shift($args); $additional_data = array_merge($additional_data, $args); From 2afbec5ac425b8913c2d3e3193b195190b7185db Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 15:50:46 +0200 Subject: [PATCH 30/56] [ticket/10714] Always try to trigger events on phpbb_dispatcher We may add the if () later again, if we decide to do that. PHPBB3-10714 --- phpBB/includes/log/log.php | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index ef74f6aaf4..780881ae13 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -207,11 +207,8 @@ class phpbb_log implements phpbb_log_interface * we won't add the entry to the database. * @since 3.1-A1 */ - if ($phpbb_dispatcher != null) - { - $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); - extract($phpbb_dispatcher->trigger_event('core.add_log', $vars)); - } + $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); + extract($phpbb_dispatcher->trigger_event('core.add_log', $vars)); // We didn't find a log_type, so we don't save it in the database. if (!isset($sql_ary['log_type'])) @@ -310,11 +307,8 @@ class phpbb_log implements phpbb_log_interface * e.g.: 'AND l.forum_id = 1' * @since 3.1-A1 */ - if ($phpbb_dispatcher != null) - { - $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_type', $vars)); - } + $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); + extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_type', $vars)); if (!isset($log_type)) { @@ -409,11 +403,8 @@ class phpbb_log implements phpbb_log_interface * @var array log_entry_data Entry's data which is returned * @since 3.1-A1 */ - if ($phpbb_dispatcher != null) - { - $vars = array('row', 'log_entry_data'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_entry_data', $vars)); - } + $vars = array('row', 'log_entry_data'); + extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_entry_data', $vars)); $log[$i] = $log_entry_data; @@ -469,11 +460,8 @@ class phpbb_log implements phpbb_log_interface * get the username strings for * @since 3.1-A1 */ - if ($phpbb_dispatcher != null) - { - $vars = array('log', 'topic_id_list', 'reportee_id_list'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_get_additional_data', $vars)); - } + $vars = array('log', 'topic_id_list', 'reportee_id_list'); + extract($phpbb_dispatcher->trigger_event('core.get_logs_get_additional_data', $vars)); if (sizeof($topic_id_list)) { From d828ef93f29eda5fe31a6f8291dd1e5b3cdfd97c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 16:00:01 +0200 Subject: [PATCH 31/56] [ticket/10714] Fix unit test because of events and moved files PHPBB3-10714 --- tests/log/add_test.php | 3 ++- tests/log/function_add_log_test.php | 3 ++- tests/log/function_view_log_test.php | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/log/add_test.php b/tests/log/add_test.php index faf5953836..fceb48ed01 100644 --- a/tests/log/add_test.php +++ b/tests/log/add_test.php @@ -38,9 +38,10 @@ class phpbb_log_add_test extends phpbb_database_test_case public function test_log_add() { - global $db; + global $db, $phpbb_dispatcher; $db = $this->new_dbal(); + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); $mode = 'critical'; $user_id = ANONYMOUS; diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index e7b3c4335f..77c4578baa 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -142,7 +142,7 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case */ public function test_add_log_function($expected, $user_id, $mode, $required1, $additional1 = null, $additional2 = null, $additional3 = null) { - global $db, $user; + global $db, $user, $phpbb_dispatcher; if ($expected) { @@ -155,6 +155,7 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case } $db = $this->new_dbal(); + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); $user->ip = 'user_ip'; if ($user_id) diff --git a/tests/log/function_view_log_test.php b/tests/log/function_view_log_test.php index 78bc2843a8..790597e9c8 100644 --- a/tests/log/function_view_log_test.php +++ b/tests/log/function_view_log_test.php @@ -7,13 +7,12 @@ * */ -require_once dirname(__FILE__) . '/../../phpBB/includes/auth.php'; require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; require_once dirname(__FILE__) . '/../../phpBB/includes/functions_admin.php'; require_once dirname(__FILE__) . '/../../phpBB/includes/functions_content.php'; require_once dirname(__FILE__) . '/../../phpBB/includes/utf/utf_tools.php'; require_once dirname(__FILE__) . '/../../phpBB/includes/session.php'; -require_once dirname(__FILE__) . '/../mock_user.php'; +require_once dirname(__FILE__) . '/../mock/user.php'; require_once dirname(__FILE__) . '/../mock/cache.php'; class phpbb_log_function_view_log_test extends phpbb_database_test_case @@ -300,13 +299,14 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case */ public function test_view_log_function($expected, $expected_returned, $mode, $log_count, $limit = 5, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_id ASC', $keywords = '') { - global $cache, $db, $user, $auth; + global $cache, $db, $user, $auth, $phpbb_dispatcher; $db = $this->new_dbal(); $cache = new phpbb_mock_cache; + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); // Create auth mock - $auth = $this->getMock('auth'); + $auth = $this->getMock('phpbb_auth'); $acl_get_map = array( array('f_read', 23, true), array('m_', 23, true), From d289bc13acc0ab0329cac25742ae22560a80c607 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 21 Aug 2012 16:49:08 +0200 Subject: [PATCH 32/56] [ticket/10714] Remove dependency injection and use global instead This avoids loading functions_admin.php globally and was suggested by naderman PHPBB3-10714 --- phpBB/common.php | 11 ++--------- phpBB/includes/functions.php | 19 +++++-------------- phpBB/includes/functions_admin.php | 26 ++++++-------------------- 3 files changed, 13 insertions(+), 43 deletions(-) diff --git a/phpBB/common.php b/phpBB/common.php index 799c1162b1..2d2b31df83 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -76,7 +76,6 @@ if (!empty($load_extensions) && function_exists('dl')) require($phpbb_root_path . 'includes/class_loader.' . $phpEx); require($phpbb_root_path . 'includes/functions.' . $phpEx); -require($phpbb_root_path . 'includes/functions_admin.' . $phpEx); require($phpbb_root_path . 'includes/functions_content.' . $phpEx); require($phpbb_root_path . 'includes/constants.' . $phpEx); @@ -119,6 +118,8 @@ $config = new phpbb_config_db($db, $cache->get_driver(), CONFIG_TABLE); set_config(null, null, null, $config); set_config_count(null, null, null, $config); +$phpbb_log = new phpbb_log(LOG_TABLE); + // load extensions $phpbb_extension_manager = new phpbb_extension_manager($db, EXT_TABLE, $phpbb_root_path, ".$phpEx", $cache->get_driver()); @@ -140,14 +141,6 @@ foreach ($cache->obtain_hooks() as $hook) @include($phpbb_root_path . 'includes/hooks/' . $hook . '.' . $phpEx); } -// make sure add_log uses this log instance -$phpbb_log = new phpbb_log(LOG_TABLE); -add_log($phpbb_log); // "dependency injection" for a function -// Parameter 2 and 3 are passed by reference, so we need to create a variable for it. -$tmp_var = ''; -view_log($phpbb_log, $tmp_var, $tmp_var); // "dependency injection" for a function -unset($tmp_var); - if (!$config['use_system_cron']) { $cron = new phpbb_cron_manager(new phpbb_cron_task_provider($phpbb_extension_manager), $cache->get_driver()); diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index b5d0c4d62f..e202273204 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3357,29 +3357,20 @@ function parse_cfg_file($filename, $lines = false) */ function add_log() { - // This is all just an ugly hack to add "Dependency Injection" to a function - // the only real code is the function call which maps this function to a method. - static $static_log = null; + global $phpbb_log; $args = func_get_args(); $log = (isset($args[0])) ? $args[0] : false; - if ($log instanceof phpbb_log_interface) - { - $static_log = $log; - return true; - } - else if ($log === false) + if ($log === false) { return false; } - $tmp_log = $static_log; - // no log class set, create a temporary one ourselves to keep backwards compatability - if ($tmp_log === null) + if ($phpbb_log === null) { - $tmp_log = new phpbb_log(LOG_TABLE); + $phpbb_log = new phpbb_log(LOG_TABLE); } $mode = array_shift($args); @@ -3407,7 +3398,7 @@ function add_log() $user_id = (empty($user->data)) ? ANONYMOUS : $user->data['user_id']; $user_ip = (empty($user->ip)) ? '' : $user->ip; - return $tmp_log->add($mode, $user_id, $user_ip, $log_operation, time(), $additional_data); + return $phpbb_log->add($mode, $user_id, $user_ip, $log_operation, time(), $additional_data); } /** diff --git a/phpBB/includes/functions_admin.php b/phpBB/includes/functions_admin.php index e7aed85e15..2a87feed51 100644 --- a/phpBB/includes/functions_admin.php +++ b/phpBB/includes/functions_admin.php @@ -2488,34 +2488,20 @@ function cache_moderators() */ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_time DESC', $keywords = '') { - // This is all just an ugly hack to add "Dependency Injection" to a function - // the only real code is the function call which maps this function to a method. - static $static_log = null; - - if ($mode instanceof phpbb_log_interface) - { - $static_log = $mode; - return true; - } - else if ($mode === false) - { - return false; - } - - $tmp_log = $static_log; + global $phpbb_log; // no log class set, create a temporary one ourselves to keep backwards compatability - if ($tmp_log === null) + if ($phpbb_log === null) { - $tmp_log = new phpbb_log(LOG_TABLE); + $phpbb_log = new phpbb_log(LOG_TABLE); } $count_logs = ($log_count !== false); - $log = $tmp_log->get_logs($mode, $count_logs, $limit, $offset, $forum_id, $topic_id, $user_id, $limit_days, $sort_by, $keywords); - $log_count = $tmp_log->get_log_count(); + $log = $phpbb_log->get_logs($mode, $count_logs, $limit, $offset, $forum_id, $topic_id, $user_id, $limit_days, $sort_by, $keywords); + $log_count = $phpbb_log->get_log_count(); - return $tmp_log->get_valid_offset(); + return $phpbb_log->get_valid_offset(); } /** From 35089bc0136e019724ee4b6c301fb94da52173cf Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 11 Nov 2012 12:18:04 +0100 Subject: [PATCH 33/56] [ticket/10714] Fix some comments PHPBB3-10714 --- phpBB/includes/functions.php | 2 +- phpBB/includes/log/interface.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index e202273204..93dfb4cd7f 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3367,7 +3367,7 @@ function add_log() return false; } - // no log class set, create a temporary one ourselves to keep backwards compatability + // no log class set, create a temporary one ourselves to keep backwards compatibility if ($phpbb_log === null) { $phpbb_log = new phpbb_log(LOG_TABLE); diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index feeab585bf..dd0df236f6 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -23,7 +23,7 @@ if (!defined('IN_PHPBB')) interface phpbb_log_interface { /** - * This function returns the state of the log-system. + * This function returns the state of the log system. * * @param string $type The log type we want to check. Empty to get global log status. * @@ -32,7 +32,7 @@ interface phpbb_log_interface public function is_enabled($type = ''); /** - * This function allows disable the log-system. When add_log is called, the log will not be added to the database. + * This function allows disable the log system. When add_log is called, the log will not be added to the database. * * @param mixed $type The log type we want to disable. Empty to disable all logs. * Can also be an array of types @@ -42,7 +42,7 @@ interface phpbb_log_interface public function disable($type = ''); /** - * This function allows re-enable the log-system. + * This function allows re-enable the log system. * * @param mixed $type The log type we want to enable. Empty to enable all logs. * Can also be an array of types @@ -93,7 +93,7 @@ interface phpbb_log_interface static public function generate_sql_keyword($keywords); /** - * Determinate whether the user is allowed to read and/or moderate the forum of the topic + * Determine whether the user is allowed to read and/or moderate the forum of the topic * * @param array $topic_ids Array with the topic ids * From 72d1cae3f35d6f72f341b06761642545c6c663d2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 11 Nov 2012 12:29:25 +0100 Subject: [PATCH 34/56] [ticket/10714] Remove some private functions from the log interface PHPBB3-10714 --- phpBB/includes/log/interface.php | 32 ------------------------- phpBB/includes/log/log.php | 41 ++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 47 deletions(-) diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index dd0df236f6..b85dc3a474 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -83,38 +83,6 @@ interface phpbb_log_interface */ public function get_logs($mode, $count_logs = true, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $log_time = 0, $sort_by = 'l.log_time DESC', $keywords = ''); - /** - * Generates a sql condition out of the specified keywords - * - * @param string $keywords The keywords the user specified to search for - * - * @return string Returns the SQL condition searching for the keywords - */ - static public function generate_sql_keyword($keywords); - - /** - * Determine whether the user is allowed to read and/or moderate the forum of the topic - * - * @param array $topic_ids Array with the topic ids - * - * @return array Returns an array with two keys 'm_' and 'read_f' which are also an array of topic_id => forum_id sets when the permissions are given. Sample: - * array( - * 'permission' => array( - * topic_id => forum_id - * ), - * ), - */ - static public function get_topic_auth($topic_ids); - - /** - * Get the data for all reportee form the database - * - * @param array $reportee_ids Array with the user ids of the reportees - * - * @return array Returns an array with the reportee data - */ - static public function get_reportee_data($reportee_ids); - /** * Get total log count * diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 780881ae13..eff084bbd2 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -23,7 +23,7 @@ if (!defined('IN_PHPBB')) class phpbb_log implements phpbb_log_interface { /** - * Keeps the status of the log-system. Is the log enabled or disabled? + * Keeps the status of the log system. Is the log enabled or disabled? */ private $disabled_logs; @@ -54,7 +54,7 @@ class phpbb_log implements phpbb_log_interface } /** - * This function returns the state of the log-system. + * This function returns the state of the log system. * * @param string $type The log type we want to check. Empty to get global log status. * @@ -70,7 +70,7 @@ class phpbb_log implements phpbb_log_interface } /** - * This function allows disable the log-system. When add_log is called, the log will not be added to the database. + * This function allows disable the log system. When add_log is called, the log will not be added to the database. * * @param mixed $type The log type we want to enable. Empty to disable all logs. * Can also be an array of types @@ -97,7 +97,7 @@ class phpbb_log implements phpbb_log_interface } /** - * This function allows re-enable the log-system. + * This function allows re-enable the log system. * * @param mixed $type The log type we want to enable. Empty to enable all logs. * @@ -320,7 +320,7 @@ class phpbb_log implements phpbb_log_interface if (!empty($keywords)) { // Get the SQL condition for our keywords - $sql_keywords = self::generate_sql_keyword($keywords); + $sql_keywords = $this->generate_sql_keyword($keywords); } if ($count_logs) @@ -465,7 +465,7 @@ class phpbb_log implements phpbb_log_interface if (sizeof($topic_id_list)) { - $topic_auth = self::get_topic_auth($topic_id_list); + $topic_auth = $this->get_topic_auth($topic_id_list); foreach ($log as $key => $row) { @@ -476,7 +476,7 @@ class phpbb_log implements phpbb_log_interface if (sizeof($reportee_id_list)) { - $reportee_data_list = self::get_reportee_data($reportee_id_list); + $reportee_data_list = $this->get_reportee_data($reportee_id_list); foreach ($log as $key => $row) { @@ -496,9 +496,11 @@ class phpbb_log implements phpbb_log_interface /** * Generates a sql condition out of the specified keywords * - * {@inheritDoc} + * @param string $keywords The keywords the user specified to search for + * + * @return string Returns the SQL condition searching for the keywords */ - static public function generate_sql_keyword($keywords) + private function generate_sql_keyword($keywords) { global $db, $user; @@ -542,11 +544,18 @@ class phpbb_log implements phpbb_log_interface } /** - * Determinate whether the user is allowed to read and/or moderate the forum of the topic + * Determine whether the user is allowed to read and/or moderate the forum of the topic * - * {@inheritDoc} + * @param array $topic_ids Array with the topic ids + * + * @return array Returns an array with two keys 'm_' and 'read_f' which are also an array of topic_id => forum_id sets when the permissions are given. Sample: + * array( + * 'permission' => array( + * topic_id => forum_id + * ), + * ), */ - static public function get_topic_auth($topic_ids) + private function get_topic_auth($topic_ids) { global $auth, $db; @@ -579,11 +588,13 @@ class phpbb_log implements phpbb_log_interface } /** - * Get the data for all reportee form the database + * Get the data for all reportee from the database * - * {@inheritDoc} + * @param array $reportee_ids Array with the user ids of the reportees + * + * @return array Returns an array with the reportee data */ - static public function get_reportee_data($reportee_ids) + private function get_reportee_data($reportee_ids) { global $db; From 5213ee1829c0ca4689d6137ac011cc759a727c59 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Dec 2012 14:39:57 +0100 Subject: [PATCH 35/56] [ticket/10714] Make attributed protected rather then private PHPBB3-10714 --- phpBB/includes/log/log.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index eff084bbd2..b6f275835c 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -25,22 +25,22 @@ class phpbb_log implements phpbb_log_interface /** * Keeps the status of the log system. Is the log enabled or disabled? */ - private $disabled_logs; + protected $disabled_logs; /** * Keeps the total log count of the last call to get_logs() */ - private $logs_total; + protected $logs_total; /** * Keeps the offset of the last valid page of the last call to get_logs() */ - private $logs_offset; + protected $logs_offset; /** * The table we use to store our logs. */ - private $log_table; + protected $log_table; /** * Constructor From 70d23380aa55c2aab33c1c2e5cea57f186314584 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Dec 2012 15:11:56 +0100 Subject: [PATCH 36/56] [ticket/10714] Rely on global instead of creating an instance PHPBB3-10714 --- phpBB/includes/functions.php | 6 ------ phpBB/includes/functions_admin.php | 6 ------ 2 files changed, 12 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 93dfb4cd7f..7c1a48f913 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3367,12 +3367,6 @@ function add_log() return false; } - // no log class set, create a temporary one ourselves to keep backwards compatibility - if ($phpbb_log === null) - { - $phpbb_log = new phpbb_log(LOG_TABLE); - } - $mode = array_shift($args); // This looks kind of dirty, but add_log has some additional data before the log_operation diff --git a/phpBB/includes/functions_admin.php b/phpBB/includes/functions_admin.php index 2a87feed51..8a1f34e76d 100644 --- a/phpBB/includes/functions_admin.php +++ b/phpBB/includes/functions_admin.php @@ -2490,12 +2490,6 @@ function view_log($mode, &$log, &$log_count, $limit = 0, $offset = 0, $forum_id { global $phpbb_log; - // no log class set, create a temporary one ourselves to keep backwards compatability - if ($phpbb_log === null) - { - $phpbb_log = new phpbb_log(LOG_TABLE); - } - $count_logs = ($log_count !== false); $log = $phpbb_log->get_logs($mode, $count_logs, $limit, $offset, $forum_id, $topic_id, $user_id, $limit_days, $sort_by, $keywords); From 0f94ff91383ee0ed533048db96a34164ba94b0a4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Dec 2012 16:05:57 +0100 Subject: [PATCH 37/56] [ticket/10714] Add global variables for the unit tests PHPBB3-10714 --- tests/log/function_add_log_test.php | 4 +++- tests/log/function_view_log_test.php | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index 77c4578baa..7ed862c523 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -142,7 +142,7 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case */ public function test_add_log_function($expected, $user_id, $mode, $required1, $additional1 = null, $additional2 = null, $additional3 = null) { - global $db, $user, $phpbb_dispatcher; + global $db, $cache, $user, $phpbb_log, $phpbb_dispatcher; if ($expected) { @@ -155,7 +155,9 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case } $db = $this->new_dbal(); + $cache = new phpbb_mock_cache; $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $phpbb_log = new phpbb_log(LOG_TABLE); $user->ip = 'user_ip'; if ($user_id) diff --git a/tests/log/function_view_log_test.php b/tests/log/function_view_log_test.php index 790597e9c8..f7e2c51c32 100644 --- a/tests/log/function_view_log_test.php +++ b/tests/log/function_view_log_test.php @@ -24,7 +24,8 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case public static function test_view_log_function_data() { - global $phpEx; + global $phpEx, $phpbb_dispatcher; + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); $expected_data_sets = array( 1 => array( @@ -299,11 +300,12 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case */ public function test_view_log_function($expected, $expected_returned, $mode, $log_count, $limit = 5, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_id ASC', $keywords = '') { - global $cache, $db, $user, $auth, $phpbb_dispatcher; + global $cache, $db, $user, $auth, $phpbb_log, $phpbb_dispatcher; $db = $this->new_dbal(); $cache = new phpbb_mock_cache; $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $phpbb_log = new phpbb_log(LOGS_TABLE); // Create auth mock $auth = $this->getMock('phpbb_auth'); From 7f1b0eeb711c57de82235d7893d793969ed0cfdd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 7 Dec 2012 16:28:46 +0100 Subject: [PATCH 38/56] [ticket/10714] Compare log_type to false, rather then null PHPBB3-10714 --- phpBB/includes/log/log.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index b6f275835c..c2ebedd6f2 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -278,14 +278,14 @@ class phpbb_log implements phpbb_log_interface break; default: - $log_type = null; + $log_type = false; $sql_additional = ''; } /** * Overwrite log type and limitations before we count and get the logs * - * NOTE: if log_type is not set, no entries will be returned. + * NOTE: if log_type is false, no entries will be returned. * * @event core.get_logs_modify_type * @var string mode Mode of the entries we display @@ -302,7 +302,7 @@ class phpbb_log implements phpbb_log_interface * keywords in log_operation or log_data * @var string profile_url URL to the users profile * @var int log_type Limit logs to a certain type. If log_type - * is not set, no entries will be returned. + * is false, no entries will be returned. * @var string sql_additional Additional conditions for the entries, * e.g.: 'AND l.forum_id = 1' * @since 3.1-A1 @@ -310,7 +310,7 @@ class phpbb_log implements phpbb_log_interface $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_type', $vars)); - if (!isset($log_type)) + if ($log_type === false) { $this->logs_offset = 0; return array(); From 83b8b65016f172baa65cbbb463015602c97e9e45 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 9 Dec 2012 23:47:46 +0100 Subject: [PATCH 39/56] [ticket/10714] Use dependencies instead of globals We use a setter for the admin root path, as it is not defined all the time. Aswell as we added a setter for the table name, so it can still be used for custom tables. PHPBB3-10714 --- phpBB/config/services.yml | 11 +++ phpBB/config/tables.yml | 1 + phpBB/includes/log/log.php | 163 ++++++++++++++++++++++++++----------- 3 files changed, 129 insertions(+), 46 deletions(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 37e6c0b5df..73d8680803 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -154,6 +154,17 @@ services: tags: - { name: kernel.event_subscriber } + log: + class: phpbb_log + arguments: + - @dbal.conn + - @user + - @auth + - @dispatcher + - %core.root_path% + - %core.php_ext% + - %tables.log% + request: class: phpbb_request diff --git a/phpBB/config/tables.yml b/phpBB/config/tables.yml index cfc6dbcfed..b506e6c6b3 100644 --- a/phpBB/config/tables.yml +++ b/phpBB/config/tables.yml @@ -1,3 +1,4 @@ parameters: tables.config: %core.table_prefix%config tables.ext: %core.table_prefix%ext + tables.log: %core.table_prefix%log diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index c2ebedd6f2..3d9620a2ee 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -42,15 +42,96 @@ class phpbb_log implements phpbb_log_interface */ protected $log_table; + /** + * Database object + * @var dbal + */ + protected $db; + + /** + * User object + * @var phpbb_user + */ + protected $user; + + /** + * Auth object + * @var phpbb_auth + */ + protected $auth; + + /** + * Event dispatcher object + * @var phpbb_dispatcher + */ + protected $dispatcher; + + /** + * phpBB root path + * @var string + */ + protected $phpbb_root_path; + + /** + * Admin root path + * @var string + */ + protected $phpbb_admin_path; + + /** + * PHP Extension + * @var string + */ + protected $php_ext; + /** * Constructor * - * @param string $log_table The table we use to store our logs + * @param dbal $db Database object + * @param phpbb_user $user User object + * @param phpbb_auth $auth Auth object + * @param phpbb_dispatcher $phpbb_dispatcher Event dispatcher + * @param string $phpbb_root_path Root path + * @param string $php_ext PHP Extension + * @param string $log_table Name of the table we use to store our logs + * @return null */ - public function __construct($log_table) + public function __construct(dbal $db, phpbb_user $user, phpbb_auth $auth, phpbb_dispatcher $phpbb_dispatcher, $phpbb_root_path, $php_ext, $log_table) + { + $this->db = $db; + $this->user = $user; + $this->auth = $auth; + $this->dispatcher = $phpbb_dispatcher; + $this->phpbb_root_path = $phpbb_root_path; + $this->php_ext = $php_ext; + $this->log_table = $log_table; + + $this->enable(); + $this->set_admin_path('', false); + } + + /** + * Set phpbb_admin_path and is_in_admin in order to return administrative user profile links in get_logs() + * + * @param string $phpbb_admin_path Full path from current file to admin root + * @param bool $is_in_admin Are we called from within the acp? + * @return null + */ + public function set_admin_path($phpbb_admin_path, $is_in_admin) + { + $this->phpbb_admin_path = $phpbb_admin_path; + $this->is_in_admin = (bool) $is_in_admin; + } + + /** + * Set table name + * + * @param string $log_table Can overwrite the table to use for the logs + * @return null + */ + public function set_log_table($log_table) { $this->log_table = $log_table; - $this->enable(); } /** @@ -134,8 +215,6 @@ class phpbb_log implements phpbb_log_interface return false; } - global $db, $phpbb_dispatcher; - if ($log_time == false) { $log_time = time(); @@ -208,7 +287,7 @@ class phpbb_log implements phpbb_log_interface * @since 3.1-A1 */ $vars = array('mode', 'user_id', 'log_ip', 'log_operation', 'log_time', 'additional_data', 'sql_ary'); - extract($phpbb_dispatcher->trigger_event('core.add_log', $vars)); + extract($this->dispatcher->trigger_event('core.add_log', $vars)); // We didn't find a log_type, so we don't save it in the database. if (!isset($sql_ary['log_type'])) @@ -216,9 +295,9 @@ class phpbb_log implements phpbb_log_interface return false; } - $db->sql_query('INSERT INTO ' . $this->log_table . ' ' . $db->sql_build_array('INSERT', $sql_ary)); + $this->db->sql_query('INSERT INTO ' . $this->log_table . ' ' . $this->db->sql_build_array('INSERT', $sql_ary)); - return $db->sql_nextid(); + return $this->db->sql_nextid(); } /** @@ -228,14 +307,12 @@ class phpbb_log implements phpbb_log_interface */ public function get_logs($mode, $count_logs = true, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $log_time = 0, $sort_by = 'l.log_time DESC', $keywords = '') { - global $db, $user, $auth, $phpEx, $phpbb_root_path, $phpbb_admin_path, $phpbb_dispatcher; - $this->logs_total = 0; $this->logs_offset = $offset; $topic_id_list = $reportee_id_list = array(); - $profile_url = (defined('IN_ADMIN')) ? append_sid("{$phpbb_admin_path}index.$phpEx", 'i=users&mode=overview') : append_sid("{$phpbb_root_path}memberlist.$phpEx", 'mode=viewprofile'); + $profile_url = ($this->is_in_admin && $this->phpbb_admin_path) ? append_sid("{$this->phpbb_admin_path}index.{$this->php_ext}", 'i=users&mode=overview') : append_sid("{$this->phpbb_root_path}memberlist.{$this->php_ext}", 'mode=viewprofile'); switch ($mode) { @@ -254,7 +331,7 @@ class phpbb_log implements phpbb_log_interface } else if (is_array($forum_id)) { - $sql_additional = 'AND ' . $db->sql_in_set('l.forum_id', array_map('intval', $forum_id)); + $sql_additional = 'AND ' . $this->db->sql_in_set('l.forum_id', array_map('intval', $forum_id)); } else if ($forum_id) { @@ -308,7 +385,7 @@ class phpbb_log implements phpbb_log_interface * @since 3.1-A1 */ $vars = array('mode', 'count_logs', 'limit', 'offset', 'forum_id', 'topic_id', 'user_id', 'log_time', 'sort_by', 'keywords', 'profile_url', 'log_type', 'sql_additional'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_type', $vars)); + extract($this->dispatcher->trigger_event('core.get_logs_modify_type', $vars)); if ($log_type === false) { @@ -332,9 +409,9 @@ class phpbb_log implements phpbb_log_interface AND l.log_time >= $log_time $sql_keywords $sql_additional"; - $result = $db->sql_query($sql); - $this->logs_total = (int) $db->sql_fetchfield('total_entries'); - $db->sql_freeresult($result); + $result = $this->db->sql_query($sql); + $this->logs_total = (int) $this->db->sql_fetchfield('total_entries'); + $this->db->sql_freeresult($result); if ($this->logs_total == 0) { @@ -358,11 +435,11 @@ class phpbb_log implements phpbb_log_interface $sql_keywords $sql_additional ORDER BY $sort_by"; - $result = $db->sql_query_limit($sql, $limit, $this->logs_offset); + $result = $this->db->sql_query_limit($sql, $limit, $this->logs_offset); $i = 0; $log = array(); - while ($row = $db->sql_fetchrow($result)) + while ($row = $this->db->sql_fetchrow($result)) { $row['forum_id'] = (int) $row['forum_id']; if ($row['topic_id']) @@ -391,8 +468,8 @@ class phpbb_log implements phpbb_log_interface 'forum_id' => (int) $row['forum_id'], 'topic_id' => (int) $row['topic_id'], - 'viewforum' => ($row['forum_id'] && $auth->acl_get('f_read', $row['forum_id'])) ? append_sid("{$phpbb_root_path}viewforum.$phpEx", 'f=' . $row['forum_id']) : false, - 'action' => (isset($user->lang[$row['log_operation']])) ? $user->lang[$row['log_operation']] : '{' . ucfirst(str_replace('_', ' ', $row['log_operation'])) . '}', + 'viewforum' => ($row['forum_id'] && $this->auth->acl_get('f_read', $row['forum_id'])) ? append_sid("{$this->phpbb_root_path}viewforum.{$this->php_ext}", 'f=' . $row['forum_id']) : false, + 'action' => (isset($this->user->lang[$row['log_operation']])) ? $this->user->lang[$row['log_operation']] : '{' . ucfirst(str_replace('_', ' ', $row['log_operation'])) . '}', ); /** @@ -404,7 +481,7 @@ class phpbb_log implements phpbb_log_interface * @since 3.1-A1 */ $vars = array('row', 'log_entry_data'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_modify_entry_data', $vars)); + extract($this->dispatcher->trigger_event('core.get_logs_modify_entry_data', $vars)); $log[$i] = $log_entry_data; @@ -413,7 +490,7 @@ class phpbb_log implements phpbb_log_interface $log_data_ary = @unserialize($row['log_data']); $log_data_ary = ($log_data_ary !== false) ? $log_data_ary : array(); - if (isset($user->lang[$row['log_operation']])) + if (isset($this->user->lang[$row['log_operation']])) { // Check if there are more occurrences of % than arguments, if there are we fill out the arguments array // It doesn't matter if we add more arguments than placeholders @@ -447,7 +524,7 @@ class phpbb_log implements phpbb_log_interface $i++; } - $db->sql_freeresult($result); + $this->db->sql_freeresult($result); /** * Get some additional data after we got all log entries @@ -461,7 +538,7 @@ class phpbb_log implements phpbb_log_interface * @since 3.1-A1 */ $vars = array('log', 'topic_id_list', 'reportee_id_list'); - extract($phpbb_dispatcher->trigger_event('core.get_logs_get_additional_data', $vars)); + extract($this->dispatcher->trigger_event('core.get_logs_get_additional_data', $vars)); if (sizeof($topic_id_list)) { @@ -469,8 +546,8 @@ class phpbb_log implements phpbb_log_interface foreach ($log as $key => $row) { - $log[$key]['viewtopic'] = (isset($topic_auth['f_read'][$row['topic_id']])) ? append_sid("{$phpbb_root_path}viewtopic.$phpEx", 'f=' . $topic_auth['f_read'][$row['topic_id']] . '&t=' . $row['topic_id']) : false; - $log[$key]['viewlogs'] = (isset($topic_auth['m_'][$row['topic_id']])) ? append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=logs&mode=topic_logs&t=' . $row['topic_id'], true, $user->session_id) : false; + $log[$key]['viewtopic'] = (isset($topic_auth['f_read'][$row['topic_id']])) ? append_sid("{$this->phpbb_root_path}viewtopic.{$this->php_ext}", 'f=' . $topic_auth['f_read'][$row['topic_id']] . '&t=' . $row['topic_id']) : false; + $log[$key]['viewlogs'] = (isset($topic_auth['m_'][$row['topic_id']])) ? append_sid("{$this->phpbb_root_path}mcp.{$this->php_ext}", 'i=logs&mode=topic_logs&t=' . $row['topic_id'], true, $this->user->session_id) : false; } } @@ -502,8 +579,6 @@ class phpbb_log implements phpbb_log_interface */ private function generate_sql_keyword($keywords) { - global $db, $user; - // Use no preg_quote for $keywords because this would lead to sole backslashes being added // We also use an OR connection here for spaces and the | string. Currently, regex is not supported for searching (but may come later). $keywords = preg_split('#[\s|]+#u', utf8_strtolower($keywords), 0, PREG_SPLIT_NO_EMPTY); @@ -517,13 +592,13 @@ class phpbb_log implements phpbb_log_interface for ($i = 0, $num_keywords = sizeof($keywords); $i < $num_keywords; $i++) { $keywords_pattern[] = preg_quote($keywords[$i], '#'); - $keywords[$i] = $db->sql_like_expression($db->any_char . $keywords[$i] . $db->any_char); + $keywords[$i] = $this->db->sql_like_expression($this->db->any_char . $keywords[$i] . $this->db->any_char); } $keywords_pattern = '#' . implode('|', $keywords_pattern) . '#ui'; $operations = array(); - foreach ($user->lang as $key => $value) + foreach ($this->user->lang as $key => $value) { if (substr($key, 0, 4) == 'LOG_' && preg_match($keywords_pattern, $value)) { @@ -534,9 +609,9 @@ class phpbb_log implements phpbb_log_interface $sql_keywords = 'AND ('; if (!empty($operations)) { - $sql_keywords .= $db->sql_in_set('l.log_operation', $operations) . ' OR '; + $sql_keywords .= $this->db->sql_in_set('l.log_operation', $operations) . ' OR '; } - $sql_lower = $db->sql_lower_text('l.log_data'); + $sql_lower = $this->db->sql_lower_text('l.log_data'); $sql_keywords .= " $sql_lower " . implode(" OR $sql_lower ", $keywords) . ')'; } @@ -557,32 +632,30 @@ class phpbb_log implements phpbb_log_interface */ private function get_topic_auth($topic_ids) { - global $auth, $db; - $forum_auth = array('f_read' => array(), 'm_' => array()); $topic_ids = array_unique($topic_ids); $sql = 'SELECT topic_id, forum_id FROM ' . TOPICS_TABLE . ' - WHERE ' . $db->sql_in_set('topic_id', array_map('intval', $topic_ids)); - $result = $db->sql_query($sql); + WHERE ' . $this->db->sql_in_set('topic_id', array_map('intval', $topic_ids)); + $result = $this->db->sql_query($sql); - while ($row = $db->sql_fetchrow($result)) + while ($row = $this->db->sql_fetchrow($result)) { $row['topic_id'] = (int) $row['topic_id']; $row['forum_id'] = (int) $row['forum_id']; - if ($auth->acl_get('f_read', $row['forum_id'])) + if ($this->auth->acl_get('f_read', $row['forum_id'])) { $forum_auth['f_read'][$row['topic_id']] = $row['forum_id']; } - if ($auth->acl_gets('a_', 'm_', $row['forum_id'])) + if ($this->auth->acl_gets('a_', 'm_', $row['forum_id'])) { $forum_auth['m_'][$row['topic_id']] = $row['forum_id']; } } - $db->sql_freeresult($result); + $this->db->sql_freeresult($result); return $forum_auth; } @@ -596,21 +669,19 @@ class phpbb_log implements phpbb_log_interface */ private function get_reportee_data($reportee_ids) { - global $db; - $reportee_ids = array_unique($reportee_ids); $reportee_data_list = array(); $sql = 'SELECT user_id, username, user_colour FROM ' . USERS_TABLE . ' - WHERE ' . $db->sql_in_set('user_id', $reportee_ids); - $result = $db->sql_query($sql); + WHERE ' . $this->db->sql_in_set('user_id', $reportee_ids); + $result = $this->db->sql_query($sql); - while ($row = $db->sql_fetchrow($result)) + while ($row = $this->db->sql_fetchrow($result)) { $reportee_data_list[$row['user_id']] = $row; } - $db->sql_freeresult($result); + $this->db->sql_freeresult($result); return $reportee_data_list; } From f4bc9c1673c18ed24a2ba1680f6b9a5de5c491bf Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 11 Dec 2012 10:24:49 +0100 Subject: [PATCH 40/56] [ticket/10714] Fix dependency injections in unit tests with mocks PHPBB3-10714 --- tests/log/add_test.php | 19 ++++++++++++++++--- tests/log/function_add_log_test.php | 7 +++++-- tests/log/function_view_log_test.php | 5 +++-- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/log/add_test.php b/tests/log/add_test.php index fceb48ed01..4354cc4cc7 100644 --- a/tests/log/add_test.php +++ b/tests/log/add_test.php @@ -18,7 +18,16 @@ class phpbb_log_add_test extends phpbb_database_test_case public function test_log_enabled() { - $log = new phpbb_log(LOG_TABLE); + global $phpbb_root_path, $phpEx, $db, $phpbb_dispatcher; + + $db = $this->new_dbal(); + $cache = new phpbb_mock_cache; + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $user = $this->getMock('phpbb_user'); + $auth = $this->getMock('phpbb_auth'); + + $log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); + $this->assertTrue($log->is_enabled(), 'Initialise failed'); $log->disable(); @@ -38,10 +47,15 @@ class phpbb_log_add_test extends phpbb_database_test_case public function test_log_add() { - global $db, $phpbb_dispatcher; + global $phpbb_root_path, $phpEx, $db, $phpbb_dispatcher; $db = $this->new_dbal(); + $cache = new phpbb_mock_cache; $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $user = $this->getMock('phpbb_user'); + $auth = $this->getMock('phpbb_auth'); + + $log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); $mode = 'critical'; $user_id = ANONYMOUS; @@ -51,7 +65,6 @@ class phpbb_log_add_test extends phpbb_database_test_case $additional_data = array(); // Add an entry successful - $log = new phpbb_log(LOG_TABLE); $this->assertEquals(1, $log->add($mode, $user_id, $log_ip, $log_operation, $log_time)); // Disable logging for all types diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index 7ed862c523..0a65ce3165 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -142,7 +142,7 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case */ public function test_add_log_function($expected, $user_id, $mode, $required1, $additional1 = null, $additional2 = null, $additional3 = null) { - global $db, $cache, $user, $phpbb_log, $phpbb_dispatcher; + global $db, $cache, $user, $phpbb_log, $phpbb_dispatcher, $phpbb_root_path, $phpEx; if ($expected) { @@ -157,7 +157,10 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case $db = $this->new_dbal(); $cache = new phpbb_mock_cache; $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $phpbb_log = new phpbb_log(LOG_TABLE); + $user = $this->getMock('phpbb_user'); + $auth = $this->getMock('phpbb_auth'); + + $phpbb_log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); $user->ip = 'user_ip'; if ($user_id) diff --git a/tests/log/function_view_log_test.php b/tests/log/function_view_log_test.php index f7e2c51c32..bb33668ae4 100644 --- a/tests/log/function_view_log_test.php +++ b/tests/log/function_view_log_test.php @@ -300,12 +300,11 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case */ public function test_view_log_function($expected, $expected_returned, $mode, $log_count, $limit = 5, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $limit_days = 0, $sort_by = 'l.log_id ASC', $keywords = '') { - global $cache, $db, $user, $auth, $phpbb_log, $phpbb_dispatcher; + global $cache, $db, $user, $auth, $phpbb_log, $phpbb_dispatcher, $phpbb_root_path, $phpEx; $db = $this->new_dbal(); $cache = new phpbb_mock_cache; $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $phpbb_log = new phpbb_log(LOGS_TABLE); // Create auth mock $auth = $this->getMock('phpbb_auth'); @@ -335,6 +334,8 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case 'LOG_INSTALL_INSTALLED' => 'installed: %s', ); + $phpbb_log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); + $log = array(); $this->assertEquals($expected_returned, view_log($mode, $log, $log_count, $limit, $offset, $forum_id, $topic_id, $user_id, $limit_days, $sort_by, $keywords)); From c7ae790d16f662ef1cdb2e697f3276b9e618f4dd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 11 Dec 2012 10:25:38 +0100 Subject: [PATCH 41/56] [ticket/10714] Remove type hinting to allow the usage of mocks in tests PHPBB3-10714 --- phpBB/includes/log/log.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 3d9620a2ee..beb2a659e1 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -96,7 +96,7 @@ class phpbb_log implements phpbb_log_interface * @param string $log_table Name of the table we use to store our logs * @return null */ - public function __construct(dbal $db, phpbb_user $user, phpbb_auth $auth, phpbb_dispatcher $phpbb_dispatcher, $phpbb_root_path, $php_ext, $log_table) + public function __construct($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $php_ext, $log_table) { $this->db = $db; $this->user = $user; From bd334d318fb61992cedfdb7ca1306ad670f392a4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 11 Dec 2012 13:59:21 +0100 Subject: [PATCH 42/56] [ticket/10714] Forgot most important, use container to create $phpbb_log PHPBB3-10714 --- phpBB/common.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/common.php b/phpBB/common.php index 344b83403f..88ab9b6b5a 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -118,7 +118,7 @@ $config = $phpbb_container->get('config'); set_config(null, null, null, $config); set_config_count(null, null, null, $config); -$phpbb_log = new phpbb_log(LOG_TABLE); +$phpbb_log = $phpbb_container->get('log'); // load extensions $phpbb_extension_manager = $phpbb_container->get('ext.manager'); From 512697341a9c570ee11697eb221ef18d2fadfe45 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 14 Dec 2012 18:47:22 +0100 Subject: [PATCH 43/56] [ticket/10714] Fix database driver class name PHPBB3-10714 --- phpBB/includes/log/log.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index beb2a659e1..521a998d8e 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -44,7 +44,7 @@ class phpbb_log implements phpbb_log_interface /** * Database object - * @var dbal + * @var phpbb_db_driver */ protected $db; @@ -87,9 +87,9 @@ class phpbb_log implements phpbb_log_interface /** * Constructor * - * @param dbal $db Database object - * @param phpbb_user $user User object - * @param phpbb_auth $auth Auth object + * @param phpbb_db_driver $db Database object + * @param phpbb_user $user User object + * @param phpbb_auth $auth Auth object * @param phpbb_dispatcher $phpbb_dispatcher Event dispatcher * @param string $phpbb_root_path Root path * @param string $php_ext PHP Extension From 37014abd022be4f7824a590b93a329f74aef442c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 16 Jan 2013 14:18:09 +0100 Subject: [PATCH 44/56] [ticket/10714] Fix several comments and variable names PHPBB3-10714 --- phpBB/includes/log/interface.php | 20 ++++--- phpBB/includes/log/log.php | 85 ++++++++++++++-------------- tests/log/function_view_log_test.php | 2 +- 3 files changed, 53 insertions(+), 54 deletions(-) diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index b85dc3a474..254b65cb19 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -25,40 +25,42 @@ interface phpbb_log_interface /** * This function returns the state of the log system. * - * @param string $type The log type we want to check. Empty to get global log status. + * @param string $type The log type we want to check. Empty to get + * global log status. * * @return bool True if log for the type is enabled */ public function is_enabled($type = ''); /** - * This function allows disable the log system. When add_log is called, the log will not be added to the database. + * This function allows disabling the log system. When add_log is called + * and the type is disabled, the log will not be added to the database. * - * @param mixed $type The log type we want to disable. Empty to disable all logs. - * Can also be an array of types + * @param mixed $type The log type we want to disable. Empty to + * disable all logs. Can also be an array of types. * * @return null */ public function disable($type = ''); /** - * This function allows re-enable the log system. + * This function allows re-enabling the log system. * - * @param mixed $type The log type we want to enable. Empty to enable all logs. - * Can also be an array of types + * @param mixed $type The log type we want to enable. Empty to + * enable all logs. Can also be an array of types. * * @return null */ public function enable($type = ''); /** - * Adds a log to the database + * Adds a log entry to the database * * @param string $mode The mode defines which log_type is used and in which log the entry is displayed. * @param int $user_id User ID of the user * @param string $log_ip IP address of the user * @param string $log_operation Name of the operation - * @param int $log_time Timestamp when the log was added. + * @param int $log_time Timestamp when the log entry was added. * @param array $additional_data More arguments can be added, depending on the log_type * * @return int|bool Returns the log_id, if the entry was added to the database, false otherwise. diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 521a998d8e..0ee3c9561f 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -23,19 +23,21 @@ if (!defined('IN_PHPBB')) class phpbb_log implements phpbb_log_interface { /** - * Keeps the status of the log system. Is the log enabled or disabled? + * An array with the disabled log types. Logs of such types will not be + * added when add_log() is called. + * @var array */ - protected $disabled_logs; + protected $disabled_types; /** * Keeps the total log count of the last call to get_logs() */ - protected $logs_total; + protected $entry_count; /** * Keeps the offset of the last valid page of the last call to get_logs() */ - protected $logs_offset; + protected $last_page_offset; /** * The table we use to store our logs. @@ -111,7 +113,8 @@ class phpbb_log implements phpbb_log_interface } /** - * Set phpbb_admin_path and is_in_admin in order to return administrative user profile links in get_logs() + * Set phpbb_admin_path and is_in_admin in order to return administrative + * user profile links in get_logs() * * @param string $phpbb_admin_path Full path from current file to admin root * @param bool $is_in_admin Are we called from within the acp? @@ -137,26 +140,22 @@ class phpbb_log implements phpbb_log_interface /** * This function returns the state of the log system. * - * @param string $type The log type we want to check. Empty to get global log status. - * - * @return bool True if log for the type is enabled + * {@inheritDoc} */ public function is_enabled($type = '') { if ($type == '' || $type == 'all') { - return !isset($this->disabled_logs['all']); + return !isset($this->disabled_types['all']); } - return !isset($this->disabled_logs[$type]) && !isset($this->disabled_logs['all']); + return !isset($this->disabled_types[$type]) && !isset($this->disabled_types['all']); } /** - * This function allows disable the log system. When add_log is called, the log will not be added to the database. + * This function allows disabling the log system. When add_log is called + * and the type is disabled, the log will not be added to the database. * - * @param mixed $type The log type we want to enable. Empty to disable all logs. - * Can also be an array of types - * - * @return null + * {@inheritDoc} */ public function disable($type = '') { @@ -169,20 +168,18 @@ class phpbb_log implements phpbb_log_interface return; } - if ($type == '' || $type == 'all') + // Empty string is an equivalent for all types. + if ($type == '') { - $this->disabled_logs['all'] = true; - return; + $type = 'all'; } - $this->disabled_logs[$type] = true; + $this->disabled_types[$type] = true; } /** - * This function allows re-enable the log system. + * This function allows re-enabling the log system. * - * @param mixed $type The log type we want to enable. Empty to enable all logs. - * - * @return null + * {@inheritDoc} */ public function enable($type = '') { @@ -197,10 +194,10 @@ class phpbb_log implements phpbb_log_interface if ($type == '' || $type == 'all') { - $this->disabled_logs = array(); + $this->disabled_types = array(); return; } - unset($this->disabled_logs[$type]); + unset($this->disabled_types[$type]); } /** @@ -269,7 +266,7 @@ class phpbb_log implements phpbb_log_interface } /** - * Allow to modify log data before we add them to the database + * Allows to modify log data before we add it to the database * * NOTE: if sql_ary does not contain a log_type value, the entry will * not be stored in the database. So ensure to set it, if needed. @@ -307,8 +304,8 @@ class phpbb_log implements phpbb_log_interface */ public function get_logs($mode, $count_logs = true, $limit = 0, $offset = 0, $forum_id = 0, $topic_id = 0, $user_id = 0, $log_time = 0, $sort_by = 'l.log_time DESC', $keywords = '') { - $this->logs_total = 0; - $this->logs_offset = $offset; + $this->entry_count = 0; + $this->last_page_offset = $offset; $topic_id_list = $reportee_id_list = array(); @@ -389,7 +386,7 @@ class phpbb_log implements phpbb_log_interface if ($log_type === false) { - $this->logs_offset = 0; + $this->last_page_offset = 0; return array(); } @@ -410,20 +407,20 @@ class phpbb_log implements phpbb_log_interface $sql_keywords $sql_additional"; $result = $this->db->sql_query($sql); - $this->logs_total = (int) $this->db->sql_fetchfield('total_entries'); + $this->entry_count = (int) $this->db->sql_fetchfield('total_entries'); $this->db->sql_freeresult($result); - if ($this->logs_total == 0) + if ($this->entry_count == 0) { // Save the queries, because there are no logs to display - $this->logs_offset = 0; + $this->last_page_offset = 0; return array(); } // Return the user to the last page that is valid - while ($this->logs_offset >= $this->logs_total) + while ($this->last_page_offset >= $this->entry_count) { - $this->logs_offset = max(0, $this->logs_offset - $limit); + $this->last_page_offset = max(0, $this->last_page_offset - $limit); } } @@ -435,7 +432,7 @@ class phpbb_log implements phpbb_log_interface $sql_keywords $sql_additional ORDER BY $sort_by"; - $result = $this->db->sql_query_limit($sql, $limit, $this->logs_offset); + $result = $this->db->sql_query_limit($sql, $limit, $this->last_page_offset); $i = 0; $log = array(); @@ -487,7 +484,7 @@ class phpbb_log implements phpbb_log_interface if (!empty($row['log_data'])) { - $log_data_ary = @unserialize($row['log_data']); + $log_data_ary = unserialize($row['log_data']); $log_data_ary = ($log_data_ary !== false) ? $log_data_ary : array(); if (isset($this->user->lang[$row['log_operation']])) @@ -571,13 +568,13 @@ class phpbb_log implements phpbb_log_interface } /** - * Generates a sql condition out of the specified keywords + * Generates a sql condition for the specified keywords * * @param string $keywords The keywords the user specified to search for * * @return string Returns the SQL condition searching for the keywords */ - private function generate_sql_keyword($keywords) + protected function generate_sql_keyword($keywords) { // Use no preg_quote for $keywords because this would lead to sole backslashes being added // We also use an OR connection here for spaces and the | string. Currently, regex is not supported for searching (but may come later). @@ -630,7 +627,7 @@ class phpbb_log implements phpbb_log_interface * ), * ), */ - private function get_topic_auth($topic_ids) + protected function get_topic_auth(array $topic_ids) { $forum_auth = array('f_read' => array(), 'm_' => array()); $topic_ids = array_unique($topic_ids); @@ -667,7 +664,7 @@ class phpbb_log implements phpbb_log_interface * * @return array Returns an array with the reportee data */ - private function get_reportee_data($reportee_ids) + protected function get_reportee_data(array $reportee_ids) { $reportee_ids = array_unique($reportee_ids); $reportee_data_list = array(); @@ -689,20 +686,20 @@ class phpbb_log implements phpbb_log_interface /** * Get total log count * - * @return int Returns the number of matching logs from the last call to get_logs() + * {@inheritDoc} */ public function get_log_count() { - return ($this->logs_total) ? $this->logs_total : 0; + return ($this->entry_count) ? $this->entry_count : 0; } /** * Get offset of the last valid log page * - * @return int Returns the offset of the last valid page from the last call to get_logs() + * {@inheritDoc} */ public function get_valid_offset() { - return ($this->logs_offset) ? $this->logs_offset : 0; + return ($this->last_page_offset) ? $this->last_page_offset : 0; } } diff --git a/tests/log/function_view_log_test.php b/tests/log/function_view_log_test.php index bb33668ae4..7401e1ee4f 100644 --- a/tests/log/function_view_log_test.php +++ b/tests/log/function_view_log_test.php @@ -215,7 +215,7 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case array( // Array of datasets that should be in $log after running the function 'expected' => array(5, 7), - // Offset that will be returned form the function + // Offset that will be returned from the function 'expected_returned' => 0, // view_log parameters (see includes/functions_admin.php for docblock) // $log is ommited! From 786e2438d5138213447003272b36b4185cad2b42 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 16 Jan 2013 16:23:41 +0100 Subject: [PATCH 45/56] [ticket/10714] Use new core.adm_relative_path to create the object. PHPBB3-10714 --- phpBB/config/services.yml | 1 + phpBB/includes/log/log.php | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 7fd13d4eea..129cd4a4f9 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -164,6 +164,7 @@ services: - @auth - @dispatcher - %core.root_path% + - %core.adm_relative_path% - %core.php_ext% - %tables.log% diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 0ee3c9561f..092fdb6a89 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -94,35 +94,40 @@ class phpbb_log implements phpbb_log_interface * @param phpbb_auth $auth Auth object * @param phpbb_dispatcher $phpbb_dispatcher Event dispatcher * @param string $phpbb_root_path Root path + * @param string $relative_admin_path Relative admin root path * @param string $php_ext PHP Extension * @param string $log_table Name of the table we use to store our logs * @return null */ - public function __construct($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $php_ext, $log_table) + public function __construct($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $relative_admin_path, $php_ext, $log_table) { $this->db = $db; $this->user = $user; $this->auth = $auth; $this->dispatcher = $phpbb_dispatcher; $this->phpbb_root_path = $phpbb_root_path; + $this->phpbb_admin_path = $this->phpbb_root_path . $relative_admin_path; $this->php_ext = $php_ext; $this->log_table = $log_table; + /* + * IN_ADMIN is set after the session was created, + * so we need to take ADMIN_START into account aswell, otherwise + * it will not work for the phpbb_log object we create in common.php + */ + $this->set_is_admin((defined('ADMIN_START') && ADMIN_START) || (defined('IN_ADMIN') && IN_ADMIN)); $this->enable(); - $this->set_admin_path('', false); } /** - * Set phpbb_admin_path and is_in_admin in order to return administrative - * user profile links in get_logs() + * Set is_in_admin in order to return administrative user profile links + * in get_logs() * - * @param string $phpbb_admin_path Full path from current file to admin root * @param bool $is_in_admin Are we called from within the acp? * @return null */ - public function set_admin_path($phpbb_admin_path, $is_in_admin) + public function set_is_admin($is_in_admin) { - $this->phpbb_admin_path = $phpbb_admin_path; $this->is_in_admin = (bool) $is_in_admin; } From 2c4e7eabe248fc785cd2b43f57a42580361c599d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 22 Jan 2013 15:10:35 +0100 Subject: [PATCH 46/56] [ticket/10714] Fix missing 8th argument in unit tests PHPBB3-10714 --- tests/log/add_test.php | 4 ++-- tests/log/function_add_log_test.php | 2 +- tests/log/function_view_log_test.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/log/add_test.php b/tests/log/add_test.php index 4354cc4cc7..a5f93232f2 100644 --- a/tests/log/add_test.php +++ b/tests/log/add_test.php @@ -26,7 +26,7 @@ class phpbb_log_add_test extends phpbb_database_test_case $user = $this->getMock('phpbb_user'); $auth = $this->getMock('phpbb_auth'); - $log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); + $log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); $this->assertTrue($log->is_enabled(), 'Initialise failed'); @@ -55,7 +55,7 @@ class phpbb_log_add_test extends phpbb_database_test_case $user = $this->getMock('phpbb_user'); $auth = $this->getMock('phpbb_auth'); - $log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); + $log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); $mode = 'critical'; $user_id = ANONYMOUS; diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index 0a65ce3165..4e54a75dd6 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -160,7 +160,7 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case $user = $this->getMock('phpbb_user'); $auth = $this->getMock('phpbb_auth'); - $phpbb_log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); + $phpbb_log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); $user->ip = 'user_ip'; if ($user_id) diff --git a/tests/log/function_view_log_test.php b/tests/log/function_view_log_test.php index 7401e1ee4f..2ecf77aeb8 100644 --- a/tests/log/function_view_log_test.php +++ b/tests/log/function_view_log_test.php @@ -334,7 +334,7 @@ class phpbb_log_function_view_log_test extends phpbb_database_test_case 'LOG_INSTALL_INSTALLED' => 'installed: %s', ); - $phpbb_log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, $phpEx, LOG_TABLE); + $phpbb_log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); $log = array(); $this->assertEquals($expected_returned, view_log($mode, $log, $log_count, $limit, $offset, $forum_id, $topic_id, $user_id, $limit_days, $sort_by, $keywords)); From c0ab3f3ddddefa8f902ffa57c864e6db5bf1f440 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 22 Jan 2013 15:45:20 +0100 Subject: [PATCH 47/56] [ticket/10714] Fix several doc blocks and comments PHPBB3-10714 --- phpBB/includes/functions_admin.php | 4 ++-- phpBB/includes/log/interface.php | 10 +++++----- phpBB/includes/log/log.php | 5 ++++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/phpBB/includes/functions_admin.php b/phpBB/includes/functions_admin.php index 18b11182d0..60591e98d3 100644 --- a/phpBB/includes/functions_admin.php +++ b/phpBB/includes/functions_admin.php @@ -2489,12 +2489,12 @@ function cache_moderators() /** * View log * -* @param string $mode The mode defines which log_type is used and in which log the entry is displayed. +* @param string $mode The mode defines which log_type is used and from which log the entry is retrieved * @param array &$log The result array with the logs * @param mixed &$log_count If $log_count is set to false, we will skip counting all entries in the database. * Otherwise an integer with the number of total matching entries is returned. * @param int $limit Limit the number of entries that are returned -* @param int $offset Offset when fetching the log entries, f.e. on paginations +* @param int $offset Offset when fetching the log entries, f.e. when paginating * @param mixed $forum_id Restrict the log entries to the given forum_id (can also be an array of forum_ids) * @param int $topic_id Restrict the log entries to the given topic_id * @param int $user_id Restrict the log entries to the given user_id diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index 254b65cb19..24bf412ce0 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -56,24 +56,24 @@ interface phpbb_log_interface /** * Adds a log entry to the database * - * @param string $mode The mode defines which log_type is used and in which log the entry is displayed. + * @param string $mode The mode defines which log_type is used and from which log the entry is retrieved * @param int $user_id User ID of the user * @param string $log_ip IP address of the user * @param string $log_operation Name of the operation - * @param int $log_time Timestamp when the log entry was added. + * @param int $log_time Timestamp when the log entry was added, if empty time() will be used * @param array $additional_data More arguments can be added, depending on the log_type * * @return int|bool Returns the log_id, if the entry was added to the database, false otherwise. */ - public function add($mode, $user_id, $log_ip, $log_operation, $log_time, $additional_data); + public function add($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = array()); /** * Grab the logs from the database * - * @param string $mode The mode defines which log_type is used and in which log the entry is displayed. + * @param string $mode The mode defines which log_type is used and ifrom which log the entry is retrieved * @param bool $count_logs Shall we count all matching log entries? * @param int $limit Limit the number of entries that are returned - * @param int $offset Offset when fetching the log entries, f.e. on paginations + * @param int $offset Offset when fetching the log entries, f.e. when paginating * @param mixed $forum_id Restrict the log entries to the given forum_id (can also be an array of forum_ids) * @param int $topic_id Restrict the log entries to the given topic_id * @param int $user_id Restrict the log entries to the given user_id diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 092fdb6a89..33c558695c 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -31,16 +31,19 @@ class phpbb_log implements phpbb_log_interface /** * Keeps the total log count of the last call to get_logs() + * @var int */ protected $entry_count; /** * Keeps the offset of the last valid page of the last call to get_logs() + * @var int */ protected $last_page_offset; /** * The table we use to store our logs. + * @var string */ protected $log_table; @@ -112,7 +115,7 @@ class phpbb_log implements phpbb_log_interface /* * IN_ADMIN is set after the session was created, - * so we need to take ADMIN_START into account aswell, otherwise + * so we need to take ADMIN_START into account as well, otherwise * it will not work for the phpbb_log object we create in common.php */ $this->set_is_admin((defined('ADMIN_START') && ADMIN_START) || (defined('IN_ADMIN') && IN_ADMIN)); From ffde887aadfcb9d3db2c42cf09e22745e5d62430 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 22 Jan 2013 15:46:48 +0100 Subject: [PATCH 48/56] [ticket/10714] Cast values to integer before using them in the query PHPBB3-10714 --- phpBB/includes/log/log.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 33c558695c..841612f7bd 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -408,10 +408,10 @@ class phpbb_log implements phpbb_log_interface if ($count_logs) { $sql = 'SELECT COUNT(l.log_id) AS total_entries - FROM ' . LOG_TABLE . ' l, ' . USERS_TABLE . " u - WHERE l.log_type = $log_type + FROM ' . LOG_TABLE . ' l, ' . USERS_TABLE . ' u + WHERE l.log_type = ' . (int) $log_type . ' AND l.user_id = u.user_id - AND l.log_time >= $log_time + AND l.log_time >= ' . (int) $log_time . " $sql_keywords $sql_additional"; $result = $this->db->sql_query($sql); @@ -433,10 +433,10 @@ class phpbb_log implements phpbb_log_interface } $sql = 'SELECT l.*, u.username, u.username_clean, u.user_colour - FROM ' . LOG_TABLE . ' l, ' . USERS_TABLE . " u - WHERE l.log_type = $log_type + FROM ' . LOG_TABLE . ' l, ' . USERS_TABLE . ' u + WHERE l.log_type = ' . (int) $log_type . ' AND u.user_id = l.user_id - " . (($log_time) ? "AND l.log_time >= $log_time" : '') . " + ' . (($log_time) ? 'AND l.log_time >= ' . (int) $log_time : '') . " $sql_keywords $sql_additional ORDER BY $sort_by"; From c2504e9300608feea540ab162e4cc0ac79cda7a0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 22 Jan 2013 15:56:34 +0100 Subject: [PATCH 49/56] [ticket/10714] Fix more comments PHPBB3-10714 --- phpBB/includes/log/log.php | 14 +++++++++----- tests/log/function_add_log_test.php | 4 ++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 841612f7bd..09dff10ae5 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -497,8 +497,10 @@ class phpbb_log implements phpbb_log_interface if (isset($this->user->lang[$row['log_operation']])) { - // Check if there are more occurrences of % than arguments, if there are we fill out the arguments array - // It doesn't matter if we add more arguments than placeholders + // Check if there are more occurrences of % than + // arguments, if there are we fill out the arguments + // array. It doesn't matter if we add more arguments than + // placeholders. if ((substr_count($log[$i]['action'], '%') - sizeof($log_data_ary)) > 0) { $log_data_ary = array_merge($log_data_ary, array_fill(0, substr_count($log[$i]['action'], '%') - sizeof($log_data_ary), '')); @@ -507,7 +509,7 @@ class phpbb_log implements phpbb_log_interface $log[$i]['action'] = vsprintf($log[$i]['action'], $log_data_ary); // If within the admin panel we do not censor text out - if (defined('IN_ADMIN')) + if ($this->is_in_admin) { $log[$i]['action'] = bbcode_nl2br($log[$i]['action']); } @@ -584,8 +586,10 @@ class phpbb_log implements phpbb_log_interface */ protected function generate_sql_keyword($keywords) { - // Use no preg_quote for $keywords because this would lead to sole backslashes being added - // We also use an OR connection here for spaces and the | string. Currently, regex is not supported for searching (but may come later). + // Use no preg_quote for $keywords because this would lead to sole + // backslashes being added. We also use an OR connection here for + // spaces and the | string. Currently, regex is not supported for + // searching (but may come later). $keywords = preg_split('#[\s|]+#u', utf8_strtolower($keywords), 0, PREG_SPLIT_NO_EMPTY); $sql_keywords = ''; diff --git a/tests/log/function_add_log_test.php b/tests/log/function_add_log_test.php index 4e54a75dd6..864b364862 100644 --- a/tests/log/function_add_log_test.php +++ b/tests/log/function_add_log_test.php @@ -35,9 +35,9 @@ class phpbb_log_function_add_log_test extends phpbb_database_test_case 'forum_id' => 56, 'topic_id' => 78, ), - // user_id Can also be false, than ANONYMOUS is used + // user_id Can also be false, then ANONYMOUS is used false, - // log_mode Used to determinate the log_type + // log_mode Used to determine the log_type 'mod', // Followed by some additional arguments // forum_id, topic_id and reportee_id are specified before log_operation From d5d282005c74a4b9539c3b37d3df723ba6e2c456 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 22 Jan 2013 16:47:05 +0100 Subject: [PATCH 50/56] [ticket/10714] Add getter for is_in_admin and use it PHPBB3-10714 --- phpBB/includes/log/log.php | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 09dff10ae5..8da8b63391 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -22,6 +22,13 @@ if (!defined('IN_PHPBB')) */ class phpbb_log implements phpbb_log_interface { + /** + * If set, administrative user profile links will be returned and messages + * will not be censored. + * @var bool + */ + protected $is_in_admin; + /** * An array with the disabled log types. Logs of such types will not be * added when add_log() is called. @@ -114,7 +121,7 @@ class phpbb_log implements phpbb_log_interface $this->log_table = $log_table; /* - * IN_ADMIN is set after the session was created, + * IN_ADMIN is set after the session is created, * so we need to take ADMIN_START into account as well, otherwise * it will not work for the phpbb_log object we create in common.php */ @@ -134,6 +141,16 @@ class phpbb_log implements phpbb_log_interface $this->is_in_admin = (bool) $is_in_admin; } + /** + * Returns the is_in_admin option + * + * @return bool + */ + public function get_is_admin() + { + return $this->is_in_admin; + } + /** * Set table name * @@ -317,7 +334,7 @@ class phpbb_log implements phpbb_log_interface $topic_id_list = $reportee_id_list = array(); - $profile_url = ($this->is_in_admin && $this->phpbb_admin_path) ? append_sid("{$this->phpbb_admin_path}index.{$this->php_ext}", 'i=users&mode=overview') : append_sid("{$this->phpbb_root_path}memberlist.{$this->php_ext}", 'mode=viewprofile'); + $profile_url = ($this->get_is_admin() && $this->phpbb_admin_path) ? append_sid("{$this->phpbb_admin_path}index.{$this->php_ext}", 'i=users&mode=overview') : append_sid("{$this->phpbb_root_path}memberlist.{$this->php_ext}", 'mode=viewprofile'); switch ($mode) { @@ -509,7 +526,7 @@ class phpbb_log implements phpbb_log_interface $log[$i]['action'] = vsprintf($log[$i]['action'], $log_data_ary); // If within the admin panel we do not censor text out - if ($this->is_in_admin) + if ($this->get_is_admin()) { $log[$i]['action'] = bbcode_nl2br($log[$i]['action']); } From e8fd8b9a4b37f7d7d3955cd2b0d79139a2c0222d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 22 Jan 2013 22:40:53 +0100 Subject: [PATCH 51/56] [ticket/10714] Fix missing parameter and global phpbb_log in unit tests PHPBB3-10714 --- phpBB/includes/functions_container.php | 1 + tests/functions_user/group_user_attributes_test.php | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/functions_container.php b/phpBB/includes/functions_container.php index a3ed21c35b..36c5ad507e 100644 --- a/phpBB/includes/functions_container.php +++ b/phpBB/includes/functions_container.php @@ -57,6 +57,7 @@ function phpbb_create_install_container($phpbb_root_path, $php_ext) $container = phpbb_create_container(array($core), $phpbb_root_path, $php_ext); $container->setParameter('core.root_path', $phpbb_root_path); + $container->setParameter('core.adm_relative_path', 'adm/'); $container->setParameter('core.php_ext', $php_ext); $container->setParameter('core.table_prefix', ''); diff --git a/tests/functions_user/group_user_attributes_test.php b/tests/functions_user/group_user_attributes_test.php index f13156c2cc..4336fd894e 100644 --- a/tests/functions_user/group_user_attributes_test.php +++ b/tests/functions_user/group_user_attributes_test.php @@ -125,7 +125,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes */ public function test_group_user_attributes($description, $user_id, $group_id, $group_row, $expected) { - global $auth, $cache, $db, $phpbb_dispatcher, $user, $phpbb_container; + global $auth, $cache, $db, $phpbb_dispatcher, $user, $phpbb_container, $phpbb_log, $phpbb_root_path, $phpEx; $user->ip = ''; $cache = new phpbb_mock_cache; @@ -141,6 +141,7 @@ class phpbb_functions_user_group_user_attributes_test extends phpbb_database_tes ->method('get') ->with('cache.driver') ->will($this->returnValue($cache_driver)); + $phpbb_log = new phpbb_log($db, $user, $auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); group_user_attributes('default', $group_id, array($user_id), false, 'group_name', $group_row); From 447e845274daedd12bc40f813680fb5df64d114a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 23 Jan 2013 00:21:01 +0100 Subject: [PATCH 52/56] [ticket/10714] Remove fallback code from previous commits and move global PHPBB3-10714 --- phpBB/includes/functions.php | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 590fc15aa7..6f9d8ad8d6 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3512,16 +3512,9 @@ function parse_cfg_file($filename, $lines = false) */ function add_log() { - global $phpbb_log; + global $phpbb_log, $user; $args = func_get_args(); - $log = (isset($args[0])) ? $args[0] : false; - - if ($log === false) - { - return false; - } - $mode = array_shift($args); // This looks kind of dirty, but add_log has some additional data before the log_operation @@ -3539,11 +3532,10 @@ function add_log() $additional_data['reportee_id'] = array_shift($args); break; } + $log_operation = array_shift($args); $additional_data = array_merge($additional_data, $args); - global $user; - $user_id = (empty($user->data)) ? ANONYMOUS : $user->data['user_id']; $user_ip = (empty($user->ip)) ? '' : $user->ip; From c1b4cdb1883bb6771a303624ece131eb5c7f071d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 25 Feb 2013 15:57:21 +0100 Subject: [PATCH 53/56] [ticket/10714] Update add_log docs block with @param and @deprecated PHPBB3-10714 --- phpBB/includes/functions.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 6f9d8ad8d6..d9116084fd 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3508,7 +3508,18 @@ function parse_cfg_file($filename, $lines = false) } /** -* Add log event +* Add log entry +* +* @param string $mode The mode defines which log_type is used and from which log the entry is retrieved +* @param int $forum_id Mode 'mod' ONLY: forum id of the related item, NOT INCLUDED otherwise +* @param int $topic_id Mode 'mod' ONLY: topic id of the related item, NOT INCLUDED otherwise +* @param int $reportee_id Mode 'user' ONLY: user id of the reportee, NOT INCLUDED otherwise +* @param string $log_operation Name of the operation +* @param array $additional_data More arguments can be added, depending on the log_type +* +* @return int|bool Returns the log_id, if the entry was added to the database, false otherwise. +* +* @deprecated Use $phpbb_log->add() instead */ function add_log() { From 46c4ff46e00e4f050a5c987027a4b05e72456162 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 3 Mar 2013 20:30:17 +0100 Subject: [PATCH 54/56] [ticket/10714] Logs are disabled for this page call only PHPBB3-10714 --- phpBB/includes/log/interface.php | 9 +++++++-- phpBB/includes/log/log.php | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/log/interface.php b/phpBB/includes/log/interface.php index 24bf412ce0..3b459c9bdf 100644 --- a/phpBB/includes/log/interface.php +++ b/phpBB/includes/log/interface.php @@ -33,8 +33,11 @@ interface phpbb_log_interface public function is_enabled($type = ''); /** - * This function allows disabling the log system. When add_log is called - * and the type is disabled, the log will not be added to the database. + * Disable log + * + * This function allows disabling the log system or parts of it, for this + * page call. When add_log is called and the type is disabled, + * the log will not be added to the database. * * @param mixed $type The log type we want to disable. Empty to * disable all logs. Can also be an array of types. @@ -44,6 +47,8 @@ interface phpbb_log_interface public function disable($type = ''); /** + * Enable log + * * This function allows re-enabling the log system. * * @param mixed $type The log type we want to enable. Empty to diff --git a/phpBB/includes/log/log.php b/phpBB/includes/log/log.php index 8da8b63391..7a26858348 100644 --- a/phpBB/includes/log/log.php +++ b/phpBB/includes/log/log.php @@ -177,8 +177,11 @@ class phpbb_log implements phpbb_log_interface } /** - * This function allows disabling the log system. When add_log is called - * and the type is disabled, the log will not be added to the database. + * Disable log + * + * This function allows disabling the log system or parts of it, for this + * page call. When add_log is called and the type is disabled, + * the log will not be added to the database. * * {@inheritDoc} */ @@ -202,6 +205,8 @@ class phpbb_log implements phpbb_log_interface } /** + * Enable log + * * This function allows re-enabling the log system. * * {@inheritDoc} From e0328814f30d51bfac45e9c66d17b29478addd79 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Sun, 3 Mar 2013 20:30:54 +0100 Subject: [PATCH 55/56] [ticket/10714] Use $phpbb_adm_relative_path instead of hardcoded adm/ PHPBB3-10714 --- phpBB/includes/functions_container.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions_container.php b/phpBB/includes/functions_container.php index 36c5ad507e..106b7d75cc 100644 --- a/phpBB/includes/functions_container.php +++ b/phpBB/includes/functions_container.php @@ -57,7 +57,7 @@ function phpbb_create_install_container($phpbb_root_path, $php_ext) $container = phpbb_create_container(array($core), $phpbb_root_path, $php_ext); $container->setParameter('core.root_path', $phpbb_root_path); - $container->setParameter('core.adm_relative_path', 'adm/'); + $container->setParameter('core.adm_relative_path', $phpbb_adm_relative_path); $container->setParameter('core.php_ext', $php_ext); $container->setParameter('core.table_prefix', ''); From 7423d48757bec0a81c8d439e911ed32d5af3a775 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 4 Mar 2013 11:25:27 +0100 Subject: [PATCH 56/56] [ticket/10714] Get log from container in install, update and download/file PHPBB3-10714 --- phpBB/download/file.php | 1 + phpBB/install/database_update.php | 1 + phpBB/install/install_install.php | 5 +++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index 4a392b002d..855813d0d2 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -67,6 +67,7 @@ if (isset($_GET['avatar'])) $phpbb_dispatcher = $phpbb_container->get('dispatcher'); $request = $phpbb_container->get('request'); $db = $phpbb_container->get('dbal.conn'); + $phpbb_log = $phpbb_container->get('log'); // Connect to DB if (!@$db->sql_connect($dbhost, $dbuser, $dbpasswd, $dbname, $dbport, false, false)) diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 4938ef0f87..23048df9fa 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -123,6 +123,7 @@ $request = $phpbb_container->get('request'); $user = $phpbb_container->get('user'); $auth = $phpbb_container->get('auth'); $db = $phpbb_container->get('dbal.conn'); +$phpbb_log = $phpbb_container->get('log'); // make sure request_var uses this request instance request_var('', 0, false, false, $request); // "dependency injection" for a function diff --git a/phpBB/install/install_install.php b/phpBB/install/install_install.php index f0280acc40..d95a500929 100644 --- a/phpBB/install/install_install.php +++ b/phpBB/install/install_install.php @@ -53,7 +53,7 @@ class install_install extends module function main($mode, $sub) { global $lang, $template, $language, $phpbb_root_path, $phpEx; - global $phpbb_container, $cache; + global $phpbb_container, $cache, $phpbb_log; switch ($sub) { @@ -105,8 +105,9 @@ class install_install extends module // Create a normal container now $phpbb_container = phpbb_create_default_container($phpbb_root_path, $phpEx); - // Sets the global $cache variable + // Sets the global variables $cache = $phpbb_container->get('cache'); + $phpbb_log = $phpbb_container->get('log'); $this->build_search_index($mode, $sub); $this->add_modules($mode, $sub);