Skip to content

Commit

Permalink
Fix request count check for BadHttpMethod (#10076)
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco authored Dec 1, 2024
1 parent 0d6a83e commit fad44f6
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGES/10055.bugfix.rst
1 change: 1 addition & 0 deletions CHANGES/10076.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed invalid method logging unexpected being logged at exception level on subsequent connections -- by :user:`bdraco`.
7 changes: 2 additions & 5 deletions aiohttp/web_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,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[_Request] = None
Expand Down Expand Up @@ -713,11 +714,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
Expand Down
2 changes: 2 additions & 0 deletions aiohttp/web_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ def __init__(
self._loop = asyncio.get_running_loop()
self._connections: Dict[RequestHandler[_Request], 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 # type: ignore[assignment]
Expand Down
14 changes: 14 additions & 0 deletions tests/test_web_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,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(
Expand Down

0 comments on commit fad44f6

Please sign in to comment.