Skip to content

Commit

Permalink
11976 stop processing pipelined HTTP/1.1 requests that are received t…
Browse files Browse the repository at this point in the history
…ogether (#11979)
  • Loading branch information
glyph committed Sep 12, 2023
2 parents 026ef42 + 3989873 commit 1e6e9d2
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 6 deletions.
32 changes: 27 additions & 5 deletions src/twisted/web/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -2431,14 +2431,38 @@ def allContentReceived(self):

self._handlingRequest = True

# We go into raw mode here even though we will be receiving lines next
# in the protocol; however, this data will be buffered and then passed
# back to line mode in the setLineMode call in requestDone.
self.setRawMode()

req = self.requests[-1]
req.requestReceived(command, path, version)

def dataReceived(self, data):
def rawDataReceived(self, data: bytes) -> None:
"""
Data was received from the network. Process it.
This is called when this HTTP/1.1 parser is in raw mode rather than
line mode.
It may be in raw mode for one of two reasons:
1. All the headers of a request have been received and this
L{HTTPChannel} is currently receiving its body.
2. The full content of a request has been received and is currently
being processed asynchronously, and this L{HTTPChannel} is
buffering the data of all subsequent requests to be parsed
later.
In the second state, the data will be played back later.
@note: This isn't really a public API, and should be invoked only by
L{LineReceiver}'s line parsing logic. If you wish to drive an
L{HTTPChannel} from a custom data source, call C{dataReceived} on
it directly.
@see: L{LineReceive.rawDataReceived}
"""
# If we're currently handling a request, buffer this data.
if self._handlingRequest:
self._dataBuffer.append(data)
if (
Expand All @@ -2450,9 +2474,7 @@ def dataReceived(self, data):
# ready. See docstring for _optimisticEagerReadSize above.
self._networkProducer.pauseProducing()
return
return basic.LineReceiver.dataReceived(self, data)

def rawDataReceived(self, data):
self.resetTimeout()

try:
Expand Down
7 changes: 7 additions & 0 deletions src/twisted/web/newsfragments/11976.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
In Twisted 16.3.0, we changed twisted.web to stop dispatching HTTP/1.1
pipelined requests to application code. There was a bug in this change which
still allowed clients which could send multiple full HTTP requests in a single
TCP segment to trigger asynchronous processing of later requests, which could
lead to out-of-order responses. This has now been corrected and twisted.web
should never process a pipelined request over HTTP/1.1 until the previous
request has fully completed.
81 changes: 80 additions & 1 deletion src/twisted/web/test/test_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@
import os
import zlib
from io import BytesIO
from typing import List

from zope.interface import implementer
from zope.interface.verify import verifyObject

from twisted.internet import interfaces
from twisted.internet.address import IPv4Address, IPv6Address
from twisted.internet.task import Clock
from twisted.internet.testing import EventLoggingObserver
from twisted.internet.testing import EventLoggingObserver, StringTransport
from twisted.logger import LogLevel, globalLogPublisher
from twisted.python import failure, reflect
from twisted.python.compat import iterbytes
from twisted.python.filepath import FilePath
from twisted.trial import unittest
from twisted.web import error, http, iweb, resource, server
from twisted.web.resource import Resource
from twisted.web.server import NOT_DONE_YET, Request, Site
from twisted.web.static import Data
from twisted.web.test.requesthelper import DummyChannel, DummyRequest
from ._util import assertIsFilesystemTemporary
Expand Down Expand Up @@ -1849,3 +1853,78 @@ def test_defaultReactor(self):

factory = http.HTTPFactory()
self.assertIs(factory.reactor, reactor)


class QueueResource(Resource):
"""
Add all requests to an internal queue,
without responding to the requests.
You can access the requests from the queue and handle their response.
"""

isLeaf = True

def __init__(self) -> None:
super().__init__()
self.dispatchedRequests: List[Request] = []

def render_GET(self, request: Request) -> int:
self.dispatchedRequests.append(request)
return NOT_DONE_YET


class TestRFC9112Section932(unittest.TestCase):
"""
Verify that HTTP/1.1 request ordering is preserved.
"""

def test_multipleRequestsInOneSegment(self) -> None:
"""
Twisted MUST NOT respond to a second HTTP/1.1 request while the first
is still pending.
"""
qr = QueueResource()
site = Site(qr)
proto = site.buildProtocol(None)
serverTransport = StringTransport()
proto.makeConnection(serverTransport)
proto.dataReceived(
b"GET /first HTTP/1.1\r\nHost: a\r\n\r\n"
b"GET /second HTTP/1.1\r\nHost: a\r\n\r\n"
)
# The TCP data contains 2 requests,
# but only 1 request was dispatched,
# as the first request was not yet finalized.
self.assertEqual(len(qr.dispatchedRequests), 1)
# The first request is finalized and the
# second request is dispatched right away.
qr.dispatchedRequests[0].finish()
self.assertEqual(len(qr.dispatchedRequests), 2)

def test_multipleRequestsInDifferentSegments(self) -> None:
"""
Twisted MUST NOT respond to a second HTTP/1.1 request while the first
is still pending, even if the second request is received in a separate
TCP package.
"""
qr = QueueResource()
site = Site(qr)
proto = site.buildProtocol(None)
serverTransport = StringTransport()
proto.makeConnection(serverTransport)
raw_data = (
b"GET /first HTTP/1.1\r\nHost: a\r\n\r\n"
b"GET /second HTTP/1.1\r\nHost: a\r\n\r\n"
)
# Just go byte by byte for the extreme case in which each byte is
# received in a separate TCP package.
for chunk in iterbytes(raw_data):
proto.dataReceived(chunk)
# The TCP data contains 2 requests,
# but only 1 request was dispatched,
# as the first request was not yet finalized.
self.assertEqual(len(qr.dispatchedRequests), 1)
# The first request is finalized and the
# second request is dispatched right away.
qr.dispatchedRequests[0].finish()
self.assertEqual(len(qr.dispatchedRequests), 2)

0 comments on commit 1e6e9d2

Please sign in to comment.