Opened 8 years ago

Last modified 15 months ago

#6752 defect new

twisted.web.client.RedirectAgent doesn't do anything with redirect response bodies, possibly leaking requests

Reported by: Glyph Owned by:
Priority: high Milestone:
Component: web Keywords:
Cc: jknight, Erlend Graff Branch:
Author:

Description

RedirectAgent._handleRedirect gets an HTTP redirect response, but does not take into account that it may have a response body.

One negative consequence of this is that if a RedirectAgent is wrapped around an agent with a busy HTTPConnectionPool attached, the application using the RedirectAgent may never have an opportunity to handle the previous response stored in response.previousResponse, since it could be waiting for the connection busy with a large redirect response-body to free up - which it never will, because nobody's around to call deliverBody with it.

(See also #6751, which is tangentially related.)

Attachments (1)

example.py (1.3 KB) - added by Illarion Kovalchuk 8 years ago.
Real example

Download all attachments as: .zip

Change History (8)

comment:1 Changed 8 years ago by DefaultCC Plugin

Cc: jknight added

comment:2 Changed 8 years ago by Glyph

Now that #6751 is fixed, the consequence of this bug will be a resource leak; since the request channel is neither reading nor writing until somebody calls deliverBody, it will hang out forever, leaving an FD allocated. The server will time it out and close the connection, but since it's not in the event loop to notice, you'll eventually run out of file descriptors.

comment:3 Changed 8 years ago by Glyph

Owner: set to Glyph
Status: newassigned

Changed 8 years ago by Illarion Kovalchuk

Attachment: example.py added

Real example

comment:4 Changed 8 years ago by Illarion Kovalchuk

Solution that works for me:

class IgnoreBody(Protocol):
    def __init__(self, deferred):
        self.deferred = deferred

    def dataReceived(self, bytes):
        pass

    def connectionLost(self, reason):
        self.deferred.callback(None)

@implementer(IAgent)
class NewRedirectAgent(RedirectAgent):

    def _handleRedirect(self, response, method, uri, headers, redirectCount):
        ignore = Deferred()
        response.deliverBody(IgnoreBody(ignore))
        return super(NewRedirectAgent, self)._handleRedirect(response, method, uri, headers, redirectCount)

comment:5 Changed 5 years ago by Glyph

Owner: Glyph deleted
Status: assignednew

Un-assigning from me since anyone can fix this :).

comment:6 Changed 5 years ago by Erlend Graff

Cc: Erlend Graff added

comment:7 Changed 15 months ago by Tom Most

Priority: normalhigh
Note: See TracTickets for help on using tickets.