From c99a1e27375285149ea82cbdcc2f2c40e57596dc Mon Sep 17 00:00:00 2001
From: Sam Bull <git@sambull.org>
Date: Wed, 7 Aug 2024 13:59:02 +0100
Subject: [PATCH] Fix reading of body when ignoring an upgrade request (#8597)

---
 CHANGES/8597.bugfix.rst   |  1 +
 aiohttp/_http_parser.pyx  | 19 +++++++++++--------
 tests/test_http_parser.py | 20 ++++++++++++++++++++
 tests/test_web_server.py  |  8 +-------
 4 files changed, 33 insertions(+), 15 deletions(-)
 create mode 100644 CHANGES/8597.bugfix.rst

diff --git a/CHANGES/8597.bugfix.rst b/CHANGES/8597.bugfix.rst
new file mode 100644
index 00000000000..27186bb52d1
--- /dev/null
+++ b/CHANGES/8597.bugfix.rst
@@ -0,0 +1 @@
+Fixed request body not being read when ignoring an Upgrade request -- by :user:`Dreamsorcerer`.
diff --git a/aiohttp/_http_parser.pyx b/aiohttp/_http_parser.pyx
index 05e190bf738..30767dae6ce 100644
--- a/aiohttp/_http_parser.pyx
+++ b/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,7 +418,6 @@ 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
@@ -425,8 +425,13 @@ cdef class HttpParser:
         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
diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py
index 5fdc5699d38..15d92f28c92 100644
--- a/tests/test_http_parser.py
+++ b/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):
diff --git a/tests/test_web_server.py b/tests/test_web_server.py
index f26f0537ec7..619b83e947c 100644
--- a/tests/test_web_server.py
+++ b/tests/test_web_server.py
@@ -6,7 +6,7 @@
 
 import pytest
 
-from aiohttp import client, helpers, web
+from aiohttp import client, web
 
 
 async def test_simple_server(aiohttp_raw_server: Any, aiohttp_client: Any) -> None:
@@ -21,12 +21,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