XMLRPC server does not honor optional parameters

kratib - September 30, 2007 - 22:45
Project:Services
Version:5.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:marcingy
Status:patch (code needs review)
Description

Steps to reproduce:
* Create a view that returns some nodes
* Download Keith Devens' XML-RPC client code at http://keithdevens.com/software/xmlrpc and rename it to xmlrpc.php
* Invoke the views.getView method using above client:

include('xmlrpc.php');
define(XMLRPC_DEBUG,1);
$result = XMLRPC_request('localhost', '/drupal/services/xmlrpc', 'views.getView', array(XMLRPC_prepare('my_view')));
XMLRPC_debug_print();

* RESULT: The debug output displays an XMLRPC_request with faultCode = -32602 and faultString = Server error. Wrong number of method parameters.
* Edit the XMLRPC_request line to send two additional empty arrays:
$result = XMLRPC_request('localhost', '/tawasol/services/xmlrpc', 'views.getView', array(XMLRPC_prepare('persons'),XMLRPC_prepare(array()),XMLRPC_prepare(array())));

* RESULT: The debug output displays the view result.

Does that mean that optional parameters are not honored?

#1

kratib - September 30, 2007 - 22:47

Note that the other params of the XMLRPC_request should be the same, contrary to what's written in the second code snippet.

#2

marcingy - March 8, 2008 - 00:18
Assigned to:Anonymous» marcingy

#3

snelson - April 12, 2008 - 05:33
Status:active» active (needs more info)

Marc,

Can you post some info here about your knowledge of this issue. From what I recall, its a Drupal core isssue?

Thanks,
Scott

#4

heydere - May 2, 2008 - 02:21

subscribe.

#5

heydere - May 2, 2008 - 21:26

I just did a little more research. The number of parameters is checked in the xmlrpc_server_call function inside xmlrpcs.inc in the core includes directory:

function xmlrpc_server_call($xmlrpc_server, $methodname, $args) {
  // Make sure parameters are in an array
  if ($args && !is_array($args)) {
    $args = array($args);
  }
  // Has this method been mapped to a Drupal function by us or by modules?
  if (!isset($xmlrpc_server->callbacks[$methodname])) {
    return xmlrpc_error(-32601, t('Server error. Requested method %methodname not specified.', array("%methodname" => $xmlrpc_server->message->methodname)));
  }
  $method = $xmlrpc_server->callbacks[$methodname];
  $signature = $xmlrpc_server->signatures[$methodname];

  // If the method has a signature, validate the request against the signature
  if (is_array($signature)) {
    $ok = TRUE;
    $return_type = array_shift($signature);
    // Check the number of arguments
    if (count($args) != count($signature)) {
      return xmlrpc_error(-32602, t('Server error. Wrong number of method parameters.'));
    }

This is where the error occurs. I'm not yet familiar enough with this code to see whether it can be changed easily. It looks like the method signature as recorded by Drupal would have to have a way of noting that some parameters are optional.

#6

marcingy - May 2, 2008 - 23:07

There is also another issue in that type checking fails (line 166 same module) so it is 2 part issue. A patch would have to be submitted to drupal core to over come this limitation.

We can simply check that the parameters passed in are less than or equal to the signature. The services module will then validate if we missing any parameters that are not optional.

#7

marcingy - May 2, 2008 - 23:46
Status:active (needs more info)» patch (code needs review)

Initial patch.

AttachmentSize
xmlrpc.patch.txt852 bytes

#8

heydere - May 6, 2008 - 20:24

I tested this patch and it works for me. On the surface, it seems like there could be implications regarding security/stability regarding this, however, if the XMLRPC interface is used without the services module to fall back on. People could be calling functions with fewer arguments than needed, which could cause unwanted behavior?

#9

marcingy - May 7, 2008 - 03:33

I agree and it maybe that patch only gets committed if/when services gets into the core. At which point in time xmlrpc.php will be removed from the root folder and all xmlrpc traffic will be handled by services.

#10

sethcohn - June 25, 2008 - 14:41

subscribe

#11

shrop - September 3, 2008 - 01:57

subscribe

#12

JacobSingh - September 3, 2008 - 12:40

Wow.. I think this one is pretty bad, perhaps critical. at least the docs should get updated to let developers know they aren't crazy.

How can I help to fix this? If it is a core issue, is there an issue for it in the drupal issue queue?

Best,
Jacob

#13

marcingy - September 3, 2008 - 15:55

This patch is for core however it only applies if services is installed because we do the optional checking and validation of the signature.

 
 

Drupal is a registered trademark of Dries Buytaert.