Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading