After I submitted bug #4231, a patch different than my suggestion was implemented for 1.3.0RC2. This did indeed correct the original problem of bug #4231 (bad parameter counts causing PHP notices). However, there was a very specific reason that I submitted the fix as I did and not as was done in the release. The fix in 1.3.0RC2 creates a new problem (and a BC break) that my suggested fix did not.
It may not be immediately obvious but the dispatch map as it has been implemented in
XML_RPC_Server
allows for a single RPC method to have multiple signatures. This implements support for both polymorphic parameter types and
also different parameter counts
(i.e., optional parameters). The fix for bug #4231 as it was implemented in 1.3.0RC2 breaks support for the signatures of a method to have different
counts of parameters.
The official XML-RPC specification makes no mention of optional parameters in method calls, however all clients inherently support them. Using them where appropriate has the advantage of lightening XML-RPC's already heavy request playload by alleviating the need to send some unnecessary parameters. The decision of whether or not to support optional parameters is left entirely to the RPC API designer. I feel that we should continue to support it for these reasons, and also because existing XML_RPC package users may already be using this feature (at least, I do).
To demonstrate the use of optional parameters and how 1.3.0RC2 breaks this feature, we must first create a server based on XML_RPC_Server
with an RPC method that has an optional parameter. To implement this, we'll need to create two signatures for the method: one with the optional
parameter supplied, and one without.
<?php require_once('XML/RPC/Server.php'); // dispatch map for functions mapped to RPC method calls $dispMap = array(); $dispMap['getParamCount'] = array('function' => 'rpc_getParamCount', 'signature' => array( array('int', 'string', 'string'), array('int', 'string', 'string', 'boolean')), 'docstring' => 'Call me with 2 or 3 parameters of the correct types.'); // service XML-RPC request now $server = new XML_RPC_Server($dispMap, 1, 1); /** * A function that demonstrates the creation of XML-RPC methods that accept multiple parameter * counts (optional parameters). This method can be called with either two or three parameters. * Other parameter counts or type mismatches will be faulted by XML_RPC_Server::verifySignature() * before this function is called. * * XML-RPC Signatures: int getParamCount(string, string) * int getParamCount(string, string, boolean) * * @param XML_RPC_Message $params * @return XML_RPC_Response +XML_RPC_Value int */ function rpc_getParamCount($params) { if ($params->getNumParams()==3) { // process all three parameters (optional third parameter was supplied) } else { // process only the two required parameters } // return the count of parameters received, if you want to explore further on your own. $val = new XML_RPC_Value($params->getNumParams(), 'int'); return new XML_RPC_Response($val); } ?>
Next, we'll create a stripped-down client with XML_RPC_Client
to call this method with and without its optional parameter,
and test whether or not the method call returned a fault.
If optional parameters are correctly supported by the version of XML_RPC_Server
used, the server should not report any
faults to the client.
<?php require_once('XML/RPC.php'); $cli = new XML_RPC_Client('/demo-server.php', 'localhost'); $p1 = new XML_RPC_Value('', 'string'); $p2 = new XML_RPC_Value('', 'string'); $p3 = new XML_RPC_Value(false, 'boolean'); /** * Call getParamCount() with only its two required parameters. */ $msg = new XML_RPC_Message('getParamCount', array($p1, $p2)); $resp = $cli->send($msg); if (!$resp) { die('Communication error: ' . $cli->errstr); } if ($resp->faultCode() ) { echo("Calling the method with its two required parameters succeeded.\n"); } else { echo("Calling the method with its two required parameters produced this fault:\n"); echo(' Fault Code: ' . $resp->faultCode() . "\n"); echo(' Fault Reason: ' . $resp->faultString() . "\n"); } /** * Call getParamCount() with three parameters. */ $msg = new XML_RPC_Message('getParamCount', array($p1, $p2, $p3)); $resp = $cli->send($msg); if (!$resp) { die('Communication error: ' . $cli->errstr); } if ($resp->faultCode() ) { echo("Calling the same method with three parameters succeeded.\n"); } else { echo("Calling the same method with three parameters produced this fault:\n"); echo(' Fault Code: ' . $resp->faultCode() . "\n"); echo(' Fault Reason: ' . $resp->faultString() . "\n"); } ?>
Content-type: text/html X-Powered-By: PHP/5.0.3 Calling the method with its two required parameters succeeded. Calling the same method with three parameters succeeded.
Both calls to the RPC method getParamCount()
succeed and do not return a fault code.
This shows that XML_RPC_Server::verifySignature()
in versions prior to 1.3.0RC2 can properly handle a dispatch map
where a method can have signatures that contain different counts of parameters. Of course, these versions of XML_RPC_Server
also have the
original bug reported in bug #4231.
The true expected result would be a version of XML_RPC_Server
that both produces the output shown here and also addresses
bug #4231. The fix that I originally submitted for bug #4231 meets these conditions.
The actual result when this same code is run on 1.3.0RC2 is that the second method call fails because verifySignature()
does
not properly evaluate all signatures in the dispatch map for the method.
Content-type: text/html X-Powered-By: PHP/5.0.3 Calling the method with its two required parameters succeeded. Calling the same method with three parameters produced this fault: Fault Code: 3 Fault Reason: Incorrect parameters passed to method: Improper call to verifySignature()
To understand how the patch implemented in 1.3.0RC2 broke support for optional parameters in XML_RPC_Server
, we need to look at its version
of the function verifySignature()
:
/** * @return array */ function verifySignature($in, $sig) { for ($i = 0; $i < sizeof($sig); $i++) { // check each possible signature in turn $cursig = $sig[$i]; if (sizeof($cursig) == $in->getNumParams() + 1) { $itsOK = 1; for ($n = 0; $n < $in->getNumParams(); $n++) { $p = $in->getParam($n); // print "\n"; if ($p->kindOf() == 'scalar') { $pt = $p->scalartyp(); } else { $pt = $p->kindOf(); } // $n+1 as first type of sig is return type if ($pt != $cursig[$n+1]) { $itsOK = 0; $pno = $n+1; $wanted = $cursig[$n+1]; $got = $pt; break; } } if ($itsOK) { return array(1); } } else { return array(0, 'Improper call to verifySignature()'); } } return array(0, "Wanted ${wanted}, got ${got} at param ${pno})"); }
In the code from 1.3.0RC2 above, the outermost for
loop iterates through all of the signatures for a single method in the dispatch map. On
each iteration, the outermost if
statement checks that the count of parameters sent by the client matches the number in that particular signature.
The patch for 1.3.0RC2 added the else
statement to this conditional. The result is that even if the dispatch map contains a signature for the
method which matches the count of parameters that the client sent, the else
statement will cause the function to immediately return
upon
encountering a signature with a different count of parameters in any earlier iteration through the signatures.
To fix the bug, and have a version of verifySignature()
that both addresses bug #4231 and supports optional
parameters, use the function verifySignature()
from 1.3.0RC1 and apply the patch that I suggested in bug #4231. This results
in the following working code:
/** * @return array */ function verifySignature($in, $sig) { for ($i = 0; $i < sizeof($sig); $i++) { // check each possible signature in turn $cursig = $sig[$i]; if (sizeof($cursig) == $in->getNumParams() + 1) { $itsOK = 1; for ($n = 0; $n < $in->getNumParams(); $n++) { $p = $in->getParam($n); // print "\n"; if ($p->kindOf() == 'scalar') { $pt = $p->scalartyp(); } else { $pt = $p->kindOf(); } // $n+1 as first type of sig is return type if ($pt != $cursig[$n+1]) { $itsOK = 0; $pno = $n+1; $wanted = $cursig[$n+1]; $got = $pt; break; } } if ($itsOK) { return array(1); } } } if (isset($pno)) { return array(0, "Wanted ${wanted}, got ${got} at param ${pno})"); } else { return array(0, 'Bad parameter count'); } }
There are some alternatives to this, such as adding a separate loop before this one to iterate through the possible signatures
and returning an error if none of them matches the count of parameters supplied. Another alternative would be to replace the isset()
with an additional flag. These variations waste extra cycles and memory, so I chose not to implement the fix this way.
In closing, I'd like to make some additional comments on the text chosen for the error message in 1.3.0RC2. The error message I suggested
in bug #4231 was "Bad parameter count
", for the reason that the condition only occurred when the client supplied a parameter count that did
not match any of the method's signatures. The message implemented in 1.3.0RC2 was instead called "Improper call to verifySignature()
".
This error is returned to the
remote XML-RPC client. This is confusing to the programmer of a client who is examining responses from the server because
it does not explain how the client actually caused the error. The client didn't call verifySignature()
improperly,
it sent a bad count of parameters.
Additionally, one final concern with the chosen error message is that including our function name in an error message returned to the client
is a security issue because it provides a means for a client to fingerprint any server that is derived from XML_RPC_Server
. With any fix
that included this error message, an attacker
could make an XML-RPC method call with a bad parameter count, and check the returned faultString
for the text "verifySignature()
". This would
immediately lead to the discovery that the server is running on XML_RPC_Server
, and PHP itself, which could have otherwise been concealed. Although it's true that the
client could test any of our error messages in this way, none of the other messages are so specific as to name our internal functions. It wouldn't
be good practice to display an error message like this on a webpage, and I feel the same applies for a remote XML-RPC client.
I recommend returning the error as "Bad parameter count
" as it is the most descriptive and perhaps has fewer security implications.
Last page update: 6 May 2005.