Ticket #3399 defect closed fixed

Opened 6 years ago

Last modified 6 years ago

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

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

Change History

Changed 6 years ago by toranin

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

1

Changed 6 years ago by exarkun

  • cc exarkun added
  • branch 8.1.0 deleted

What response provokes this?

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.

3

Changed 6 years ago by therve

  • owner changed from jknight to therve

4

Changed 6 years ago by therve

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

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

5

Changed 6 years ago by therve

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

Refs #3399

6

Changed 6 years ago by therve

  • keywords review added
  • owner therve deleted

Please review!

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?

8

Changed 6 years ago by therve

  • keywords review added
  • owner therve deleted

Hum I got confused. It should be better now.

9

Changed 6 years ago by exarkun

  • owner set to therve
  • keywords review removed

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.

10

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

11

Changed 3 years ago by <automation>

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