Ticket #3399 (closed defect: fixed )

Opened 10 months ago

Last modified 10 months ago

twisted.web.xmlrpc._QueryFactory.parseResponse fails to errback for a particular malformed response

Reported by: toranin Assigned to: therve
Type: defect Priority: normal
Milestone: Component: web
Keywords: Cc: exarkun
Branch: branches/xmlrpc-empty-response-3399 Author: therve
Launchpad Bug:

Description

(applies to 8.1.0, maybe newer)

If the server returns XML that parses but contains no data, an IndexError is thrown from line 321, in the else part of the try/except block that is supposed to catch problems with the response structure.

I moved the indexing operation in question into the try block in the attached patch.

Attachments

twisted.web.xmlrpc-8.1.0-fix.patch (0.6 kB) - added by toranin 10 months ago.
Fix for this issue in twisted.web.xmlrpc in version 8.1.0

Change History

  2008-08-27 19:06:21+00:00 changed by toranin

  • attachment twisted.web.xmlrpc-8.1.0-fix.patch added

Fix for this issue in twisted.web.xmlrpc in version 8.1.0

  2008-08-27 19:06:30+00:00 changed by exarkun

  • cc set to exarkun
  • branch deleted

What response provokes this?

  2008-08-27 19:26:46+00:00 changed by toranin

Here's the response: 

Date: Wed, 27 Aug 2008 19:24:45 UTC
Content-Type: text/xml
Connection: close
Cache-control: private
Content-Length: 99
<?xml version="1.0"?>
<methodResponse>
 <params>
  <param>
  </param>
 </params>
</methodResponse>

It's produced by an in-house server with a couple of bugs, and that will be fixed at some point. Nonetheless, twisted's xmlrpc handler ought not to 'hang' when presented with this.

  2008-09-01 14:16:30+00:00 changed by therve

  • owner changed from jknight to therve

  2008-09-01 14:17:14+00:00 changed by therve

  • branch set to branches/xmlrpc-empty-response-3399
  • author set to therve

(In [24664]) Branching to 'xmlrpc-empty-response-3399'

  2008-09-01 14:18:11+00:00 changed by therve

(In [24666]) Apply the fix, with a test.

Refs #3399

  2008-09-01 14:18:52+00:00 changed by therve

  • keywords set to review
  • owner deleted

Please review!

  2008-09-01 14:23:30+00:00 changed by exarkun

  • keywords deleted
  • owner set to therve

The comment in test_parseResponseWithoutData isn't quite right. The failure mode is that it parseResponse raises IndexError, right?

  2008-09-02 07:14:22+00:00 changed by therve

  • keywords set to review
  • owner deleted

Hum I got confused. It should be better now.

  2008-09-04 13:52:10+00:00 changed by exarkun

  • keywords deleted
  • owner set to therve

Looks better. twisted.web.xmlrpc.Proxy.callRemote could perhaps use some documentation regarding its errors (well, it has no documentation at all right now). This change adds a new way it can fail, so it would be good to at least mention that.

Merge when you're happy with it.

  2008-09-05 09:18:34+00:00 changed by therve

  • status changed from new to closed
  • resolution set to fixed

(In [24679]) Merge xmlrpc-empty-response-3399

Author: therve Reviewer: exarkun Fixes #3399

Fix xmlrpc client so that he manages to return an error for buggy servers sending an empty response.

Note: See TracTickets for help on using tickets.