[PR #8597/c99a1e27 backport][3.10] Fix reading of body when ignoring an upgrade request (#8629)

Co-authored-by: Sam Bull <[email protected]>
Co-authored-by: J. Nick Koston <[email protected]>
Fixes #8414.

Author: patchback[bot]

CHANGES/8597.bugfix.rst

@@ -0,0 +1 @@
+Fixed request body not being read when ignoring an Upgrade request -- by :user:`Dreamsorcerer`.

aiohttp/_http_parser.pyx

@@ -47,6 +47,7 @@ include "_headers.pxi"
 
 from aiohttp cimport _find_header
 
+ALLOWED_UPGRADES = frozenset({"websocket"})
 DEF DEFAULT_FREELIST_SIZE = 250
 
 cdef extern from "Python.h":
@@ -417,16 +418,20 @@ cdef class HttpParser:
     cdef _on_headers_complete(self):
         self._process_header()
 
-        method = http_method_str(self._cparser.method)
         should_close = not cparser.llhttp_should_keep_alive(self._cparser)
         upgrade = self._cparser.upgrade
         chunked = self._cparser.flags & cparser.F_CHUNKED
 
         raw_headers = tuple(self._raw_headers)
         headers = CIMultiDictProxy(self._headers)
 
-        if upgrade or self._cparser.method == cparser.HTTP_CONNECT:
-            self._upgraded = True
+        if self._cparser.type == cparser.HTTP_REQUEST:
+            allowed = upgrade and headers.get("upgrade", "").lower() in ALLOWED_UPGRADES
+            if allowed or self._cparser.method == cparser.HTTP_CONNECT:
+                self._upgraded = True
+        else:
+            if upgrade and self._cparser.status_code == 101:
+                self._upgraded = True
 
         # do not support old websocket spec
         if SEC_WEBSOCKET_KEY1 in headers:
@@ -441,6 +446,7 @@ cdef class HttpParser:
                 encoding = enc
 
         if self._cparser.type == cparser.HTTP_REQUEST:
+            method = http_method_str(self._cparser.method)
             msg = _new_request_message(
                 method, self._path,
                 self.http_version(), headers, raw_headers,
@@ -565,7 +571,7 @@ cdef class HttpParser:
         if self._upgraded:
             return messages, True, data[nb:]
         else:
-            return messages, False, b''
+            return messages, False, b""
 
     def set_upgraded(self, val):
         self._upgraded = val
@@ -748,10 +754,7 @@ cdef int cb_on_headers_complete(cparser.llhttp_t* parser) except -1:
         pyparser._last_error = exc
         return -1
     else:
-        if (
-            pyparser._cparser.upgrade or
-            pyparser._cparser.method == cparser.HTTP_CONNECT
-        ):
+        if pyparser._upgraded or pyparser._cparser.method == cparser.HTTP_CONNECT:
             return 2
         else:
             return 0

tests/test_http_parser.py

@@ -823,6 +823,26 @@ def test_http_request_upgrade(parser: Any) -> None:
     assert tail == b"some raw data"
 
 
+async def test_http_request_upgrade_unknown(parser: Any) -> None:
+    text = (
+        b"POST / HTTP/1.1\r\n"
+        b"Connection: Upgrade\r\n"
+        b"Content-Length: 2\r\n"
+        b"Upgrade: unknown\r\n"
+        b"Content-Type: application/json\r\n\r\n"
+        b"{}"
+    )
+    messages, upgrade, tail = parser.feed_data(text)
+
+    msg = messages[0][0]
+    assert not msg.should_close
+    assert msg.upgrade
+    assert not upgrade
+    assert not msg.chunked
+    assert tail == b""
+    assert await messages[0][-1].read() == b"{}"
+
+
 @pytest.fixture
 def xfail_c_parser_url(request) -> None:
     if isinstance(request.getfixturevalue("parser"), HttpRequestParserPy):

tests/test_web_server.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from aiohttp import client, helpers, web
+from aiohttp import client, web
 
 
 async def test_simple_server(aiohttp_raw_server, aiohttp_client) -> None:
@@ -19,12 +19,6 @@ async def handler(request):
     assert txt == "/path/to"
 
 
-@pytest.mark.xfail(
-    not helpers.NO_EXTENSIONS,
-    raises=client.ServerDisconnectedError,
-    reason="The behavior of C-extensions differs from pure-Python: "
-    "https://github.com/aio-libs/aiohttp/issues/6446",
-)
 async def test_unsupported_upgrade(aiohttp_raw_server, aiohttp_client) -> None:
     # don't fail if a client probes for an unsupported protocol upgrade
     # https://github.com/aio-libs/aiohttp/issues/6446#issuecomment-999032039