From 7875c9c1ae9fa3a1bbddbff8aa9f74b8bc8f3cd1 Mon Sep 17 00:00:00 2001 From: Arcadiy Ivanov Date: Mon, 29 Jul 2024 09:49:28 -0400 Subject: [PATCH 1/6] WSS: Fix heartbeat timeout logic Make sure to unblock the `receive` operation by feeding the receiver an error in a `WSMessage` Change `TimeoutError` to `ServerTimeoutError` to accurately represent failure (this is backwards compatible since `ServerTimeoutError` has `TimeoutError` in the MRO) fixes #8540 --- CHANGES/8540.bugfix.rst | 7 +++++++ aiohttp/client_ws.py | 8 ++++++-- tests/test_client_ws_functional.py | 31 +++++++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 CHANGES/8540.bugfix.rst diff --git a/CHANGES/8540.bugfix.rst b/CHANGES/8540.bugfix.rst new file mode 100644 index 00000000000..325f052424c --- /dev/null +++ b/CHANGES/8540.bugfix.rst @@ -0,0 +1,7 @@ +WSS: Fix heartbeat timeout logic -- by :user:`arcivanov`. + +The :py:meth:`~aiohttp.client_ws.ClientWebSocketResponse._pong_not_received` did not + terminate a :py:meth:`~aiohttp.client_ws.ClientWebSocketResponse.receive` operation. + This change causes `_pong_not_received` to feed the `reader` an error message, causing + pending `receive` to terminate and return the error message. The error message contains + the exception :py:class:`~aiohttp.client_exceptions.ServerTimeoutError`. diff --git a/aiohttp/client_ws.py b/aiohttp/client_ws.py index 28d1117502f..e6c720b4264 100644 --- a/aiohttp/client_ws.py +++ b/aiohttp/client_ws.py @@ -5,7 +5,7 @@ import sys from typing import Any, Final, Optional, cast -from .client_exceptions import ClientError +from .client_exceptions import ClientError, ServerTimeoutError from .client_reqrep import ClientResponse from .helpers import call_later, set_result from .http import ( @@ -132,8 +132,12 @@ def _pong_not_received(self) -> None: if not self._closed: self._closed = True self._close_code = WSCloseCode.ABNORMAL_CLOSURE - self._exception = asyncio.TimeoutError() + self._exception = ServerTimeoutError() self._response.close() + if self._waiting and not self._closing: + self._reader.feed_data( + WSMessage(WSMsgType.ERROR, self._exception, None) + ) @property def closed(self) -> bool: diff --git a/tests/test_client_ws_functional.py b/tests/test_client_ws_functional.py index 2d65b317c6d..6dde93cb660 100644 --- a/tests/test_client_ws_functional.py +++ b/tests/test_client_ws_functional.py @@ -6,7 +6,7 @@ import pytest import aiohttp -from aiohttp import hdrs, web +from aiohttp import ServerTimeoutError, WSMsgType, hdrs, web from aiohttp.client_ws import ClientWSTimeout from aiohttp.http import WSCloseCode from aiohttp.pytest_plugin import AiohttpClient @@ -685,6 +685,35 @@ async def handler(request): assert resp.close_code is WSCloseCode.ABNORMAL_CLOSURE +async def test_heartbeat_no_pong_concurrent_receive(aiohttp_client: Any) -> None: + ping_received = False + + async def handler(request): + nonlocal ping_received + ws = web.WebSocketResponse(autoping=False) + await ws.prepare(request) + msg = await ws.receive() + if msg.type is aiohttp.WSMsgType.PING: + ping_received = True + ws._reader.feed_eof = lambda: None + await asyncio.sleep(10.0) + + app = web.Application() + app.router.add_route("GET", "/", handler) + + client = await aiohttp_client(app) + resp = await client.ws_connect("/", heartbeat=0.1) + resp._reader.feed_eof = lambda: None + + # Connection should be closed roughly after 1.5x heartbeat. + msg = await resp.receive(5.0) + assert ping_received + assert resp.close_code is WSCloseCode.ABNORMAL_CLOSURE + assert msg + assert msg.type is WSMsgType.ERROR + assert isinstance(msg.data, ServerTimeoutError) + + async def test_send_recv_compress(aiohttp_client: Any) -> None: async def handler(request): ws = web.WebSocketResponse() From 53928dffd521a7c1b4d9d6c107ff44945c842f6a Mon Sep 17 00:00:00 2001 From: Arcadiy Ivanov Date: Tue, 30 Jul 2024 17:35:17 -0400 Subject: [PATCH 2/6] Eliminate an if branch --- tests/test_client_ws_functional.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_client_ws_functional.py b/tests/test_client_ws_functional.py index 6dde93cb660..6830f9131b4 100644 --- a/tests/test_client_ws_functional.py +++ b/tests/test_client_ws_functional.py @@ -693,8 +693,7 @@ async def handler(request): ws = web.WebSocketResponse(autoping=False) await ws.prepare(request) msg = await ws.receive() - if msg.type is aiohttp.WSMsgType.PING: - ping_received = True + ping_received = msg.type is aiohttp.WSMsgType.PING ws._reader.feed_eof = lambda: None await asyncio.sleep(10.0) From 7e78c54944cd43b825db21e89bb22c718845ac52 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 30 Jul 2024 21:40:13 -0500 Subject: [PATCH 3/6] Update CHANGES/8540.bugfix.rst --- CHANGES/8540.bugfix.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGES/8540.bugfix.rst b/CHANGES/8540.bugfix.rst index 325f052424c..281ede4bc38 100644 --- a/CHANGES/8540.bugfix.rst +++ b/CHANGES/8540.bugfix.rst @@ -1,7 +1,7 @@ WSS: Fix heartbeat timeout logic -- by :user:`arcivanov`. -The :py:meth:`~aiohttp.client_ws.ClientWebSocketResponse._pong_not_received` did not - terminate a :py:meth:`~aiohttp.client_ws.ClientWebSocketResponse.receive` operation. +The :py:meth:`~aiohttp.ClientWebSocketResponse._pong_not_received` did not + terminate a :py:meth:`~aiohttp.ClientWebSocketResponse.receive` operation. This change causes `_pong_not_received` to feed the `reader` an error message, causing pending `receive` to terminate and return the error message. The error message contains - the exception :py:class:`~aiohttp.client_exceptions.ServerTimeoutError`. + the exception :py:class:`~aiohttp.ServerTimeoutError`. From 9a335b7b427efd3daa5ba8ee1987c133982c4589 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 30 Jul 2024 21:51:31 -0500 Subject: [PATCH 4/6] Update CHANGES/8540.bugfix.rst --- CHANGES/8540.bugfix.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES/8540.bugfix.rst b/CHANGES/8540.bugfix.rst index 281ede4bc38..4038fffde7d 100644 --- a/CHANGES/8540.bugfix.rst +++ b/CHANGES/8540.bugfix.rst @@ -1,7 +1,7 @@ WSS: Fix heartbeat timeout logic -- by :user:`arcivanov`. -The :py:meth:`~aiohttp.ClientWebSocketResponse._pong_not_received` did not - terminate a :py:meth:`~aiohttp.ClientWebSocketResponse.receive` operation. +When a WebSocket pong message was not received, the + :py:meth:`~aiohttp.ClientWebSocketResponse.receive` operation was not terminated. This change causes `_pong_not_received` to feed the `reader` an error message, causing pending `receive` to terminate and return the error message. The error message contains the exception :py:class:`~aiohttp.ServerTimeoutError`. From 1684fc8ada8d5f752fd8dad683d0dcac8bbb0c9f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 30 Jul 2024 21:52:38 -0500 Subject: [PATCH 5/6] Update CHANGES/8540.bugfix.rst --- CHANGES/8540.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/8540.bugfix.rst b/CHANGES/8540.bugfix.rst index 4038fffde7d..78a0b45a34c 100644 --- a/CHANGES/8540.bugfix.rst +++ b/CHANGES/8540.bugfix.rst @@ -1,7 +1,7 @@ WSS: Fix heartbeat timeout logic -- by :user:`arcivanov`. When a WebSocket pong message was not received, the - :py:meth:`~aiohttp.ClientWebSocketResponse.receive` operation was not terminated. + :py:meth:`~aiohttp.ClientWebSocketResponse.receive` operation did not terminate. This change causes `_pong_not_received` to feed the `reader` an error message, causing pending `receive` to terminate and return the error message. The error message contains the exception :py:class:`~aiohttp.ServerTimeoutError`. From 7f8ef98585141f2986e53d31e57db1830b2abac1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 30 Jul 2024 22:42:51 -0500 Subject: [PATCH 6/6] Update CHANGES/8540.bugfix.rst --- CHANGES/8540.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/8540.bugfix.rst b/CHANGES/8540.bugfix.rst index 78a0b45a34c..ab7c4767635 100644 --- a/CHANGES/8540.bugfix.rst +++ b/CHANGES/8540.bugfix.rst @@ -1,4 +1,4 @@ -WSS: Fix heartbeat timeout logic -- by :user:`arcivanov`. +Fixed WebSocket server heartbeat timeout logic to terminate `receive` and return :py:class:`~aiohttp.ServerTimeoutError` -- by :user:`arcivanov`. When a WebSocket pong message was not received, the :py:meth:`~aiohttp.ClientWebSocketResponse.receive` operation did not terminate.