From: gggeek Date: Mon, 16 Jan 2023 09:11:03 +0000 (+0000) Subject: revert usage of fault code 4 in xmlparser; code comments X-Git-Tag: 4.10.0~158 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=4388aab7fdce62b6a96f1c738e02f0b833746722;p=plcapi.git revert usage of fault code 4 in xmlparser; code comments --- diff --git a/src/Helper/XMLParser.php b/src/Helper/XMLParser.php index b6f3c041..c348867d 100644 --- a/src/Helper/XMLParser.php +++ b/src/Helper/XMLParser.php @@ -42,8 +42,7 @@ class XMLParser * lv - used to indicate "looking for a value": implements the logic to allow values with no types to be strings * (values: 0=not looking, 1=looking, 3=found) * public: - * isf - used to indicate an xml-rpc response fault (1), invalid xml-rpc fault (2), xml parsing fault (3) or - * bad parameters passed to the parsing call (4) + * isf - used to indicate an xml-rpc response fault (1), invalid xml-rpc fault (2), xml parsing fault (3) * isf_reason - used for storing xml-rpc response fault string * value - used to store the value in responses * method - used to store method name in requests @@ -133,6 +132,7 @@ class XMLParser * @param array $options integer-key options are passed to the xml parser, in addition to the options received in * the constructor. String-key options are used independently * @return void + * @throws \Exception this can happen if a callback function is set and it does throw (ie. we do not catch exceptions) */ public function parse($data, $returnType = self::RETURN_XMLRPCVALS, $accept = 3, $options = array()) { @@ -167,9 +167,10 @@ class XMLParser switch($key) { case 'methodname_callback': if (!is_callable($val)) { - $this->_xh['isf'] = 4; - $this->_xh['isf_reason'] = "Callback passed as 'methodname_callback' is not callable"; - return; + //$this->_xh['isf'] = 4; + //$this->_xh['isf_reason'] = "Callback passed as 'methodname_callback' is not callable"; + //return; + $this->getLogger()->errorLog('XML-RPC: ' . __METHOD__ . ": Callback passed as 'methodname_callback' is not callable"); } else { $this->callbacks['methodname'] = $val; } @@ -447,6 +448,7 @@ class XMLParser * @param string $name * @param int $rebuildXmlrpcvals >1 for rebuilding xmlrpcvals, 0 for rebuilding php values, -1 for xmlrpc-extension compatibility * @return void + * @throws \Exception this can happen if a callback function is set and it does throw (ie. we do not catch exceptions) */ public function xmlrpc_ee($parser, $name, $rebuildXmlrpcvals = 1) { diff --git a/src/Request.php b/src/Request.php index b626e6c5..37cba2be 100644 --- a/src/Request.php +++ b/src/Request.php @@ -284,7 +284,7 @@ class Request // be tolerant of extra whitespace in response body $data = trim($data); - /// @todo return an error msg if $data == '' ? + /// @todo optimization creep - return an error msg if $data == '' // be tolerant of junk after methodResponse (e.g. javascript ads automatically inserted by free hosts) // idea from Luca Mariano originally in PEARified version of the lib @@ -349,10 +349,14 @@ class Request $xmlRpcParser->parse($data, $returnType, XMLParser::ACCEPT_RESPONSE, $options); // first error check: xml not well-formed - if ($xmlRpcParser->_xh['isf'] > 2) { + if ($xmlRpcParser->_xh['isf'] == 3) { // BC break: in the past for some cases we used the error message: 'XML error at line 1, check URL' + // Q: should we give back an error with variable error number, as we do server-side? But if we do, will + // we be able to tell apart the two cases? In theory, we never emit invalid xml on our end, but + // there could be proxies meddling with the request, or network data corruption... + $r = new Response(0, PhpXmlRpc::$xmlrpcerr['invalid_xml'], PhpXmlRpc::$xmlrpcstr['invalid_xml'] . ' ' . $xmlRpcParser->_xh['isf_reason'], '', $this->httpResponse @@ -375,7 +379,7 @@ class Request } // third error check: parsing of the response has somehow gone boink. /// @todo shall we omit this check, since we trust the parsing code? - elseif ($returnType == XMLParser::RETURN_XMLRPCVALS && !is_object($xmlRpcParser->_xh['value'])) { + elseif ($xmlRpcParser->_xh['isf'] > 3 || $returnType == XMLParser::RETURN_XMLRPCVALS && !is_object($xmlRpcParser->_xh['value'])) { // something odd has happened and it's time to generate a client side error indicating something odd went on $r = new Response(0, PhpXmlRpc::$xmlrpcerr['xml_parsing_error'], PhpXmlRpc::$xmlrpcstr['xml_parsing_error'], '', $this->httpResponse diff --git a/src/Server.php b/src/Server.php index 20bf0a93..c69d3fe2 100644 --- a/src/Server.php +++ b/src/Server.php @@ -625,8 +625,7 @@ class Server return new Response(0, $e->getCode(), $e->getMessage()); } -/// @todo handle the (unlikely) case of _xh['isf'] = 4 - if ($xmlRpcParser->_xh['isf'] > 2) { + if ($xmlRpcParser->_xh['isf'] == 2) { // (BC) we return XML error as a faultCode preg_match('/^XML error ([0-9]+)/', $xmlRpcParser->_xh['isf_reason'], $matches); return new Response( @@ -634,6 +633,8 @@ class Server PhpXmlRpc::$xmlrpcerrxml + (int)$matches[1], $xmlRpcParser->_xh['isf_reason']); } elseif ($xmlRpcParser->_xh['isf']) { + /// @todo separate better the various cases, as we have done in Request::parseResponse: invalid xml-rpc, + /// parsing error return new Response( 0, PhpXmlRpc::$xmlrpcerr['invalid_request'], @@ -653,9 +654,8 @@ class Server return $this->execute($xmlRpcParser->_xh['method'], $xmlRpcParser->_xh['params'], $xmlRpcParser->_xh['pt']); } else { - // build a Request object with data parsed from xml + // build a Request object with data parsed from xml and add parameters in $req = new Request($xmlRpcParser->_xh['method']); - // now add parameters in for ($i = 0; $i < count($xmlRpcParser->_xh['params']); $i++) { $req->addParam($xmlRpcParser->_xh['params'][$i]); }