Add new NotHttpProtocol exception to differentiate invalid methods and gibberish

CHANGES/10067.misc.rst

@@ -0,0 +1 @@
+Added a new ``NotHttpProtocol`` internal exception to better differentiate invalid HTTP methods and gibberish on the wire -- by :user:`bdraco`.

aiohttp/_http_parser.pyx

@@ -29,6 +29,7 @@ from .http_exceptions import (
     InvalidHeader,
     InvalidURLError,
     LineTooLong,
+    NotHttpProtocol,
     PayloadEncodingError,
     TransferEncodingError,
 )
@@ -833,6 +834,8 @@ cdef parser_error_from_errno(cparser.llhttp_t* parser, data, pointer):
                  cparser.HPE_INVALID_TRANSFER_ENCODING}:
         return BadHttpMessage(err_msg)
     elif errno == cparser.HPE_INVALID_METHOD:
+        if b" " not in data:
+            return NotHttpProtocol(err_msg)
         return BadHttpMethod(error=err_msg)
     elif errno in {cparser.HPE_INVALID_STATUS,
                    cparser.HPE_INVALID_VERSION}:

aiohttp/http_exceptions.py

@@ -104,5 +104,11 @@ def __init__(self, line: str = "", error: Optional[str] = None) -> None:
         super().__init__(line, error or f"Bad HTTP method in status line {line!r}")
 
 
+class NotHttpProtocol(BadStatusLine):
+
+    def __init__(self, line: str = "", error: Optional[str] = None) -> None:
+        super().__init__(line, error or f"Not HTTP protocol {line!r}")
+
+
 class InvalidURLError(BadHttpMessage):
     pass

aiohttp/http_parser.py

@@ -45,6 +45,7 @@
     InvalidHeader,
     InvalidURLError,
     LineTooLong,
+    NotHttpProtocol,
     TransferEncodingError,
 )
 from .http_writer import HttpVersion, HttpVersion10
@@ -565,7 +566,7 @@ def parse_message(self, lines: List[bytes]) -> RawRequestMessage:
         try:
             method, path, version = line.split(" ", maxsplit=2)
         except ValueError:
-            raise BadHttpMethod(line) from None
+            raise NotHttpProtocol(line) from None
 
         if len(path) > self.max_line_size:
             raise LineTooLong(

aiohttp/web_protocol.py

@@ -35,7 +35,7 @@
     RawRequestMessage,
     StreamWriter,
 )
-from .http_exceptions import BadHttpMethod
+from .http_exceptions import NotHttpProtocol
 from .log import access_logger, server_logger
 from .streams import EMPTY_PAYLOAD, StreamReader
 from .tcp_helpers import tcp_keepalive
@@ -716,9 +716,9 @@ def handle_error(
         if (
             self._manager
             and self._manager.requests_count == 1
-            and isinstance(exc, BadHttpMethod)
+            and isinstance(exc, NotHttpProtocol)
         ):
-            # BadHttpMethod is common when a client sends non-HTTP
+            # NotHttpProtocol 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
             # it at the debug level as to not fill logs with noise.

tests/test_http_parser.py

@@ -992,6 +992,25 @@ def test_http_request_parser_bad_method(
         parser.feed_data(rfc9110_5_6_2_token_delim + b'ET" /get HTTP/1.1\r\n\r\n')
 
 
+@pytest.mark.parametrize(
+    "not_http_protocol_data",
+    [
+        b"\x16\x03\x03\x01F\x01",
+        b"\x16\x03\x01",
+        b"\x16\x03\x01\x02",
+        b"\x16",
+    ],
+)
+def test_http_request_parser_not_http_protocol(
+    parser: HttpRequestParser, not_http_protocol_data: bytes
+) -> None:
+    with pytest.raises(http_exceptions.NotHttpProtocol):
+        # The \r\n\r\n is to ensure that the py parser gets to the
+        # point where it tries to parse the protocol as it differs
+        # from the C parser.
+        parser.feed_data(not_http_protocol_data + b"\r\n\r\n")
+
+
 def test_http_request_parser_bad_version(parser: HttpRequestParser) -> None:
     with pytest.raises(http_exceptions.BadHttpMessage):
         parser.feed_data(b"GET //get HT/11\r\n\r\n")

tests/test_web_server.py

@@ -7,7 +7,7 @@
 import pytest
 
 from aiohttp import client, web
-from aiohttp.http_exceptions import BadHttpMethod, BadStatusLine
+from aiohttp.http_exceptions import BadStatusLine, NotHttpProtocol
 from aiohttp.pytest_plugin import AiohttpClient, AiohttpRawServer
 
 
@@ -70,12 +70,12 @@ async def handler(request: web.BaseRequest) -> NoReturn:
     logger.exception.assert_called_with("Error handling request", exc_info=exc)
 
 
-async def test_raw_server_logs_invalid_method_with_loop_debug(
+async def test_raw_server_logs_non_http_protocol_with_loop_debug(
     aiohttp_raw_server: AiohttpRawServer,
     aiohttp_client: AiohttpClient,
     loop: asyncio.AbstractEventLoop,
 ) -> None:
-    exc = BadHttpMethod(b"\x16\x03\x03\x01F\x01".decode(), "error")
+    exc = NotHttpProtocol(b"\x16\x03\x03\x01F\x01".decode(), "error")
 
     async def handler(request: web.BaseRequest) -> NoReturn:
         raise exc
@@ -99,12 +99,12 @@ async def handler(request: web.BaseRequest) -> NoReturn:
     logger.debug.assert_called_with("Error handling request", exc_info=exc)
 
 
-async def test_raw_server_logs_invalid_method_without_loop_debug(
+async def test_raw_server_logs_non_http_protocol_without_loop_debug(
     aiohttp_raw_server: AiohttpRawServer,
     aiohttp_client: AiohttpClient,
     loop: asyncio.AbstractEventLoop,
 ) -> None:
-    exc = BadHttpMethod(b"\x16\x03\x03\x01F\x01".decode(), "error")
+    exc = NotHttpProtocol(b"\x16\x03\x03\x01F\x01".decode(), "error")
 
     async def handler(request: web.BaseRequest) -> NoReturn:
         raise exc
@@ -128,12 +128,12 @@ async def handler(request: web.BaseRequest) -> NoReturn:
     logger.debug.assert_called_with("Error handling request", exc_info=exc)
 
 
-async def test_raw_server_logs_invalid_method_second_request(
+async def test_raw_server_logs_non_http_protocol_second_request(
     aiohttp_raw_server: AiohttpRawServer,
     aiohttp_client: AiohttpClient,
     loop: asyncio.AbstractEventLoop,
 ) -> None:
-    exc = BadHttpMethod(b"\x16\x03\x03\x01F\x01".decode(), "error")
+    exc = NotHttpProtocol(b"\x16\x03\x03\x01F\x01".decode(), "error")
     request_count = 0
 
     async def handler(request: web.BaseRequest) -> web.Response: