Skip to content

Commit

Permalink
[PR #8597/c99a1e27 backport][3.10] Fix reading of body when ignoring …
Browse files Browse the repository at this point in the history
…an upgrade request (#8629)

Co-authored-by: Sam Bull <git@sambull.org>
Co-authored-by: J. Nick Koston <nick@koston.org>
Fixes #8414.
  • Loading branch information
patchback[bot] authored Aug 7, 2024
1 parent 266608d commit 4815765
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGES/8597.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed request body not being read when ignoring an Upgrade request -- by :user:`Dreamsorcerer`.
19 changes: 11 additions & 8 deletions aiohttp/_http_parser.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions tests/test_http_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
8 changes: 1 addition & 7 deletions tests/test_web_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down

0 comments on commit 4815765

Please sign in to comment.