XML-RPC on fault return puts <em> tags into <string> resulting in loss of information

snufkin - March 7, 2008 - 12:19
Project:Drupal
Version:5.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

xmlrpcs.inc uses t() with placeholders when reporting error. The placeholde will be surrounded with <em> tags, and when the message parsing is done in xmlrpc.inc in xmlrpc_message_parse() parts of the message will be cropped.

Example: when requesting an invalid method the result is the following XML:

<methodResponse>
  <fault>
  <value>
    <struct>
    <member>
      <name>faultCode</name>
      <value><int>-32601</int></value>
    </member>
    <member>
      <name>faultString</name>
      <value><string>Server error. Requested method <em>node.loader</em> not specified.</string></value>
    </member>
    </struct>
  </value>
  </fault>
</methodResponse>

From which the parsed error will be:

[params] => Array
        (
            [0] => Array
                (
                    [faultCode] => -32601
                    [faultString] =>  not specified.
                )

        )

You can see that the fault string is only part of the original message, moreover it contains a leading white space, which suggests that there was a mistake when the original XML tags were removed. I suggest we remove the extra formatting tags from the faultString, so the parsed message can be retreived.

This bug is also there in Drupal 6, where the same faulty behaviour is expected, but I did not test it.

If you want to test the bug there is an xmlrpc tester module in my sandbox. Patch replaces the placeholders with the actual variables.

AttachmentSizeStatusTest resultOperations
xmlrpcs_tags.patch2.36 KBIgnoredNoneNone
xmlrpcs_tags.patch2.36 KBIgnoredNoneNone

#1

Jorrit - March 30, 2008 - 11:56

I think it is better to just replace the % by the @. At least for D6, it will simply not add the <em> tag. Together with that, perhaps it is a wise consideration to force XML error responses to English regardless of the system default language.

#2

snufkin - March 30, 2008 - 14:01

I am not so sure. You might want to have the error message localized.

I didn't know about the difference between % and @, thanks for that, i will investigate and if it works in 5 too, i will update the patch.

#3

keith.smith - March 30, 2008 - 18:38
Status:needs review» needs work

There are some minor code style violations here (spacing around concatenation operators).

#4

snufkin - March 31, 2008 - 11:36
Status:needs work» needs review

and indeed the @ sign works even in 5.7. cool, every day a new trick. New patch, I simply replaced the substituting % operators to @, thus no <em> tag is in the XML. I will port the patch to 6 as soon as this is fixed.

AttachmentSizeStatusTest resultOperations
xmlrpcs_tags_1.patch2.47 KBIgnoredNoneNone

#5

drumm - April 28, 2008 - 08:19
Version:5.7» 7.x-dev
Status:needs review» needs work

Replacing % with @ is the correct approach. However, I would like to see this fixed in the development version, currently 7.x, first, and then backported from there.

#6

Dries - April 28, 2008 - 10:06
Version:7.x-dev» 5.x-dev
Status:needs work» reviewed & tested by the community

The patch didn't apply cleanly against either DRUPAL-6 or CVS HEAD, but I've fixed it up, and committed it to both branches. The patch should go into DRUPAL-5 now. I leave that up to Neil. Lowering the version and updating the status field.

#7

Jorrit - April 29, 2008 - 13:29

Now I wonder what the procedure is for Drupal 5 bugs? There are still some, for instance http://drupal.org/node/172830. Do they need a patch for Drupal 7 first? What if the issue is not present in Drupal 7? Does this apply for Drupal 6 bugs as well?

#8

drumm - May 10, 2008 - 02:02
Status:reviewed & tested by the community» fixed

Committed to 5.x.

Jorrit- Ideally development is done against HEAD and the backported. There are of course cases where bugs do not exist in more recent versions.

#9

Anonymous (not verified) - May 24, 2008 - 02:13
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.