Error when handling HTTP requests with HTTP2 related "Upgrade: h2c" in header #6446
Comments
|
Also got some TCP dumps from the good and bad test case: aiohttp 3.6, good aiohttp >= 3.7, bad |
pwuertz
changed the title
|
All right, it seems that this problem is solely triggered by this line in the request header: When this header is present, aiohttp to fails and drops the connection. |
|
HTTP/2 is not supported at the moment. But you could contribute it: #5999. |
Description said that the connection works with aiohttp 3.6. So there is presumably some kind of regression that is not related to supporting HTTP/2. |
Missing HTTP/2 support is not the issue here. The client did send a perfectly valid HTTP/1.1 request with the suggestion of switching to another protocol the server may or may not support. If the server supports and agrees to switching to a suggested protocol, the server replies with a The server doesn't decide if a protocol upgrade is mandatory. It's the clients decision to either continue using the non-upgraded protocol or to drop the connection and report the absence of a 101 reply as sort of a |
|
Well, then it would be useful to get a PR with a failing regression test. |
|
Here is a minimal from aiohttp import web, test_utils
async def test_unsupported_upgrade() -> None:
# don't fail if a client probes for an unsupported protocol upgrade
# https://github.com/aio-libs/aiohttp/issues/6446#issuecomment-999032039
async def post_handler(request: web.Request):
return web.Response(body=await request.read())
upgrade_headers = {"Connection": "Upgrade", "Upgrade": "unsupported_proto"}
app = web.Application()
app.add_routes([web.post("/", post_handler)])
async with test_utils.TestServer(app) as server:
async with test_utils.TestClient(server) as cli:
resp = await cli.post("/", data=b"Test", headers=upgrade_headers)
assert resp.status == 200
assert (await resp.read()) == b"Test"
# asyncio.run(test_unsupported_upgrade()) or similarOddly enough, no error is triggered when running this function as part of the aiohttp tests, e.g. by adding it to |
|
Could you submit that as a pull request? It'd give us visibility into the behavior in multiple envs, not just your laptop. As for the difference, it is possible that you're running the tests with pure-Python fallback parser implementation which may be different from the Cython-based one. If this is true, we'd need to make sure to make the behavior equivalent too. |
Maybe also double check what versions you are testing with. Maybe the issue is not present on master, but only in the 3.8/3.9 branches.. |
Right, I assumed this could be the case, but I'm pretty sure I excluded this possibility by checking out 3.8.1 (still no error in tests) + installing from source and running the code (error is triggered) 🤷 |
Added PR #6451 for the regression test. Sorry, I didn't understand the need for PR earlier because the snippet in #6446 (comment) already provided visibility beyond my PC, while the testing framework doesn't seem to do the 'right' thing at this time.
Oh, that sounds reasonable, that difference kept me bamboozled for quite some time. |
Oh, that is exactly what happened here! The tests were performed against |
FYI it should be enough to run Meanwhile, I've approved the CI run and we should soon have the logs of how it fails there. If everything goes as expected, that test can be marked as xfail and merged per https://pganssle-talks.github.io/xfail-lightning. |
|
So, I think the change that caused this is probably switching to llhttp for the parsing. I'm not familiar with this, so not sure if the bug needs to be reported upstream to llhttp or if there's something we can fix here. Maybe @derlih might know more. |
I don't think so. The reporter claims that the error started happening in aiohttp 3.7 but the switch hasn't happened until aiohttp 3.8. |
|
Here's the log from the CI failure for history: =================================== FAILURES ===================================
_______________________ test_unsupported_upgrade[pyloop] _______________________
aiohttp_raw_server = <function aiohttp_raw_server.<locals>.go at 0x7f077f28ec80>
aiohttp_client = <function aiohttp_client.<locals>.go at 0x7f077f28e0d0>
async def test_unsupported_upgrade(aiohttp_raw_server, aiohttp_client) -> None:
# don't fail if a client probes for an unsupported protocol upgrade
# aio-libs/aiohttp/issues/6446#issuecomment-999032039
async def handler(request: web.Request):
return web.Response(body=await request.read())
upgrade_headers = {"Connection": "Upgrade", "Upgrade": "unsupported_proto"}
server = await aiohttp_raw_server(handler)
cli = await aiohttp_client(server)
test_data = b"Test"
> resp = await cli.post("/path/to", data=test_data, headers=upgrade_headers)
aiohttp_client = <function aiohttp_client.<locals>.go at 0x7f077f28e0d0>
aiohttp_raw_server = <function aiohttp_raw_server.<locals>.go at 0x7f077f28ec80>
cli = <aiohttp.test_utils.TestClient object at 0x7f077f2dd7f0>
handler = <function test_unsupported_upgrade.<locals>.handler at 0x7f077f28e510>
server = <aiohttp.test_utils.RawTestServer object at 0x7f077f03f4e0>
test_data = b'Test'
upgrade_headers = {'Connection': 'Upgrade', 'Upgrade': 'unsupported_proto'}
tests/test_web_server.py:31:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
aiohttp/test_utils.py:324: in _request
resp = await self._session.request(method, self.make_url(path), **kwargs)
kwargs = {'data': b'Test',
'headers': {'Connection': 'Upgrade', 'Upgrade': 'unsupported_proto'}}
method = 'POST'
path = '/path/to'
self = <aiohttp.test_utils.TestClient object at 0x7f077f2dd7f0>
aiohttp/client.py:559: in _request
await resp.start(conn)
all_cookies = <SimpleCookie: >
allow_redirects = True
auth = None
auth_from_url = None
chunked = None
compress = None
conn = Connection<ConnectionKey(host='127.0.0.1', port=42413, is_ssl=False, ssl=None, proxy=None, proxy_auth=None, proxy_headers_hash=None)>
cookies = None
data = b'Test'
expect100 = False
fingerprint = None
handle = None
headers = <CIMultiDict('Connection': 'Upgrade', 'Upgrade': 'unsupported_proto')>
history = []
json = None
max_redirects = 10
method = 'POST'
params = None
proxy = None
proxy_auth = None
proxy_headers = <CIMultiDict()>
raise_for_status = None
read_bufsize = 65536
read_until_eof = True
real_timeout = ClientTimeout(total=300, connect=None, sock_read=None, sock_connect=None)
redirects = 0
req = <aiohttp.client_reqrep.ClientRequest object at 0x7f077f2dd4a8>
resp = <ClientResponse(http://127.0.0.1:42413/path/to) [None None]>
None
self = <aiohttp.client.ClientSession object at 0x7f077f2dd320>
skip_auto_headers = None
skip_headers = set()
ssl = None
ssl_context = None
str_or_url = URL('http://127.0.0.1:42413/path/to')
timeout = <object object at 0x7f0785b8d6d0>
timer = <aiohttp.helpers.TimerContext object at 0x7f077f2ddc50>
tm = <aiohttp.helpers.TimeoutHandle object at 0x7f077f2dda90>
trace_request_ctx = None
traces = []
url = URL('http://127.0.0.1:42413/path/to')
verify_ssl = None
version = HttpVersion(major=1, minor=1)
aiohttp/client_reqrep.py:896: in start
message, payload = await protocol.read() # type: ignore[union-attr]
connection = Connection<ConnectionKey(host='127.0.0.1', port=42413, is_ssl=False, ssl=None, proxy=None, proxy_auth=None, proxy_headers_hash=None)>
protocol = <aiohttp.client_proto.ResponseHandler object at 0x7f077f087ce0>
self = <ClientResponse(http://127.0.0.1:42413/path/to) [None None]>
None
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <aiohttp.client_proto.ResponseHandler object at 0x7f077f087ce0>
async def read(self) -> _T:
if not self._buffer and not self._eof:
assert not self._waiter
self._waiter = self._loop.create_future()
try:
> await self._waiter
E aiohttp.client_exceptions.ServerDisconnectedError: Server disconnected
self = <aiohttp.client_proto.ResponseHandler object at 0x7f077f087ce0>
aiohttp/streams.py:616: ServerDisconnectedError
------------------------------ Captured log call -------------------------------
ERROR aiohttp.server:web_protocol.py:405 Unhandled exception
Traceback (most recent call last):
File "/home/runner/work/aiohttp/aiohttp/aiohttp/web_protocol.py", line 514, in start
resp, reset = await task
File "/home/runner/work/aiohttp/aiohttp/aiohttp/web_protocol.py", line 460, in _handle_request
reset = await self.finish_response(request, resp, start_time)
File "/home/runner/work/aiohttp/aiohttp/aiohttp/web_protocol.py", line 599, in finish_response
self._request_parser.feed_data(self._message_tail)
File "aiohttp/_http_parser.pyx", line 551, in aiohttp._http_parser.HttpParser.feed_data
raise ex
aiohttp.http_exceptions.BadStatusLine: 400, message="Bad status line 'Invalid method encountered'"It shows that the server seems to attempt sending HTTP 400 ( |
|
Ah, OK. I just looked at the last line:
And noticed that it comes from some llhttp thing a few lines earlier (plus the error doesn't occur without extensions): https://github.com/aio-libs/aiohttp/blob/master/aiohttp/_http_parser.pyx#L531 So, I thought that was it. |
Actually, isn't that 2 separate issues? If the connection works correctly in aiohttp 3.6, then raising a BadStatusLine is a regression. Then failing to send the 400 response is another issue. |
… (#6451) Co-authored-by: Sviatoslav Sydorenko <webknjaz@redhat.com>
…libs#6446) (aio-libs#6451) Co-authored-by: Sviatoslav Sydorenko <webknjaz@redhat.com> (cherry picked from commit a60e8a5)
|
Yep, maybe it's two issues. I don't have time to look into that right now but I've taken care of adding the xfailing test to the supported branches. The original PR added it to |
webknjaz
added
reproducer: missing
|
This was fixed relatively recently. |
Describe the bug
I'm experiencing problems when trying to handle HTTP requests sent by Qt6 clients. It looks like Qt now sends additional HTTP2 related headers by default. HTTP servers based on
aiohttp 3.6and pythonhttp.serverare handling those requests just fine, butaiohttp 3.7and3.8are showing irregularities in the parsed headers, are unable to read request bodies, and fail to send replies.Test code and examples for good/bad observations are attached to this issue.
To Reproduce
Qt6 / Qt5 client test code:
Aiohttp test server:
Expected behavior
aiohttp server should be able to handle the requests.
Logs/tracebacks
Good result with aiohttp 3.6:
Bad result with aiohttp 3.7 and 3.8:
Python Version
Python 3.9.7aiohttp Version
Version: 3.8.1multidict Version
Version: 4.7.6yarl Version
Version: 1.5.1OS
Ubuntu 21.10
Related component
Server
Additional context
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: