Repository: twisted/twisted

Issue:
twisted.web.server doesn't support HTTP pipelining
**Describe the incorrect behavior you saw**
twisted.web.server responds in wrong order

**Describe how to cause this behavior**

https://gist.github.com/mukeran/01874b38334bb7618415695d666a3f29

When running the above code with `xxx.xxx.xxx.xxx:10780` as the target reverse proxy site and sending the following HTTP pipeline request to twisted, it seems that twisted processes the request asynchronously.

The `/hello` route responds first, then the `/reverse` route responds. The pipeline response is out-of-order.

```http
GET /reverse HTTP/1.1
Host: a.com
Connection: keep-alive

GET /hello HTTP/1.1
Host: a.com
Connection: close

```

**Describe the correct behavior you'd like to see**
Twisted should send the response of `/reverse` first instead of `/hello`'s.

**Testing environment**

 - Operating System and Version:
     ```
     Linux *** 6.2.0-26-generic #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Jul 13 16:27:29 UTC 2 x86_64 x86_64 x86_64 GNU/Linux
     DISTRIB_ID=Ubuntu
     DISTRIB_RELEASE=22.04
     DISTRIB_CODENAME=jammy
     DISTRIB_DESCRIPTION="Ubuntu 22.04.2 LTS"
    ```
 - Twisted version: 23.8.0



Discussion:

Patch:
diff --git a/src/twisted/web/http.py b/src/twisted/web/http.py
index 583512b3f0..2f3c0d7bf9 100644
--- a/src/twisted/web/http.py
+++ b/src/twisted/web/http.py
@@ -2431,14 +2431,38 @@ class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
 
         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 (
@@ -2450,9 +2474,7 @@ class HTTPChannel(basic.LineReceiver, policies.TimeoutMixin):
                 # ready.  See docstring for _optimisticEagerReadSize above.
                 self._networkProducer.pauseProducing()
             return
-        return basic.LineReceiver.dataReceived(self, data)
 
-    def rawDataReceived(self, data):
         self.resetTimeout()
 
         try:
diff --git a/src/twisted/web/newsfragments/11976.bugfix b/src/twisted/web/newsfragments/11976.bugfix
new file mode 100644
index 0000000000..8ac292bef5
--- /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.


Test patch:
diff --git a/src/twisted/web/test/test_web.py b/src/twisted/web/test/test_web.py
index c196249722..13c3291651 100644
--- 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
@@ -15,12 +16,15 @@ 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
@@ -1849,3 +1853,78 @@ class ExplicitHTTPFactoryReactor(unittest.TestCase):
 
         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)