revert usage of fault code 4 in xmlparser; code comments

diff --git a/src/Helper/XMLParser.php b/src/Helper/XMLParser.php
index b6f3c04..c348867 100644 (file)
--- 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 b626e6c..37cba2b 100644 (file)
--- 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 <luca.mariano@email.it> 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 20bf0a9..c69d3fe 100644 (file)
--- 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]);
                 }