[ticket/10631] Fix some issues as noted in github comments, significantly simplified validation

PHPBB3-10631
author: Nathan Guse <nathaniel.guse@gmail.com> 2012-07-23 18:22:35 -0500
committer: Unknown Bliss <m@michaelcullum.com> 2012-09-01 15:05:32 +0100
commit: 106c105113886f9a9e603dbb11549c06049b255f (patch)
tree: d6f242600b03211e983f1879a28790384049703d
parent: 2273ae2b34071160ff930ca8d49326b8dd308899 (diff)
download: forums-106c105113886f9a9e603dbb11549c06049b255f.tar
forums-106c105113886f9a9e603dbb11549c06049b255f.tar.gz
forums-106c105113886f9a9e603dbb11549c06049b255f.tar.bz2
forums-106c105113886f9a9e603dbb11549c06049b255f.tar.xz
forums-106c105113886f9a9e603dbb11549c06049b255f.zip
4 files changed, 82 insertions, 140 deletions
diff --git a/phpBB/adm/style/acp_ext_details.html b/phpBB/adm/style/acp_ext_details.html
index 5af9603a75..7408e88758 100644
--- a/phpBB/adm/style/acp_ext_details.html
+++ b/phpBB/adm/style/acp_ext_details.html
@@ -16,36 +16,35 @@
 			<dt><label for="md_name">{L_CLEAN_NAME}:</label></dt>
 			<dd><strong id="md_name">{MD_NAME}</strong></dd>
 		</dl>
-		<dl>
-			<dt><label for="md_type">{L_TYPE}:</label></dt>
-			<dd><p id="md_type">{MD_TYPE}</p></dd>
-		</dl>
+		<!-- IF MD_DESCRIPTION -->
 		<dl>
 			<dt><label for="md_description">{L_DESCRIPTION}:</label></dt>
 			<dd><p id="md_description">{MD_DESCRIPTION}</p></dd>
 		</dl>
+		<!-- ENDIF -->
 		<dl>
 			<dt><label for="md_version">{L_VERSION}:</label></dt>
 			<dd><p id="md_version">{MD_VERSION}</p></dd>
 		</dl>
+		<!-- IF MD_HOMEPAGE -->
 		<dl>
 			<dt><label for="md_homepage">{L_HOMEPAGE}:</label></dt>
 			<dd><p id="md_homepage">{MD_HOMEPAGE}</p></dd>
 		</dl>
-<!-- 		<dl>
-			<dt><label for="path">{L_PATH}:</label></dt>
-			<dd><p id="path">{PATH}</p></dd>
-		</dl> -->
+		<!-- ENDIF -->
+		<!-- IF MD_TIME -->
 		<dl>
 			<dt><label for="md_time">{L_TIME}:</label></dt>
 			<dd><p id="md_time">{MD_TIME}</p></dd>
 		</dl>
+		<!-- ENDIF -->
 		<dl>
 			<dt><label for="md_license">{L_LICENCE}:</label></dt>
 			<dd><p id="md_license">{MD_LICENCE}</p></dd>
 		</dl>
 	</fieldset>
 
+	<!-- IF MD_REQUIRE_PHPBB || MD_REQUIRE_PHP -->
 	<fieldset>
 		<legend>{L_REQUIREMENTS}</legend>
 		<!-- IF MD_REQUIRE_PHPBB -->
@@ -61,34 +60,35 @@
 		</dl>
 		<!-- ENDIF -->
 	</fieldset>
+	<!-- ENDIF -->
 
 	<fieldset>
 		<legend>{L_AUTHOR_INFORMATION}</legend>
 		<!-- BEGIN md_authors -->
-		<dl>
-			<dt><label for="md_author_name">{L_AUTHOR_NAME}:</label></dt>
-			<dd><strong id="md_author_name">{md_authors.AUTHOR_NAME}</strong></dd>
-		</dl>
-		<!-- IF md_authors.AUTHOR_EMAIL -->
-		<dl>
-			<dt><label for="md_author_email">{L_AUTHOR_EMAIL}:</label></dt>
-			<dd><strong id="md_author_email"><a href="mailto:{md_authors.AUTHOR_EMAIL}">{md_authors.AUTHOR_EMAIL}</a></strong></dd>
-		</dl>
-		<!-- ENDIF -->
-		<!-- IF md_authors.AUTHOR_HOMEPAGE -->
-		<dl>
-			<dt><label for="md_author_url">{L_AUTHOR_HOMEPAGE}:</label></dt>
-			<dd><strong id="md_author_url"><a href="{md_authors.AUTHOR_HOMEPAGE}">{md_authors.AUTHOR_HOMEPAGE}</a></strong></dd>
-		</dl>
-		<!-- ENDIF -->
-		<!-- IF md_authors.AUTHOR_ROLE -->
-		<dl>
-			<dt><label for="author_role">{L_AUTHOR_ROLE}:</label></dt>
-			<dd><strong id="md_author_role">{md_authors.AUTHOR_ROLE}</strong></dd>
-		</dl>
-		<!-- ENDIF -->
-
-		<br /><br />
+		<fieldset>
+			<dl>
+				<dt><label for="md_author_name">{L_AUTHOR_NAME}:</label></dt>
+				<dd><strong id="md_author_name">{md_authors.AUTHOR_NAME}</strong></dd>
+			</dl>
+			<!-- IF md_authors.AUTHOR_EMAIL -->
+			<dl>
+				<dt><label for="md_author_email">{L_AUTHOR_EMAIL}:</label></dt>
+				<dd><strong id="md_author_email"><a href="mailto:{md_authors.AUTHOR_EMAIL}">{md_authors.AUTHOR_EMAIL}</a></strong></dd>
+			</dl>
+			<!-- ENDIF -->
+			<!-- IF md_authors.AUTHOR_HOMEPAGE -->
+			<dl>
+				<dt><label for="md_author_url">{L_AUTHOR_HOMEPAGE}:</label></dt>
+				<dd><strong id="md_author_url"><a href="{md_authors.AUTHOR_HOMEPAGE}">{md_authors.AUTHOR_HOMEPAGE}</a></strong></dd>
+			</dl>
+			<!-- ENDIF -->
+			<!-- IF md_authors.AUTHOR_ROLE -->
+			<dl>
+				<dt><label for="author_role">{L_AUTHOR_ROLE}:</label></dt>
+				<dd><strong id="md_author_role">{md_authors.AUTHOR_ROLE}</strong></dd>
+			</dl>
+			<!-- ENDIF -->
+		</fieldset>
 		<!-- END md_authors -->
 	</fieldset>
 
diff --git a/phpBB/includes/acp/acp_extensions.php b/phpBB/includes/acp/acp_extensions.php
index 0e1d5c88a8..0e825514e6 100644
--- a/phpBB/includes/acp/acp_extensions.php
+++ b/phpBB/includes/acp/acp_extensions.php
@@ -155,11 +155,11 @@ class acp_extensions
 	 * @param  $template 					An instance of the template engine
 	 * @return null
 	 */
-	private function list_enabled_exts(phpbb_extension_manager $phpbb_extension_manager, phpbb_template $template)
+	public function list_enabled_exts(phpbb_extension_manager $phpbb_extension_manager, phpbb_template $template)
 	{
 		foreach ($phpbb_extension_manager->all_enabled() as $name => $location)
 		{
-			$md_manager = $phpbb_extension_manager->get_extension_metadata($name, $template);
+			$md_manager = $phpbb_extension_manager->get_extension_metadata_manager($name, $template);
 
 			$template->assign_block_vars('enabled', array(
 				'EXT_NAME'		=> $md_manager->get_metadata('display-name'),
@@ -178,11 +178,11 @@ class acp_extensions
 	 * @param  $template 					An instance of the template engine
 	 * @return null
 	 */
-	private function list_disabled_exts(phpbb_extension_manager $phpbb_extension_manager, phpbb_template $template)
+	public function list_disabled_exts(phpbb_extension_manager $phpbb_extension_manager, phpbb_template $template)
 	{
 		foreach ($phpbb_extension_manager->all_disabled() as $name => $location)
 		{
-			$md_manager = $phpbb_extension_manager->get_extension_metadata($name, $template);
+			$md_manager = $phpbb_extension_manager->get_extension_metadata_manager($name, $template);
 
 			$template->assign_block_vars('disabled', array(
 				'EXT_NAME'		=> $md_manager->get_metadata('display-name'),
@@ -201,13 +201,13 @@ class acp_extensions
 	 * @param  $template 					An instance of the template engine
 	 * @return null
 	 */
-	function list_available_exts(phpbb_extension_manager $phpbb_extension_manager, phpbb_template $template)
+	public function list_available_exts(phpbb_extension_manager $phpbb_extension_manager, phpbb_template $template)
 	{
 		$uninstalled = array_diff_key($phpbb_extension_manager->all_available(), $phpbb_extension_manager->all_configured());
 
 		foreach ($uninstalled as $name => $location)
 		{
-			$md_manager = $phpbb_extension_manager->get_extension_metadata($name, $template);
+			$md_manager = $phpbb_extension_manager->get_extension_metadata_manager($name, $template);
 
 			$template->assign_block_vars('disabled', array(
 				'EXT_NAME'		=> $md_manager->get_metadata('display-name'),
diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php
index 9dfc0a067c..9342c936f9 100644
--- a/phpBB/includes/extension/manager.php
+++ b/phpBB/includes/extension/manager.php
@@ -131,7 +131,7 @@ class phpbb_extension_manager
 	* @param string $template The template manager
 	* @return phpbb_extension_metadata_manager Instance of the metadata manager
 	*/
-	public function get_extension_metadata($name, phpbb_template $template)
+	public function get_extension_metadata_manager($name, phpbb_template $template)
 	{
 		return new phpbb_extension_metadata_manager($name, $this->db, $this, $this->phpbb_root_path, $this->php_ext, $template, $this->config);
 	}
diff --git a/phpBB/includes/extension/metadata_manager.php b/phpBB/includes/extension/metadata_manager.php
index d2dc72e5fc..aa163b1190 100644
--- a/phpBB/includes/extension/metadata_manager.php
+++ b/phpBB/includes/extension/metadata_manager.php
@@ -28,63 +28,10 @@ class phpbb_extension_metadata_manager
 	protected $phpbb_root_path;
 	protected $template;
 	protected $ext_name;
-	public $metadata;
+	protected $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'				=> '#.+#',
-		//'homepage'				=> '#([\d\w-.]+?\.(a[cdefgilmnoqrstuwz]|b[abdefghijmnorstvwyz]|c[acdfghiklmnoruvxyz]|d[ejkmnoz]|e[ceghrst]|f[ijkmnor]|g[abdefghilmnpqrstuwy]|h[kmnrtu]|i[delmnoqrst]|j[emop]|k[eghimnprwyz]|l[abcikrstuvy]|m[acdghklmnopqrstuvwxyz]|n[acefgilopruz]|om|p[aefghklmnrstwy]|qa|r[eouw]|s[abcdeghijklmnortuvyz]|t[cdfghjkmnoprtvwz]|u[augkmsyz]|v[aceginu]|w[fs]|y[etu]|z[amw]|aero|arpa|biz|com|coop|edu|info|int|gov|mil|museum|name|net|org|pro)(\b|\W(?<!&|=)(?!\.\s|\.{3}).*?))(\s|$)#',
-		'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]) && isset($this->metadata[$name]))
-	        {
-        		return preg_match($this->validation[$name], $this->metadata[$name]);
-			}
-		}
-    }
-
-	/**
 	* Creates the metadata manager
 	*
 	* @param dbal $db A database connection
@@ -136,7 +83,7 @@ class phpbb_extension_metadata_manager
 			case 'all':
 			default:
 				// Validate the metadata
-				if (!$this->validate_metadata_array())
+				if (!$this->validate())
 				{
 					return false;
 				}
@@ -145,17 +92,17 @@ class phpbb_extension_metadata_manager
 			break;
 
 			case 'name':
-				return ($this->validate_name()) ? $this->metadata['name'] : false;
+				return ($this->validate('name')) ? $this->metadata['name'] : false;
 			break;
 
 			case 'display-name':
-				if (isset($this->metadata['extra']['display-name']) && $this->validate_extra_display_name())
+				if (isset($this->metadata['extra']['display-name']))
 				{
 					return $this->metadata['extra']['display-name'];
 				}
 				else
 				{
-					return ($this->validate_name()) ? $this->metadata['name'] : false;
+					return ($this->validate('name')) ? $this->metadata['name'] : false;
 				}
 			break;
 			// TODO: Add remaining cases as needed
@@ -216,7 +163,7 @@ class phpbb_extension_metadata_manager
 	/**
 	 * This array handles the validation and cleaning of the array
 	 *
-	 * @return array Contains the cleaned and validated metadata array
+	 * @return array Contains the cleaned metadata array
 	 */
 	private function clean_metadata_array()
 	{
@@ -227,40 +174,44 @@ class phpbb_extension_metadata_manager
 	}
 
 	/**
-	 * This array handles the validation of strings
-	 *
-	 * @return bool True if validation succeeded, False if failed
-	 */
-	public function validate_metadata_array()
-	{
-		foreach ($this->validation as $name => $regex)
-		{
-			if (is_array($regex))
-			{
-				foreach ($regex as $extra_name => $extra_regex)
-				{
-					$type = 'validate_' . $name . '_' . $extra_name;
+	* Validate fields
+	*
+	* @param string $name  ("all" for display and enable validation
+	* 						"display" for name, type, and authors
+	* 						"name", "type")
+	* @return Bool False if validation fails, true if valid
+	*/
+	public function validate($name = 'display')
+    {
+    	// Basic fields
+    	$fields = array(
+    		'name'		=> '#^[a-zA-Z0-9_\x7f-\xff]{2,}/[a-zA-Z0-9_\x7f-\xff]{2,}$#',
+    		'type'		=> '#^phpbb3-extension$#',
+    		'licence'	=> '#.+#',
+    		'version'	=> '#.+#',
+    	);
+
+    	if (isset($fields[$name]))
+    	{
+    		return (isset($this->metadata[$name])) ? (bool) preg_match($this->validation[$name], $this->metadata[$name]) : false;
+		}
 
-					if (!$this->$type())
-					{
-						return false;
-					}
-				}
-			}
-			else
+    	// Validate all fields
+    	if ($name == 'all')
+    	{
+    		foreach ($fields as $field => $data)
 			{
-
-				$type = 'validate_' . $name;
-
-				if (!$this->$type())
+				if (!$this->validate($field))
 				{
 					return false;
 				}
 			}
+
+			return $this->validate_authors();
 		}
 
-		return $this->validate_authors();
-	}
+		return true;
+    }
 
 	/**
 	 * Validates the contents of the authors field
@@ -292,19 +243,10 @@ class phpbb_extension_metadata_manager
 	 */
 	public function validate_enable()
 	{
-		$validate = array(
-			'require_phpbb',
-			'require_php',
-		);
-
-		foreach ($validate as $type)
+		// Check for phpBB, PHP versions
+		if (!$this->validate_require_phpbb || !$this->validate_require_php)
 		{
-			$type = 'validate_' . $type;
-
-			if (!$this->$type())
-			{
-				return false;
-			}
+			return false;
 		}
 
 		return true;
@@ -372,10 +314,10 @@ class phpbb_extension_metadata_manager
 		$this->template->assign_vars(array(
 			'MD_NAME'			=> htmlspecialchars($this->metadata['name']),
 			'MD_TYPE'			=> htmlspecialchars($this->metadata['type']),
-			'MD_DESCRIPTION'	=> htmlspecialchars($this->metadata['description']),
+			'MD_DESCRIPTION'	=> (isset($this->metadata['description'])) ? htmlspecialchars($this->metadata['description']) : '',
 			'MD_HOMEPAGE'		=> (isset($this->metadata['homepage'])) ? $this->metadata['homepage'] : '',
-			'MD_VERSION'		=> htmlspecialchars($this->metadata['version']),
-			'MD_TIME'			=> htmlspecialchars($this->metadata['time']),
+			'MD_VERSION'		=> (isset($this->metadata['version'])) ? htmlspecialchars($this->metadata['version']) : '',
+			'MD_TIME'			=> (isset($this->metadata['time'])) ? htmlspecialchars($this->metadata['time']) : '',
 			'MD_LICENCE'		=> htmlspecialchars($this->metadata['licence']),
 			'MD_REQUIRE_PHP'	=> (isset($this->metadata['require']['php'])) ? htmlspecialchars($this->metadata['require']['php']) : '',
 			'MD_REQUIRE_PHPBB'	=> (isset($this->metadata['require']['phpbb'])) ? htmlspecialchars($this->metadata['require']['phpbb']) : '',
@@ -386,7 +328,7 @@ class phpbb_extension_metadata_manager
 		{
 			$this->template->assign_block_vars('md_authors', array(
 				'AUTHOR_NAME'		=> htmlspecialchars($author['name']),
-				'AUTHOR_EMAIL'		=> $author['email'],
+				'AUTHOR_EMAIL'		=> (isset($author['email'])) ? $author['email'] : '',
 				'AUTHOR_HOMEPAGE'	=> (isset($author['homepage'])) ? $author['homepage'] : '',
 				'AUTHOR_ROLE'		=> (isset($author['role'])) ? htmlspecialchars($author['role']) : '',
 			));
author	Nathan Guse <nathaniel.guse@gmail.com>	2012-07-23 18:22:35 -0500
committer	Unknown Bliss <m@michaelcullum.com>	2012-09-01 15:05:32 +0100
commit	106c105113886f9a9e603dbb11549c06049b255f (patch)
tree	d6f242600b03211e983f1879a28790384049703d
parent	2273ae2b34071160ff930ca8d49326b8dd308899 (diff)
download	forums-106c105113886f9a9e603dbb11549c06049b255f.tar forums-106c105113886f9a9e603dbb11549c06049b255f.tar.gz forums-106c105113886f9a9e603dbb11549c06049b255f.tar.bz2 forums-106c105113886f9a9e603dbb11549c06049b255f.tar.xz forums-106c105113886f9a9e603dbb11549c06049b255f.zip