twisted.web.http does not throttle pipelined requests

[ivank]

twisted.web.http loads an unlimited amount of pipelined HTTP requests into memory, even when it is busy processing. This allows for a very easy DoS attack.

My suggestion is to call .pauseProducing() on the underlying transport when there are 8 pipelined requests queued, and .resumeProducing() when there are fewer.

Note that even browsers may send as many concurrent pipelined requests as they want to (Opera for example has no limit).

[jknight]

It might actually make sense to not handle *any* additional requests while processing one. The main win of the pipelining is usually gonna be the RTT, not processing time. And normally the buffering in TCP will handle storing up a few extra requests. So reading more requests is probably not often a very useful thing to do.

Alternatively, web2's http server implementation is awesome in every way, and in particular, restricts it to 4. :)

[Indradhanush Gupta]

Added functionality for limiting HTTP requests to 8.

[Jean-Paul Calderone]

Thanks for your work on this issue. The attached patch looks like a step in the right direction. All changes and new functionality need to be verified by automated unit tests as well. Can you add unit tests for these changes as well? Thanks again! (Also, a random note, trac separates keywords using spaces, not commas)

[Indradhanush Gupta]

Thanks for the 'random note'. This one is my first patch. Felt nervous. I'll try and do something about the unit tests.

[Indradhanush Gupta]

Made a small change, _pauseFlag = False once the resumeProducing() is about to be called.

[Indradhanush Gupta]

I've added the test. It creates 9 requests to invoke pauseProducing() and removes 2 of them from the queue to invoke reumeProducing() once again. The problem I am facing is in the first assertTrue() call, even if I'm comparing _pauseFlag with False the test is passing, however _pauseFlag should actually be True once 8 requests are pipelined. I've intentionally left the first assertTrue() statement as :

self.assertTrue(Request._pauseFlag == False)

instead of :

self.assertTrue(Request._pauseFlag == True)

Please someone do have a look at it.

[Indradhanush Gupta]

I am interested in participating in GSoC 13 under Twisted. itamar suggested to assign it to you.

[Tom Prince]

twistd -n web --resource-script=test.rpy

[Tom Prince]

I think this code is written at the wrong level. The code you have is only in Request, so that self._quededRequests is only ever 0 or 1. The code probably wants to be on HTTPChanel. I figured this out after actually trying to run the attached code.

Accessing '​http://localhost:8080/never' never completes, so should never complete, and '​http://localhost:8080' returns an empty page. So trying to open 8 copies of the former and then one of the later should hang. But the final one will complete if any of the `/never' requests are cancelled.

A couple of points about the code itself.

1. You appear to be using a large (>4) indent in a couple of places.
2. You are assigning to a local variable _pauseFlag.
3. Explanation of instance variables should be in the class docstring, rather than comments.

[Cory Benfield]

This is no longer relevant because of #8320, which removed pipelining support from twisted.web by doing exactly this.