[ticket/10631] Validation for extensions

PHPBB3-10631

phpBB/adm/style/acp_ext_details.html

@@ -6,10 +6,12 @@
 
 	<fieldset>
 		<legend>{L_EXT_DETAILS}</legend>
+		<!-- IF MD_DISPLAY_NAME -->
 		<dl>
 			<dt><label for="md_display_name">{L_DISPLAY_NAME}:</label></dt>
 			<dd><strong id="md_display_name">{MD_DISPLAY_NAME}</strong></dd>
 		</dl>
+		<!-- ENDIF -->
 		<dl>
 			<dt><label for="md_name">{L_CLEAN_NAME}:</label></dt>
 			<dd><strong id="md_name">{MD_NAME}</strong></dd>

phpBB/common.php

@@ -119,7 +119,7 @@ set_config(null, null, null, $config);
 set_config_count(null, null, null, $config);
 
 // load extensions
-$phpbb_extension_manager = new phpbb_extension_manager($db, EXT_TABLE, $phpbb_root_path, ".$phpEx", $cache->get_driver());
+$phpbb_extension_manager = new phpbb_extension_manager($db, $config, EXT_TABLE, $phpbb_root_path, ".$phpEx", $cache->get_driver());
 
 // Initialize style
 $phpbb_style_resource_locator = new phpbb_style_resource_locator();

phpBB/download/file.php

@@ -77,7 +77,7 @@ if (isset($_GET['avatar']))
 	set_config_count(null, null, null, $config);
 
 	// load extensions
-	$phpbb_extension_manager = new phpbb_extension_manager($db, EXT_TABLE, $phpbb_root_path, ".$phpEx", $cache->get_driver());
+	$phpbb_extension_manager = new phpbb_extension_manager($db, $config, EXT_TABLE, $phpbb_root_path, ".$phpEx", $cache->get_driver());
 
 	$phpbb_subscriber_loader = new phpbb_event_extension_subscriber_loader($phpbb_dispatcher, $phpbb_extension_manager);
 	$phpbb_subscriber_loader->load();

phpBB/includes/acp/acp_extensions.php

@@ -25,7 +25,7 @@ class acp_extensions
 	function main()
 	{
 		// Start the page
-		global $user, $template, $request, $phpbb_extension_manager, $db, $phpbb_root_path, $phpEx;
+		global $config, $user, $template, $request, $phpbb_extension_manager, $db, $phpbb_root_path, $phpEx;
 
 		$user->add_lang(array('install', 'acp/extensions'));
 
@@ -34,6 +34,17 @@ class acp_extensions
 		$action = $request->variable('action', 'list');
 		$ext_name = $request->variable('ext_name', '');
 
+		// If they've specificed an extension, let's load the metadata manager and validate it.
+		if ($ext_name)
+		{
+			$md_manager = new phpbb_extension_metadata_manager($ext_name, $db, $phpbb_extension_manager, $phpbb_root_path, ".$phpEx", $template, $config);
+
+			if ($md_manager->get_metadata('all') === false)
+			{
+				trigger_error('EXTENSION_INVALID');
+			}
+		}
+
 		// What are we doing?
 		switch ($action)
 		{
@@ -47,6 +58,11 @@ class acp_extensions
 			break;
 
 			case 'enable_pre':
+				if (!$md_manager->validate_enable())
+				{
+					trigger_error('EXTENSION_NOT_AVAILABLE');
+				}
+
 				$this->tpl_name = 'acp_ext_enable';
 
 				$template->assign_vars(array(
@@ -56,6 +72,11 @@ class acp_extensions
 			break;
 
 			case 'enable':
+				if (!$md_manager->validate_enable())
+				{
+					trigger_error('EXTENSION_NOT_AVAILABLE');
+				}
+
 				if ($phpbb_extension_manager->enable_step($ext_name))
 				{
 					$template->assign_var('S_NEXT_STEP', true);
@@ -132,12 +153,8 @@ class acp_extensions
 			break;*/
 
 			case 'details':
-				$md_manager = new phpbb_extension_metadata_manager($ext_name, $db, $phpbb_extension_manager, $phpbb_root_path, ".$phpEx", $template);
+				// Output it to the template
-				
+				$md_manager->output_template_data();
-				if ($md_manager->get_metadata('all', true) === false)
-				{
-					trigger_error('EXTENSION_INVALID');
-				}
 
 				$this->tpl_name = 'acp_ext_details';
 			break;

phpBB/includes/extension/manager.php

@@ -23,6 +23,7 @@ if (!defined('IN_PHPBB'))
 class phpbb_extension_manager
 {
 	protected $db;
+	protected $config;
 	protected $cache;
 	protected $php_ext;
 	protected $extensions;
@@ -34,16 +35,18 @@ class phpbb_extension_manager
 	* Creates a manager and loads information from database
 	*
 	* @param dbal $db A database connection
+	* @param phpbb_config $config phpbb_config
 	* @param string $extension_table The name of the table holding extensions
 	* @param string $phpbb_root_path Path to the phpbb includes directory.
 	* @param string $php_ext php file extension
 	* @param phpbb_cache_driver_interface $cache A cache instance or null
 	* @param string $cache_name The name of the cache variable, defaults to _ext
 	*/
-	public function __construct(dbal $db, $extension_table, $phpbb_root_path, $php_ext = '.php', phpbb_cache_driver_interface $cache = null, $cache_name = '_ext')
+	public function __construct(dbal $db, phpbb_config $config, $extension_table, $phpbb_root_path, $php_ext = '.php', phpbb_cache_driver_interface $cache = null, $cache_name = '_ext')
 	{
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->db = $db;
+		$this->config = $config;
 		$this->cache = $cache;
 		$this->php_ext = $php_ext;
 		$this->extension_table = $extension_table;
@@ -130,7 +133,7 @@ class phpbb_extension_manager
 	*/
 	public function get_extension_metadata($name, phpbb_template $template)
 	{
-		return new phpbb_extension_metadata_manager($name, $this->db, $this, $this->phpbb_root_path, $this->phpEx, $template);
+		return new phpbb_extension_metadata_manager($name, $this->db, $this, $this->phpbb_root_path, $this->php_ext, $template, $this->config);
 	}
 
 	/**

phpBB/includes/extension/metadata_manager.php

@@ -31,6 +31,58 @@ class phpbb_extension_metadata_manager
 	public $metadata;
 	protected $metadata_file;
 
+	/**
+	* Array of validation regular expressions, see __call()
+	*
+	* @var mixed
+	*/
+	protected $validation = array(
+		'name'					=> '#^[a-zA-Z0-9_\x7f-\xff]{2,}/[a-zA-Z0-9_\x7f-\xff]{2,}$#',
+		'type'					=> '#^phpbb3-extension$#',
+		'description'			=> '#.*#',
+		'version'				=> '#.+#',
+		'licence'				=> '#.+#',
+		'extra'					=> array(
+			'display-name'			=> '#.*#',
+		),
+	);
+
+	/**
+	* Magic method to catch validation calls
+	*
+	* @param string $name
+	* @param mixed $arguments
+	* @return int
+	*/
+	public function __call($name, $arguments)
+    {
+    	// Validation Magic methods
+        if (strpos($name, 'validate_') === 0)
+        {
+        	// Remove validate_
+        	$name = substr($name, 9);
+
+        	// Replace underscores with dashes (underscores are not used)
+        	$name = str_replace('_', '-', $name);
+
+        	if (strpos($name, 'extra-') === 0)
+        	{
+        		// Remove extra_
+        		$name = substr($name, 6);
+
+        		if (isset($this->validation['extra'][$name]))
+        		{
+        			// Extra means it's optional, so return true if it does not exist
+        			return (isset($this->metadata['extra'][$name])) ? preg_match($this->validation['extra'][$name], $this->metadata['extra'][$name]) : true;
+				}
+			}
+			else if (isset($this->validation[$name]))
+	        {
+        		return preg_match($this->validation[$name], $this->metadata[$name]);
+			}
+		}
+    }
+
 	/**
 	* Creates the metadata manager
 	*
@@ -39,10 +91,11 @@ class phpbb_extension_metadata_manager
 	* @param string $phpbb_root_path Path to the phpbb includes directory.
 	* @param string $phpEx php file extension
 	*/
-	public function __construct($ext_name, dbal $db, phpbb_extension_manager $extension_manager, $phpbb_root_path, $phpEx = '.php', phpbb_template $template)
+	public function __construct($ext_name, dbal $db, phpbb_extension_manager $extension_manager, $phpbb_root_path, $phpEx = '.php', phpbb_template $template, phpbb_config $config)
 	{
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->db = $db;
+		$this->config = $config;
 		$this->phpEx = $phpEx;
 		$this->template = $template;
 		$this->extension_manager = $extension_manager;
@@ -54,11 +107,10 @@ class phpbb_extension_metadata_manager
 	/**
 	 * Processes and gets the metadata requested
 	 *
-	 * @param  string $element         All for all metadata that it has and is valid, otherwise specify which section you want by its shorthand term.
+	 * @param  string $element			All for all metadata that it has and is valid, otherwise specify which section you want by its shorthand term.
-	 * @param  boolean $template_output   True if you want the requested metadata assigned to template vars (only works on the 'all" case
+	 * @return bool|array				Contains all of the requested metadata or bool False if not valid
-	 * @return array                   Contains all of the requested metadata
 	 */
-	public function get_metadata($element = 'all', $template_output = false)
+	public function get_metadata($element = 'all')
 	{
 		// TODO: Check ext_name exists and is an extension that exists
 		if (!$this->set_metadata_file())
@@ -66,25 +118,28 @@ class phpbb_extension_metadata_manager
 			return false;
 		}
 
+		// Fetch the metadata
 		if (!$this->fetch_metadata())
 		{
 			return false;
 		}
 
+		// Clean the metadata
+		if (!$this->clean_metadata_array())
+		{
+			return false;
+		}
+
 		switch ($element)
 		{
 			case 'all':
 			default:
-				if (!$this->clean_metadata_array())
+				// Validate the metadata
+				if (!$this->validate_metadata_array())
 				{
 					return false;
 				}
 
-				if ($template_output) 
-				{
-					$this->output_template_data();
-				}
-
 				return $this->metadata;
 			break;
 
@@ -93,7 +148,7 @@ class phpbb_extension_metadata_manager
 			break;
 
 			case 'display-name':
-				if ($this->validate_extra_display_name())
+				if (isset($this->metadata['extra']['display-name']) && $this->validate_extra_display_name())
 				{
 					return $this->metadata['extra']['display-name'];
 				}
@@ -128,6 +183,35 @@ class phpbb_extension_metadata_manager
 		}
 	}
 
+	/**
+	 * Gets the contents of the composer.json file
+	 *
+	 * @return bool True of false (if loading succeeded or failed)
+	 */
+	private function fetch_metadata()
+	{
+		if (!file_exists($this->metadata_file))
+		{
+			return false;
+		}
+		else
+		{
+			if (!($file_contents = file_get_contents($this->metadata_file)))
+			{
+				return false;
+			}
+
+			if (($metadata = json_decode($file_contents, true)) === NULL)
+			{
+				return false;
+			}
+
+			$this->metadata = $metadata;
+
+			return true;
+		}
+	}
+
 	/**
 	 * This array handles the validation and cleaning of the array
 	 *
@@ -135,13 +219,6 @@ class phpbb_extension_metadata_manager
 	 */
 	private function clean_metadata_array()
 	{
-		if (!$this->validate_name() || !$this->validate_type() || !$this->validate_licence() || !$this->validate_description() || !$this->validate_version() || !$this->validate_require_phpbb() || !$this->validate_extra_display_name())
-		{
-			return false;
-		}
-		
-		$this->check_for_optional(true);
-
 //		TODO: Remove all parts of the array we don't want or shouldn't be there due to nub mod authors
 //		$this->metadata = $metadata_finished;
 
@@ -149,55 +226,59 @@ class phpbb_extension_metadata_manager
 	}
 
 	/**
-	 * Validates the contents of the name field
+	 * This array handles the validation of strings
 	 *
-	 * @return boolean True when passes validation
+	 * @return bool True if validation succeeded, False if failed
 	 */
-	private function validate_name()
+	public function validate_metadata_array()
 	{
-		return preg_match('#^[a-zA-Z0-9_\x7f-\xff]{2,}/[a-zA-Z0-9_\x7f-\xff]{2,}$#', $this->metadata['name']);
+		$validate = array(
+			'name',
+			'type',
+			'licence',
+			'description',
+			'version',
+			'extra_display-name',
+		);
+
+		foreach ($validate as $type)
+		{
+			$type = 'validate_' . $type;
+
+			if (!$this->$type())
+			{
+				return false;
+			}
+		}
+
+		return true;
 	}
 
 	/**
-	 * Validates the contents of the type field
+	 * This array handles the verification that this extension can be enabled on this board
 	 *
-	 * @return boolean True when passes validation
+	 * @return bool True if validation succeeded, False if failed
 	 */
-	private function validate_type()
+	public function validate_enable()
 	{
-		return $this->metadata['type'] == 'phpbb3-extension';
+		$validate = array(
+			'require_phpbb',
+			'require_php',
+		);
+
+		foreach ($validate as $type)
+		{
+			$type = 'validate_' . $type;
+
+			if (!$this->$type())
+			{
+				return false;
+			}
+		}
+
+		return true;
 	}
 
-	/**
-	 * Validates the contents of the description field
-	 * 
-	 * @return boolean True when passes validation
-	 */
-	private function validate_description()
-	{
-		return true;//preg_match('#^{10,}$#', $this->metadata['description']);
-	}
-
-	/**
-	 * Validates the contents of the version field
-	 * 
-	 * @return boolean True when passes validation
-	 */
-	private function validate_version()
-	{
-		return preg_match('#^[0-9]{1,2}\.[0-9]{1,2}\.[0-9]{1,2}#', $this->metadata['version']);
-	}
-
-	/**
-	 * Validates the contents of the license field
-	 * 
-	 * @return boolean True when passes validation
-	 */
-	private function validate_licence()
-	{
-		// Nothing to validate except existence
-		return isset($this->metadata['licence']);
-	}
 
 	/**
 	 * Validates the contents of the phpbb requirement field
@@ -206,30 +287,12 @@ class phpbb_extension_metadata_manager
 	 */
 	private function validate_require_phpbb()
 	{
-		return (preg_match('#^[0-9]{1,2}\.[0-9]{1,2}\.[0-9]{1,2}$#', $this->metadata['require']['phpbb']) && version_compare($this->metadata['require']['phpbb'], '3.1.0', '>='));
+		if (!isset($this->metadata['require']['phpbb']))
-	}
-
-	/**
-	 * Validates the contents of the display name field
-	 * 
-	 * @return boolean True when passes validation
-	 */
-	private function validate_extra_display_name()
-	{
-		return true;//preg_match('#^[a-zA-Z0-9_]{2,0}$#', $this->metadata['name']);
-	}
-
-	/**
-	 * Checks which optional fields exist
-	 * 
-	 * @return boolean           False if any that exist fail validation, otherwise true.
-	 */
-	public function check_for_optional()
-	{
-		if ((isset($this->metadata['require']['php']) && !$this->validate_require_php()) || (isset($this->metadata['time']) && !$this->validate_time()) || (isset($this->metadata['validate_homepage']) && !$this->validate_homepage()))
 		{
-			return false;
+			return true;
 		}
+
+		return $this->_validate_version($this->metadata['require']['phpbb'], $this->config['version']);
 	}
 
 	/**
@@ -239,7 +302,33 @@ class phpbb_extension_metadata_manager
 	 */
 	private function validate_require_php()
 	{
-		return (preg_match('#^[0-9]{1,2}\.[0-9]{1,2}\.[0-9]{1,2}$#', $this->metadata['require']['php']) && version_compare($this->metadata['require']['php'], phpversion(), '>='));
+		if (!isset($this->metadata['require']['php']))
+		{
+			return true;
+		}
+
+		return $this->_validate_version($this->metadata['require']['php'], phpversion());
+	}
+
+	/**
+	* Version validation helper
+	*
+	* @param string $string The string for comparing to a version
+	* @param string $current_version The version to compare to
+	* @return bool True/False if meets version requirements
+	*/
+	private function _validate_version($string, $current_version)
+	{
+		// Allow them to specify their own comparison operator (ex: <3.1.2, >=3.1.0)
+		$comparison_matches = false;
+		preg_match('#[=<>]+#', $string, $comparison_matches);
+
+		if (!empty($comparison_matches))
+		{
+			return version_compare($current_version, str_replace(array($comparison_matches[0], ' '), '', $string), $comparison_matches[0]);
+		}
+
+		return version_compare($current_version, $string, '>=');
 	}
 
 	/**
@@ -291,35 +380,6 @@ class phpbb_extension_metadata_manager
 		return true;
 	}
 
-	/**
-	 * Gets the contents of the composer.json file
-	 *
-	 * @return bool True of false (if loading succeeded or failed)
-	 */
-	private function fetch_metadata()
-	{
-		if (!file_exists($this->metadata_file))
-		{
-			return false;
-		}
-		else
-		{
-			if (!($file_contents = file_get_contents($this->metadata_file)))
-			{
-				return false;
-			}
-			
-			if (($metadata = json_decode($file_contents, true)) === NULL)
-			{
-				return false;
-			}
-			
-			$this->metadata = $metadata;
-			
-			return true;
-		}
-	}
-
 	/**
 	 * Outputs the metadata into the template
 	 *

phpBB/install/database_update.php

@@ -683,12 +683,12 @@ function _write_result($no_updates, $errored, $error_ary)
 
 function _add_modules($modules_to_install)
 {
-	global $phpbb_root_path, $phpEx, $db, $phpbb_extension_manager;
+	global $phpbb_root_path, $phpEx, $db, $phpbb_extension_manager, $config;
 
 	// modules require an extension manager
 	if (empty($phpbb_extension_manager))
 	{
-		$phpbb_extension_manager = new phpbb_extension_manager($db, EXT_TABLE, $phpbb_root_path, ".$phpEx");
+		$phpbb_extension_manager = new phpbb_extension_manager($db, $config, EXT_TABLE, $phpbb_root_path, ".$phpEx");
 	}
 
 	include_once($phpbb_root_path . 'includes/acp/acp_modules.' . $phpEx);

phpBB/install/install_install.php

@@ -1481,12 +1481,12 @@ class install_install extends module
 	*/
 	function add_modules($mode, $sub)
 	{
-		global $db, $lang, $phpbb_root_path, $phpEx, $phpbb_extension_manager;
+		global $db, $lang, $phpbb_root_path, $phpEx, $phpbb_extension_manager, $config;
 
 		// modules require an extension manager
 		if (empty($phpbb_extension_manager))
 		{
-			$phpbb_extension_manager = new phpbb_extension_manager($db, EXT_TABLE, $phpbb_root_path, ".$phpEx");
+			$phpbb_extension_manager = new phpbb_extension_manager($db, $config, EXT_TABLE, $phpbb_root_path, ".$phpEx");
 		}
 
 		include_once($phpbb_root_path . 'includes/acp/acp_modules.' . $phpEx);

phpBB/language/en/acp/extensions.php

@@ -35,11 +35,12 @@ if (empty($lang) || !is_array($lang))
 
 
 $lang = array_merge($lang, array(
-	'EXTENSION'				=> 'Extension',
+	'EXTENSION'					=> 'Extension',
-	'EXTENSIONS'			=> 'Extensions',
+	'EXTENSIONS'				=> 'Extensions',
-	'EXTENSIONS_ADMIN'		=> 'Extensions Manager',
+	'EXTENSIONS_ADMIN'			=> 'Extensions Manager',
-	'EXTENSIONS_EXPLAIN'	=> 'The Extensions Manager is a tool in your phpBB Board which allows you to manage all of your extensions statuses and view information about them.',
+	'EXTENSIONS_EXPLAIN'		=> 'The Extensions Manager is a tool in your phpBB Board which allows you to manage all of your extensions statuses and view information about them.',
-	'EXTENSION_INVALID'		=> 'The selected extension is not valid.',
+	'EXTENSION_INVALID'			=> 'The selected extension is not valid.',
+	'EXTENSION_NOT_AVAILABLE'	=> 'The selected extension is not available for this board, please verify your phpBB and PHP versions are allowed (see the details page).',
 
 	'DETAILS'				=> 'Details',
 
@@ -63,27 +64,27 @@ $lang = array_merge($lang, array(
 	'DISABLE_IN_PROGRESS'	=> 'The extension is currently being disabled, please do not leave this page or refresh until it is completed.',
 	'ENABLE_IN_PROGRESS'	=> 'The extension is currently being installed, please do not leave this page or refresh until it is completed.',
 	'PURGE_IN_PROGRESS'		=> 'The extension is currently being purged, please do not leave this page or refresh until it is completed.',
-	'ENABLE_SUCCESS'			=> 'The extension was enabled successfully',
+	'ENABLE_SUCCESS'		=> 'The extension was enabled successfully',
 	'DISABLE_SUCCESS'		=> 'The extension was disabled successfully',
 	'PURGE_SUCCESS'			=> 'The extension was purged successfully',
-	'DELETE_SUCCESS'			=> 'The extension was deleted successfully',
+	'DELETE_SUCCESS'		=> 'The extension was deleted successfully',
 
 	'ENABLE_FAIL'			=> 'The extension could not be enabled',
 	'DISABLE_FAIL'			=> 'The extension could not be disabled',
 	'PURGE_FAIL'			=> 'The extension could not be purged',
 	'DELETE_FAIL'			=> 'The extension could not be deleted',
 
-	'EXTENSION_NAME'			=> 'Extension Name',
+	'EXTENSION_NAME'		=> 'Extension Name',
-	'EXTENSION_ACTIONS'			=> 'Actions',
+	'EXTENSION_ACTIONS'		=> 'Actions',
-	'EXTENSION_OPTIONS'			=> 'Options',
+	'EXTENSION_OPTIONS'		=> 'Options',
 
-	'ENABLE_CONFIRM'			=> 'Are you sure that you wish to enable this extension?',
+	'ENABLE_CONFIRM'		=> 'Are you sure that you wish to enable this extension?',
-	'DISABLE_CONFIRM'			=> 'Are you sure that you wish to disable this extension?',
+	'DISABLE_CONFIRM'		=> 'Are you sure that you wish to disable this extension?',
-	'PURGE_CONFIRM'				=> 'Are you sure that you wish to purge this extension's data? This cannot be undone.',
+	'PURGE_CONFIRM'			=> 'Are you sure that you wish to purge this extension's data? This cannot be undone.',
-	'DELETE_CONFIRM'			=> 'Are you sure that you wish to data this extension's files and clear its data? This cannot be undone.',
+	'DELETE_CONFIRM'		=> 'Are you sure that you wish to data this extension's files and clear its data? This cannot be undone.',
 
-	'WARNING'			=> 'Warning',
+	'WARNING'				=> 'Warning',
-	'RETURN'			=> 'Return',
+	'RETURN'				=> 'Return',
 
 	'EXT_DETAILS'			=> 'Extension Details',
 	'DISPLAY_NAME'			=> 'Display Name',