From 9360256937dc907dcd9eb6b79f99bda4d4804fb2 Mon Sep 17 00:00:00 2001 From: Jerome Mouneyrac Date: Fri, 7 Oct 2011 16:06:19 +0800 Subject: [PATCH] MDL-29277 when validate externallib function parameters or return values, build the path to the error into the debuginfo. This patch also revert the exception to the original invalid_parameter_exception and invalid_response_exception --- lang/en/debug.php | 4 +-- lang/en/webservice.php | 9 ------ lib/externallib.php | 59 +++++++++++++++++++++++------------- lib/setuplib.php | 6 ++-- webservice/soap/locallib.php | 2 +- 5 files changed, 44 insertions(+), 36 deletions(-) diff --git a/lang/en/debug.php b/lang/en/debug.php index f951261939d..a7e25638dce 100644 --- a/lang/en/debug.php +++ b/lang/en/debug.php @@ -35,8 +35,8 @@ $string['configmoodle'] = 'Moodle has not been configured yet. You need to edit $string['erroroccur'] = 'An error has occurred during this process'; $string['invalidarraysize'] = 'Incorrect size of arrays in params of {$a}'; $string['invalideventdata'] = 'Incorrect eventadata submitted: {$a}'; -$string['invalidparameter'] = 'Invalid parameter value detected, execution can not continue.'; -$string['invalidresponse'] = 'Invalid response value detected, execution can not continue.'; +$string['invalidparameter'] = 'Invalid parameter value detected'; +$string['invalidresponse'] = 'Invalid response value detected'; $string['missingconfigversion'] = 'Config table does not contain version, can not continue, sorry.'; $string['modulenotexist'] = '{$a} module doesn\'t exist'; $string['morethanonerecordinfetch'] = 'Found more than one record in fetch() !'; diff --git a/lang/en/webservice.php b/lang/en/webservice.php index d4f16bd2d4f..075ca231585 100644 --- a/lang/en/webservice.php +++ b/lang/en/webservice.php @@ -79,17 +79,8 @@ $string['errorcatcontextnotvalid'] = 'You cannot execute functions in the catego $string['errorcodes'] = 'Error message'; $string['errorcoursecontextnotvalid'] = 'You cannot execute functions in the course context (course id:{$a->courseid}). The context error message was: {$a->message}'; $string['errorinvalidparam'] = 'The param "{$a}" is invalid.'; -$string['errorinvalidparamsapi'] = 'Invalid external api parameter'; -$string['errorinvalidparamsdesc'] = 'Invalid external api description'; -$string['errorinvalidresponseapi'] = 'Invalid external api response'; -$string['errorinvalidresponsedesc'] = 'Invalid external api response description'; -$string['errormissingkey'] = 'Missing required key in single structure: {$a}'; $string['errornotemptydefaultparamarray'] = 'The web service description parameter named \'{$a}\' is an single or multiple structure. The default can only be empty array. Check web service description.'; -$string['erroronlyarray'] = 'Only arrays accepted.'; $string['erroroptionalparamarray'] = 'The web service description parameter named \'{$a}\' is an single or multiple structure. It can not be set as VALUE_OPTIONAL. Check web service description.'; -$string['errorresponsemissingkey'] = 'Error in response - Missing following required key in a single structure: {$a}'; -$string['errorscalartype'] = 'Scalar type expected, array or object received.'; -$string['errorunexpectedkey'] = 'Unexpected keys ({$a}) detected in parameter array.'; $string['execute'] = 'Execute'; $string['executewarnign'] = 'WARNING: If you press execute your database will be modified and changes can not be reverted automatically!'; $string['externalservice'] = 'External service'; diff --git a/lib/externallib.php b/lib/externallib.php index 0efc009655c..eb6a996de4b 100644 --- a/lib/externallib.php +++ b/lib/externallib.php @@ -149,7 +149,7 @@ class external_api { public static function validate_parameters(external_description $description, $params) { if ($description instanceof external_value) { if (is_array($params) or is_object($params)) { - throw new invalid_parameter_exception(get_string('errorscalartype', 'webservice')); + throw new invalid_parameter_exception('Scalar type expected, array or object received.'); } if ($description->type == PARAM_BOOL) { @@ -158,31 +158,37 @@ class external_api { return (bool)$params; } } - return validate_param($params, $description->type, $description->allownull, get_string('errorinvalidparamsapi', 'webservice')); + $debuginfo = 'Invalid external api parameter: the value is "' . $params . + '", the server was expecting "' . $description->type . '" type'; + return validate_param($params, $description->type, $description->allownull, $debuginfo); } else if ($description instanceof external_single_structure) { if (!is_array($params)) { - throw new invalid_parameter_exception(get_string('erroronlyarray', 'webservice')); + throw new invalid_parameter_exception('Only arrays accepted. The bad value is: \'' + . print_r($params, true) . '\''); } $result = array(); foreach ($description->keys as $key=>$subdesc) { if (!array_key_exists($key, $params)) { if ($subdesc->required == VALUE_REQUIRED) { - throw new invalid_parameter_exception(get_string('errormissingkey', 'webservice', $key)); + throw new invalid_parameter_exception('Missing required key in single structure: '. $key); } if ($subdesc->required == VALUE_DEFAULT) { try { $result[$key] = self::validate_parameters($subdesc, $subdesc->default); } catch (invalid_parameter_exception $e) { - throw new webservice_parameter_exception('invalidextparam',$key); + //we are only interested by exceptions returned by validate_param() and validate_parameters() + //(in order to build the path to the faulty attribut) + throw new invalid_parameter_exception($key." => ".$e->getMessage() . ': ' .$e->debuginfo); } } } else { try { $result[$key] = self::validate_parameters($subdesc, $params[$key]); } catch (invalid_parameter_exception $e) { - //it's ok to display debug info as here the information is useful for ws client/dev - throw new webservice_parameter_exception('invalidextparam',$key." (".$e->debuginfo.")"); + //we are only interested by exceptions returned by validate_param() and validate_parameters() + //(in order to build the path to the faulty attribut) + throw new invalid_parameter_exception($key." => ".$e->getMessage() . ': ' .$e->debuginfo); } } unset($params[$key]); @@ -193,13 +199,14 @@ class external_api { foreach($params as $key => $value) { $keys .= $key . ','; } - throw new invalid_parameter_exception(get_string('errorunexpectedkey', 'webservice', $keys)); + throw new invalid_parameter_exception('Unexpected keys (' . $keys . ') detected in parameter array.'); } return $result; } else if ($description instanceof external_multiple_structure) { if (!is_array($params)) { - throw new invalid_parameter_exception(get_string('erroronlyarray', 'webservice')); + throw new invalid_parameter_exception('Only arrays accepted. The bad value is: \'' + . print_r($params, true) . '\''); } $result = array(); foreach ($params as $param) { @@ -208,7 +215,7 @@ class external_api { return $result; } else { - throw new invalid_parameter_exception(get_string('errorinvalidparamsdesc', 'webservice')); + throw new invalid_parameter_exception('Invalid external api description'); } } @@ -225,7 +232,7 @@ class external_api { public static function clean_returnvalue(external_description $description, $response) { if ($description instanceof external_value) { if (is_array($response) or is_object($response)) { - throw new invalid_response_exception(get_string('errorscalartype', 'webservice')); + throw new invalid_response_exception('Scalar type expected, array or object received.'); } if ($description->type == PARAM_BOOL) { @@ -234,33 +241,42 @@ class external_api { return (bool)$response; } } - return validate_param($response, $description->type, $description->allownull, get_string('errorinvalidresponseapi', 'webservice')); + $debuginfo = 'Invalid external api response: the value is "' . $response . + '", the server was expecting "' . $description->type . '" type'; + try { + return validate_param($response, $description->type, $description->allownull, $debuginfo); + } catch (invalid_parameter_exception $e) { + //proper exception name, to be recursively catched to build the path to the faulty attribut + throw new invalid_response_exception($e->debuginfo); + } } else if ($description instanceof external_single_structure) { if (!is_array($response)) { - throw new invalid_response_exception(get_string('erroronlyarray', 'webservice')); + throw new invalid_response_exception('Only arrays accepted. The bad value is: \'' . + print_r($response, true) . '\''); } $result = array(); foreach ($description->keys as $key=>$subdesc) { if (!array_key_exists($key, $response)) { if ($subdesc->required == VALUE_REQUIRED) { - throw new webservice_parameter_exception('errorresponsemissingkey', $key); + throw new invalid_response_exception('Error in response - Missing following required key in a single structure: ' . $key); } if ($subdesc instanceof external_value) { if ($subdesc->required == VALUE_DEFAULT) { try { $result[$key] = self::clean_returnvalue($subdesc, $subdesc->default); - } catch (Exception $e) { - throw new webservice_parameter_exception('invalidextresponse',$key." (".$e->debuginfo.")"); + } catch (invalid_response_exception $e) { + //build the path to the faulty attribut + throw new invalid_response_exception($key." => ".$e->getMessage() . ': ' . $e->debuginfo); } } } } else { try { $result[$key] = self::clean_returnvalue($subdesc, $response[$key]); - } catch (Exception $e) { - //it's ok to display debug info as here the information is useful for ws client/dev - throw new webservice_parameter_exception('invalidextresponse',$key." (".$e->debuginfo.")"); + } catch (invalid_response_exception $e) { + //build the path to the faulty attribut + throw new invalid_response_exception($key." => ".$e->getMessage() . ': ' . $e->debuginfo); } } unset($response[$key]); @@ -270,7 +286,8 @@ class external_api { } else if ($description instanceof external_multiple_structure) { if (!is_array($response)) { - throw new invalid_response_exception(get_string('erroronlyarray', 'webservice')); + throw new invalid_response_exception('Only arrays accepted. The bad value is: \'' . + print_r($response, true) . '\''); } $result = array(); foreach ($response as $param) { @@ -279,7 +296,7 @@ class external_api { return $result; } else { - throw new invalid_response_exception(get_string('errorinvalidresponsedesc', 'webservice')); + throw new invalid_response_exception('Invalid external api response description'); } } diff --git a/lib/setuplib.php b/lib/setuplib.php index f45221b8b54..1abf90b6bd9 100644 --- a/lib/setuplib.php +++ b/lib/setuplib.php @@ -139,7 +139,7 @@ class require_login_exception extends moodle_exception { /** * Web service parameter exception class - * + * @deprecated - use moodle exception instead * This exception must be thrown to the web service client when a web service parameter is invalid * The error string is gotten from webservice.php */ @@ -149,8 +149,8 @@ class webservice_parameter_exception extends moodle_exception { * @param string $errorcode The name of the string from webservice.php to print * @param string $a The name of the parameter */ - function __construct($errorcode=null, $a = '') { - parent::__construct($errorcode, 'webservice', '', $a, null); + function __construct($errorcode=null, $a = '', $debuginfo = null) { + parent::__construct($errorcode, 'webservice', '', $a, $debuginfo); } } diff --git a/webservice/soap/locallib.php b/webservice/soap/locallib.php index 426b05ad20e..a9437a8cb8a 100644 --- a/webservice/soap/locallib.php +++ b/webservice/soap/locallib.php @@ -76,7 +76,7 @@ class webservice_soap_server extends webservice_zend_server { $this->zend_server->setReturnResponse(true); //TODO: the error handling in Zend Soap server is useless, XML-RPC is much, much better :-( $this->zend_server->registerFaultException('moodle_exception'); - $this->zend_server->registerFaultException('webservice_parameter_exception'); + $this->zend_server->registerFaultException('webservice_parameter_exception'); //deprecated - kept for backward compatibility $this->zend_server->registerFaultException('invalid_parameter_exception'); $this->zend_server->registerFaultException('invalid_response_exception'); }