From 106c105113886f9a9e603dbb11549c06049b255f Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Mon, 23 Jul 2012 18:22:35 -0500 Subject: [PATCH] [ticket/10631] Fix some issues as noted in github comments, significantly simplified validation PHPBB3-10631 --- phpBB/adm/style/acp_ext_details.html | 64 ++++---- phpBB/includes/acp/acp_extensions.php | 12 +- phpBB/includes/extension/manager.php | 2 +- phpBB/includes/extension/metadata_manager.php | 146 ++++++------------ 4 files changed, 83 insertions(+), 141 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 @@
{MD_NAME}
-
-
-

{MD_TYPE}

-
+

{MD_DESCRIPTION}

+

{MD_VERSION}

+

{MD_HOMEPAGE}

- + +

{MD_TIME}

+

{MD_LICENCE}

+
{L_REQUIREMENTS} @@ -61,34 +60,35 @@
+
{L_AUTHOR_INFORMATION} -
-
-
{md_authors.AUTHOR_NAME}
-
- -
-
-
{md_authors.AUTHOR_EMAIL}
-
- - -
-
-
{md_authors.AUTHOR_HOMEPAGE}
-
- - -
-
-
{md_authors.AUTHOR_ROLE}
-
- - -

+
+
+
+
{md_authors.AUTHOR_NAME}
+
+ +
+
+
{md_authors.AUTHOR_EMAIL}
+
+ + +
+
+
{md_authors.AUTHOR_HOMEPAGE}
+
+ + +
+
+
{md_authors.AUTHOR_ROLE}
+
+ +
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,62 +28,9 @@ 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(? 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 * @@ -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)) + * 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; + } + + // Validate all fields + if ($name == 'all') + { + foreach ($fields as $field => $data) { - foreach ($regex as $extra_name => $extra_regex) - { - $type = 'validate_' . $name . '_' . $extra_name; - - if (!$this->$type()) - { - return false; - } - } - } - else - { - - $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']) : '', ));