Opened 6 years ago

Closed 6 years ago

#3399 defect closed fixed (fixed)

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

Reported by: toranin Owned by:
Priority: normal Milestone:
Component: web Keywords:
Cc: exarkun Branch: branches/xmlrpc-empty-response-3399
(diff, github, buildbot, log)
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 (1)

twisted.web.xmlrpc-8.1.0-fix.patch (650 bytes) - added by toranin 6 years ago.
Fix for this issue in twisted.web.xmlrpc in version 8.1.0

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by toranin

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

comment:1 Changed 6 years ago by exarkun

  • Branch 8.1.0 deleted
  • Cc exarkun added

What response provokes this?

comment:2 Changed 6 years ago 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.

comment:3 Changed 6 years ago by therve

  • Owner changed from jknight to therve

comment:4 Changed 6 years ago by therve

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

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

comment:5 Changed 6 years ago by therve

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

Refs #3399

comment:6 Changed 6 years ago by therve

  • Keywords review added
  • Owner therve deleted

Please review!

comment:7 Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner set to therve

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

comment:8 Changed 6 years ago by therve

  • Keywords review added
  • Owner therve deleted

Hum I got confused. It should be better now.

comment:9 Changed 6 years ago by exarkun

  • Keywords review removed
  • 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.

comment:10 Changed 6 years ago by therve

  • Resolution set to fixed
  • Status changed from new to closed

(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.

comment:11 Changed 4 years ago by <automation>

  • Owner therve deleted
Note: See TracTickets for help on using tickets.