From c9c5cc81e9a2ef42f6c7b1413e6f3a224b65dec1 Mon Sep 17 00:00:00 2001 From: skodak Date: Fri, 2 Oct 2009 12:13:56 +0000 Subject: [PATCH] MDL-12886 external lib parameter validation routine + some unit tests --- group/externallib.php | 20 ++++----- lib/externallib.php | 65 ++++++++++++++++++++++++------ lib/simpletest/testexternallib.php | 57 ++++++++++++++++++++++++++ user/externallib.php | 4 +- 4 files changed, 118 insertions(+), 28 deletions(-) create mode 100644 lib/simpletest/testexternallib.php diff --git a/group/externallib.php b/group/externallib.php index 57ba8d90a4..118d3df5c8 100644 --- a/group/externallib.php +++ b/group/externallib.php @@ -45,7 +45,7 @@ class moodle_group_external extends external_api { global $CFG; require_once("$CFG->dirroot/group/lib.php"); - $params = self::validate_prameters(self::create_groups_parameters(), array('groups'=>$groups)); + $params = self::validate_parameters(self::create_groups_parameters(), array('groups'=>$groups)); $groups = array(); @@ -82,13 +82,7 @@ class moodle_group_external extends external_api { public static function get_groups_parameters() { return new external_function_parameters( array( - 'groups' => new external_multiple_structure( - new external_single_structure( - array( - 'groupid' => new external_param(PARAM_INT, 'Group ID') - ) - ) - ) + 'groups' => new external_multiple_structure(new external_param(PARAM_INT, 'Group ID')) ) ); } @@ -101,7 +95,7 @@ class moodle_group_external extends external_api { public static function get_groups($groupids) { $groups = array(); - $params = self::validate_prameters(self::get_groups_parameters(), array('groupids'=>$groupids)); + $params = self::validate_parameters(self::get_groups_parameters(), array('groupids'=>$groupids)); //TODO: we do need to search for groups in courses too, // fetching by id is not enough! @@ -147,7 +141,7 @@ class moodle_group_external extends external_api { global $CFG; require_once("$CFG->dirroot/group/lib.php"); - $params = self::validate_prameters(self::delete_groups_parameters(), array('groupids'=>$groupids)); + $params = self::validate_parameters(self::delete_groups_parameters(), array('groupids'=>$groupids)); $groups = array(); @@ -185,7 +179,7 @@ class moodle_group_external extends external_api { public static function get_groupmembers($groupids) { $groups = array(); - $params = self::validate_prameters(self::get_groupmembers_parameters(), array('groupids'=>$groupids)); + $params = self::validate_parameters(self::get_groupmembers_parameters(), array('groupids'=>$groupids)); foreach ($params['groupids'] as $groupid) { // validate params @@ -221,7 +215,7 @@ class moodle_group_external extends external_api { global $CFG; require_once("$CFG->dirroot/group/lib.php"); - $params = self::validate_prameters(self::add_groupmembers_parameters(), array('members'=>$members)); + $params = self::validate_parameters(self::add_groupmembers_parameters(), array('members'=>$members)); foreach ($params['members'] as $member) { // validate params @@ -259,7 +253,7 @@ class moodle_group_external extends external_api { global $CFG; require_once("$CFG->dirroot/group/lib.php"); - $params = self::validate_prameters(self::delete_groupmembers_parameters(), array($members=>'members')); + $params = self::validate_parameters(self::delete_groupmembers_parameters(), array($members=>'members')); foreach ($params['members'] as $member) { // validate params diff --git a/lib/externallib.php b/lib/externallib.php index 63f56b4df7..d2ec267610 100644 --- a/lib/externallib.php +++ b/lib/externallib.php @@ -53,16 +53,55 @@ class external_api { } /** - * Validates submitted function barameters, if anything is incorrect + * Validates submitted function parameters, if anything is incorrect * invalid_parameter_exception is thrown. - * @param ? $description description of parameters - * @param ? $params the actual parameters - * @return ? params with added defaults for optional items, invalid_parameters_exception thrown if any problem found + * @param external_description $description description of parameters + * @param mixed $params the actual parameters + * @return mixed params with added defaults for optional items, invalid_parameters_exception thrown if any problem found */ - public static function validate_prameters($description, $params) { - //TODO: we need to define the structure of param descriptions + public static function validate_parameters(external_description $description, $params) { + if ($description instanceof external_param) { + if (is_array($params) or is_object($params)) { + throw new invalid_parameter_exception('Scalar type expected, array or object received.'); + } + return validate_param($params, $description->type, $description->allownull, 'Invalid external api parameter'); + + } else if ($description instanceof external_single_structure) { + if (!is_array($params)) { + throw new invalid_parameter_exception('Only arrays accepted.'); + } + $result = array(); + foreach ($description->keys as $key=>$subdesc) { + if (!array_key_exists($key, $params)) { + if ($subdesc->required) { + throw new invalid_parameter_exception('Missing required key in single structure.'); + } + if ($subdesc instanceof external_param) { + $result[$key] = self::validate_parameters($subdesc, $subdesc->default); + } + } else { + $result[$key] = self::validate_parameters($subdesc, $params[$key]); + } + unset($params[$key]); + } + if (!empty($params)) { + throw new invalid_parameter_exception('Unexpected keys detected in parameter array.'); + } + return $result; - return $params; + } else if ($description instanceof external_multiple_structure) { + if (!is_array($params)) { + throw new invalid_parameter_exception('Only arrays accepted.'); + } + $result = array(); + foreach ($params as $param) { + $result[] = self::validate_parameters($description->content, $param); + } + return $result; + + } else { + throw new invalid_parameter_exception('Invalid external api description.'); + } } /** @@ -97,7 +136,7 @@ class external_api { // proper enrolment and course visibility check // similar to require_login() (which can not be used // because it can be used only once and redirects) - // oh - did I tell we need to rewrite enrolments in 2.0 + // oh - did I say we need to rewrite enrolments in 2.0 // to solve this bloody mess? // // missing: hidden courses and categories, groupmembersonly, @@ -121,7 +160,7 @@ abstract class external_description { * @param string $desc * @param bool $required */ - public function __contruct($desc, $required) { + public function __construct($desc, $required) { $this->desc = $desc; $this->required = $required; } @@ -146,8 +185,8 @@ class external_param extends external_description { * @param mixed $default * @param bool $allownull */ - public function __contruct($type, $desc='', $required=true, $default=null, $allownull=true) { - parent::_construct($desc, $required); + public function __construct($type, $desc='', $required=true, $default=null, $allownull=true) { + parent::__construct($desc, $required); $this->type = $type; $this->default = $default; $this->allownull = $allownull; @@ -168,7 +207,7 @@ class external_single_structure extends external_description { * @param bool $required */ public function __construct(array $keys, $desc='', $required=true) { - parent::_construct($desc, $required); + parent::__construct($desc, $required); $this->keys = $keys; } } @@ -187,7 +226,7 @@ class external_multiple_structure extends external_description { * @param bool $required */ public function __construct(external_description $content, $desc='', $required=true) { - parent::_construct($desc, $required); + parent::__construct($desc, $required); $this->content = $content; } } diff --git a/lib/simpletest/testexternallib.php b/lib/simpletest/testexternallib.php new file mode 100644 index 0000000000..fb98102822 --- /dev/null +++ b/lib/simpletest/testexternallib.php @@ -0,0 +1,57 @@ +. + + +/** + * Unit tests for /lib/externallib.php. + * + * @package webservices + * @copyright 2009 Pwetr Skoda + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +if (!defined('MOODLE_INTERNAL')) { + die('Direct access to this script is forbidden.'); /// It must be included from a Moodle page +} +require_once($CFG->libdir . '/externallib.php'); + +class externallib_test extends UnitTestCase { + public function test_validate_params1() { + $params = array('text'=>'aaa', 'someid'=>'6',); + $description = new external_function_parameters(array('someid' => new external_param(PARAM_INT, 'Some int value'), + 'text' => new external_param(PARAM_ALPHA, 'Some text value'))); + $result = external_api::validate_parameters($description, $params); + $this->assertEqual(count($result), 2); + reset($result); + $this->assertTrue(key($result) === 'someid'); + $this->assertTrue($result['someid'] === 6); + $this->assertTrue($result['text'] === 'aaa'); + + + $params = array('someids'=>array('1', 2, 'a'=>'3'), 'scalar'=>666); + $description = new external_function_parameters(array('someids' => new external_multiple_structure(new external_param(PARAM_INT, 'Some ID')), + 'scalar' => new external_param(PARAM_ALPHANUM, 'Some text value'))); + $result = external_api::validate_parameters($description, $params); + $this->assertEqual(count($result), 2); + reset($result); + $this->assertTrue(key($result) === 'someids'); + $this->assertTrue($result['someids'] == array(0=>1, 1=>2, 2=>3)); + $this->assertTrue($result['scalar'] === '666'); + } + + //TODO: add unittests for all description options and validation failures +} diff --git a/user/externallib.php b/user/externallib.php index 5d71155d2c..58ddf17f4f 100644 --- a/user/externallib.php +++ b/user/externallib.php @@ -87,7 +87,7 @@ class moodle_user_external extends external_api { // 2) All required items were sent // 3) All data passes clean_param without changes (yes this is strict) // If any problems are found then exceptions are thrown with helpful error messages - $params = self::validate_prameters(self::create_users_parameters(), $params); + $params = self::validate_parameters(self::create_users_parameters(), $params); // Perform further checks and build up a clean array of user data @@ -184,7 +184,7 @@ class moodle_user_external extends external_api { require_capability('moodle/user:viewdetails', $context); self::validate_context($context); - $params = self::validate_prameters(self::get_users_parameters(), $params); + $params = self::validate_parameters(self::get_users_parameters(), $params); //TODO: this search is probably useless for external systems because it is not exact // 1/ we should specify multiple search parameters including the mnet host id -- 2.39.5