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 #10076/fad44f6b backport][3.12] Fix request count check for BadHttpMethod #10082

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/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 @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions aiohttp/web_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 @@ -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(
Expand Down
Loading