[feature/system-cron] Reorganized cron task parametrization.

PHPBB3-9596

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
 	{
@@ -63,13 +63,17 @@ if ($cron_lock->lock())
 
 		output_image();
 		
-		if ($cron->is_valid_task($cron_type) && $cron->is_task_runnable($cron_type))
+		$task = $cron->find_task($cron_type);
-		{
+		if ($task) {
-			if ($use_shutdown_function && !$cron->is_task_shutdown_function_compatible($cron_type))
+			if ($task->is_parametrized()) {
-			{
+				$task->parse_parameters($_GET);
-				$use_shutdown_function = false;
+			}
+			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)

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);
 }

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;
 	}
 

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);
+			$this->forum_data = null;
-
-			$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;
 		}
 	}
 
@@ -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;
+			}
+		}
 	}
 }