175 lines
6.8 KiB
Diff
175 lines
6.8 KiB
Diff
|
---
|
||
|
src/twisted/web/http.py | 32 +++++++++--
|
||
|
src/twisted/web/newsfragments/11976.bugfix | 7 ++
|
||
|
src/twisted/web/test/test_web.py | 81 ++++++++++++++++++++++++++++-
|
||
|
3 files changed, 114 insertions(+), 6 deletions(-)
|
||
|
|
||
|
--- a/src/twisted/web/http.py
|
||
|
+++ b/src/twisted/web/http.py
|
||
|
@@ -2419,14 +2419,38 @@ class HTTPChannel(basic.LineReceiver, po
|
||
|
|
||
|
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 (
|
||
|
@@ -2438,9 +2462,7 @@ class HTTPChannel(basic.LineReceiver, po
|
||
|
# ready. See docstring for _optimisticEagerReadSize above.
|
||
|
self._networkProducer.pauseProducing()
|
||
|
return
|
||
|
- return basic.LineReceiver.dataReceived(self, data)
|
||
|
|
||
|
- def rawDataReceived(self, data):
|
||
|
self.resetTimeout()
|
||
|
|
||
|
try:
|
||
|
--- /dev/null
|
||
|
+++ b/src/twisted/web/newsfragments/11976.bugfix
|
||
|
@@ -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.
|
||
|
--- a/src/twisted/web/test/test_web.py
|
||
|
+++ b/src/twisted/web/test/test_web.py
|
||
|
@@ -8,6 +8,7 @@ Tests for various parts of L{twisted.web
|
||
|
import os
|
||
|
import zlib
|
||
|
from io import BytesIO
|
||
|
+from typing import List
|
||
|
|
||
|
from zope.interface import implementer
|
||
|
from zope.interface.verify import verifyObject
|
||
|
@@ -17,10 +18,13 @@ from twisted.internet.address import IPv
|
||
|
from twisted.internet.task import Clock
|
||
|
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.test.proto_helpers import EventLoggingObserver
|
||
|
+from twisted.test.proto_helpers import EventLoggingObserver, StringTransport
|
||
|
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
|
||
|
@@ -1849,3 +1853,78 @@ class ExplicitHTTPFactoryReactor(unittes
|
||
|
|
||
|
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)
|