From c99a1e27375285149ea82cbdcc2f2c40e57596dc Mon Sep 17 00:00:00 2001 From: Sam Bull 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