From 0cfbdcc7449f1cc17b819ffe49aec88c274dd090 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 17 Apr 2010 06:32:15 -0400 Subject: [PATCH] [feature/system-cron] Reorganized cron task parametrization. PHPBB3-9596 --- phpBB/cron.php | 24 ++++---- phpBB/includes/cron/cron_task.php | 26 +++++++- phpBB/includes/cron/cron_task_wrapper.php | 32 +++++++++- .../includes/cron/tasks/core/prune_forum.php | 59 ++++++++++++------- 4 files changed, 107 insertions(+), 34 deletions(-) diff --git a/phpBB/cron.php b/phpBB/cron.php index 5f21e105d3..2d9e6afe5d 100644 --- a/phpBB/cron.php +++ b/phpBB/cron.php @@ -38,9 +38,9 @@ function do_cron($run_tasks) { global $cron_lock; - foreach ($run_tasks as $cron_type) + foreach ($run_tasks as $task) { - $cron->run_task($cron_type); + $task->run(); } // Unloading cache and closing db after having done the dirty work. @@ -54,7 +54,7 @@ if ($cron_lock->lock()) { $use_shutdown_function = false; - $run_tasks = $cron->find_all_runnable_tasks(); + $run_tasks = $cron->find_all_ready_tasks(); } else { @@ -62,14 +62,18 @@ if ($cron_lock->lock()) $use_shutdown_function = (@function_exists('register_shutdown_function')) ? true : false; output_image(); - - if ($cron->is_valid_task($cron_type) && $cron->is_task_runnable($cron_type)) - { - if ($use_shutdown_function && !$cron->is_task_shutdown_function_compatible($cron_type)) - { - $use_shutdown_function = false; + + $task = $cron->find_task($cron_type); + if ($task) { + if ($task->is_parametrized()) { + $task->parse_parameters($_GET); + } + if ($task->is_ready()) { + if ($use_shutdown_function && !$task->is_shutdown_function_safe()) { + $use_shutdown_function = false; + } + $run_tasks = array($task); } - $run_tasks = array($cron_type); } } if ($use_shutdown_function) diff --git a/phpBB/includes/cron/cron_task.php b/phpBB/includes/cron/cron_task.php index 8b9ffacae6..46eacff517 100644 --- a/phpBB/includes/cron/cron_task.php +++ b/phpBB/includes/cron/cron_task.php @@ -48,13 +48,33 @@ interface cron_task } /** -* Parametrized cron task interface +* Parametrized cron task interface. +* +* Parametrized cron tasks are somewhat of a cross between regular cron tasks and +* delayed jobs. Whereas regular cron tasks perform some action globally, +* parametrized cron tasks perform actions on a particular object (or objects). +* Parametrized cron tasks do not make sense and are not usable without +* specifying these objects. +* * @package phpBB3 */ interface parametrized_cron_task extends cron_task { /** - * Returns parameters of this cron task as a query string. + * Returns parameters of this cron task as an array. + * + * The array must map string keys to string values. */ - public function get_url_query_string(); + public function get_parameters(); + + /** + * Parses parameters found in $params, which is an array. + * + * $params contains user input and must not be trusted. + * In normal operation $params contains the same data that was returned by + * get_parameters method. However, a malicious user can supply arbitrary + * data in $params. + * Cron task must validate all keys and values in $params before using them. + */ + public function parse_parameters($params); } diff --git a/phpBB/includes/cron/cron_task_wrapper.php b/phpBB/includes/cron/cron_task_wrapper.php index 3919e4f049..0e63000846 100644 --- a/phpBB/includes/cron/cron_task_wrapper.php +++ b/phpBB/includes/cron/cron_task_wrapper.php @@ -24,11 +24,25 @@ if (!defined('IN_PHPBB')) */ class cron_task_wrapper { + /** + * Wraps a task $task, which must implement cron_task interface. + */ public function __construct($task) { $this->task = $task; } + /** + * Returns whether this task is parametrized. + * + * Parametrized tasks accept parameters during initialization and must + * normally be scheduled with parameters. + */ + public function is_parametrized() + { + return $this->task instanceof parametrized_cron_task; + } + /** * Returns whether the wrapped task is ready to run. * @@ -49,12 +63,28 @@ class cron_task_wrapper return preg_replace('/^cron_task_/', '', $class); } + /** + * Returns a url through which this task may be invoked via web. + */ public function get_url() { global $phpbb_root_path, $phpEx; $name = $this->get_name(); - $url = append_sid($phpbb_root_path . 'cron.' . $phpEx, 'cron_type=' . $name); + if ($this->is_parametrized()) + { + $params = $this->task->get_parameters(); + $extra = ''; + foreach ($params as $key => $value) + { + $extra .= '&' . $key . '=' . urlencode($value); + } + } + else + { + $extra = ''; + } + $url = append_sid($phpbb_root_path . 'cron.' . $phpEx, 'cron_type=' . $name . $extra); return $url; } diff --git a/phpBB/includes/cron/tasks/core/prune_forum.php b/phpBB/includes/cron/tasks/core/prune_forum.php index 4925447162..f9fea7a5b8 100644 --- a/phpBB/includes/cron/tasks/core/prune_forum.php +++ b/phpBB/includes/cron/tasks/core/prune_forum.php @@ -27,6 +27,8 @@ if (!defined('IN_PHPBB')) */ class cron_task_core_prune_forum extends cron_task_base implements parametrized_cron_task { + private $forum_data; + /** * Constructor. * @@ -46,22 +48,7 @@ class cron_task_core_prune_forum extends cron_task_base implements parametrized_ } else { - $forum_id = request_var('f', 0); - - $sql = 'SELECT forum_id, prune_next, enable_prune, prune_days, prune_viewed, forum_flags, prune_freq - FROM ' . FORUMS_TABLE . " - WHERE forum_id = $forum_id"; - $result = $db->sql_query($sql); - $row = $db->sql_fetchrow($result); - $db->sql_freeresult($result); - - if (!$row) - { - // FIXME what to do? - break; - } - - $this->forum_data = $row; + $this->forum_data = null; } } @@ -90,7 +77,7 @@ class cron_task_core_prune_forum extends cron_task_base implements parametrized_ public function is_runnable() { global $config; - return !$config['use_system_cron']; + return !$config['use_system_cron'] && $this->forum_data; } /** @@ -103,10 +90,42 @@ class cron_task_core_prune_forum extends cron_task_base implements parametrized_ } /** - * Returns parameters of this cron task as a query string. + * Returns parameters of this cron task as an array. + * + * The array has one key, f, whose value is id of the forum to be pruned. */ - public function get_url_query_string() + public function get_parameters() { - return 'f=' . $this->forum_data['forum_id']; + return array('f' => $this->forum_data['forum_id']); + } + + /** + * Parses parameters found in $params, which is an array. + * + * $params may contain user input and is not trusted. + * + * $params is expected to have a key f whose value is id of the forum to be pruned. + */ + public function parse_parameters($params) + { + global $db; + + $this->forum_data = null; + if (isset($params['f'])) + { + $forum_id = int($params['f']); + + $sql = 'SELECT forum_id, prune_next, enable_prune, prune_days, prune_viewed, forum_flags, prune_freq + FROM ' . FORUMS_TABLE . " + WHERE forum_id = $forum_id"; + $result = $db->sql_query($sql); + $row = $db->sql_fetchrow($result); + $db->sql_freeresult($result); + + if ($row) + { + $this->forum_data = $row; + } + } } }