From f54608dcba59ed8758f5ae9e954fbf6a1b3069e3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 1 Dec 2024 15:08:30 -0600 Subject: [PATCH] Fix request count check for BadHttpMethod (#10076) (cherry picked from commit fad44f6bc96e4f54a5b8e7e30d1e9e01a021bec9) --- CHANGES/10055.bugfix.rst | 1 + CHANGES/10076.bugfix.rst | 1 + aiohttp/web_protocol.py | 7 ++----- aiohttp/web_server.py | 2 ++ tests/test_web_server.py | 14 ++++++++++++++ 5 files changed, 20 insertions(+), 5 deletions(-) create mode 120000 CHANGES/10055.bugfix.rst create mode 100644 CHANGES/10076.bugfix.rst diff --git a/CHANGES/10055.bugfix.rst b/CHANGES/10055.bugfix.rst new file mode 120000 index 00000000000..b1f45d8b887 --- /dev/null +++ b/CHANGES/10055.bugfix.rst @@ -0,0 +1 @@ +10076.bugfix.rst \ No newline at end of file diff --git a/CHANGES/10076.bugfix.rst b/CHANGES/10076.bugfix.rst new file mode 100644 index 00000000000..c577366bbe8 --- /dev/null +++ b/CHANGES/10076.bugfix.rst @@ -0,0 +1 @@ +Fixed invalid method logging unexpected being logged at exception level on subsequent connections -- by :user:`bdraco`. diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index fe2ae8a1269..e8bb41abf97 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -192,6 +192,7 @@ def __init__( ): super().__init__(loop) + # _request_count is the number of requests processed with the same connection. self._request_count = 0 self._keepalive = False self._current_request: Optional[BaseRequest] = None @@ -688,11 +689,7 @@ def handle_error( Returns HTTP response with specific status code. Logs additional information. It always closes current connection. """ - if ( - self._manager - and self._manager.requests_count == 1 - and isinstance(exc, BadHttpMethod) - ): + if self._request_count == 1 and isinstance(exc, BadHttpMethod): # BadHttpMethod is common when a client sends non-HTTP # or encrypted traffic to an HTTP port. This is expected # to happen when connected to the public internet so we log diff --git a/aiohttp/web_server.py b/aiohttp/web_server.py index b6ac25ac1a5..328aca1e405 100644 --- a/aiohttp/web_server.py +++ b/aiohttp/web_server.py @@ -25,6 +25,8 @@ def __init__( self._loop = loop or asyncio.get_running_loop() self._connections: Dict[RequestHandler, asyncio.Transport] = {} self._kwargs = kwargs + # requests_count is the number of requests being processed by the server + # for the lifetime of the server. self.requests_count = 0 self.request_handler = handler self.request_factory = request_factory or self._make_request diff --git a/tests/test_web_server.py b/tests/test_web_server.py index e77df623020..7b9b87a374a 100644 --- a/tests/test_web_server.py +++ b/tests/test_web_server.py @@ -86,6 +86,20 @@ async def handler(request: web.BaseRequest) -> NoReturn: # be probing for TLS/SSL support which is # expected to fail logger.debug.assert_called_with("Error handling request", exc_info=exc) + logger.debug.reset_mock() + + # Now make another connection to the server + # to make sure that the exception is logged + # at debug on a second fresh connection + cli2 = await aiohttp_client(server) + resp = await cli2.get("/path/to") + assert resp.status == 500 + assert resp.headers["Content-Type"].startswith("text/plain") + # BadHttpMethod should be logged as debug + # on the first request since the client may + # be probing for TLS/SSL support which is + # expected to fail + logger.debug.assert_called_with("Error handling request", exc_info=exc) async def test_raw_server_logs_invalid_method_without_loop_debug(