Reply with HTTP 405 to non-GET handshake requests.

Fix #1677.

Author: aaugustin

docs/project/changelog.rst

@@ -40,6 +40,13 @@ Backwards-incompatible changes
 
     websockets 16.1 is the last version supporting Python 3.10.
 
+.. admonition:: ``process_request`` may receive requests using an HTTP method
+    other than ``GET``.
+    :class: important
+
+    Previously, the server closed the connection without returning an HTTP
+    response. Now, ``process_request`` runs and can return an HTTP response.
+
 .. admonition:: In the :mod:`threading` implementation, the ``socket`` argument
     is renamed to ``sock``.
     :class: note
@@ -64,6 +71,9 @@ New features
 Improvements
 ............
 
+* Replied with HTTP 405 Method Not Allowed when the handshake request doesn't
+  use the GET method, instead of closing the connection.
+
 * Added the ``reconnect_delays`` argument for customizing the delays between
   reconnection attempts in :func:`~asyncio.client.connect`, beyond existing
   ``WEBSOCKETS_BACKOFF_*`` environment variables.

src/websockets/__init__.py

@@ -40,6 +40,7 @@
     "InvalidHeaderFormat",
     "InvalidHeaderValue",
     "InvalidMessage",
+    "InvalidMethod",
     "InvalidOrigin",
     "InvalidParameterName",
     "InvalidParameterValue",
@@ -105,6 +106,7 @@
         InvalidHeaderFormat,
         InvalidHeaderValue,
         InvalidMessage,
+        InvalidMethod,
         InvalidOrigin,
         InvalidParameterName,
         InvalidParameterValue,
@@ -171,6 +173,7 @@
             "InvalidHeaderFormat": ".exceptions",
             "InvalidHeaderValue": ".exceptions",
             "InvalidMessage": ".exceptions",
+            "InvalidMethod": ".exceptions",
             "InvalidOrigin": ".exceptions",
             "InvalidParameterName": ".exceptions",
             "InvalidParameterValue": ".exceptions",

src/websockets/exceptions.py

@@ -13,6 +13,7 @@
             * :exc:`InvalidProxyMessage`
             * :exc:`InvalidProxyStatus`
         * :exc:`InvalidMessage`
+        * :exc:`InvalidMethod`
         * :exc:`InvalidStatus`
         * :exc:`InvalidStatusCode` (legacy)
         * :exc:`InvalidHeader`
@@ -53,6 +54,7 @@
     "InvalidProxyMessage",
     "InvalidProxyStatus",
     "InvalidMessage",
+    "InvalidMethod",
     "InvalidStatus",
     "InvalidHeader",
     "InvalidHeaderFormat",
@@ -242,6 +244,19 @@ class InvalidMessage(InvalidHandshake):
     """
 
 
+class InvalidMethod(InvalidHandshake):
+    """
+    Raised when a handshake request doesn't use HTTP GET.
+
+    """
+
+    def __init__(self, method: str) -> None:
+        self.method = method
+
+    def __str__(self) -> str:
+        return f"unsupported HTTP method: {self.method}"
+
+
 class InvalidStatus(InvalidHandshake):
     """
     Raised when a handshake response rejects the WebSocket upgrade.

src/websockets/http11.py

@@ -83,10 +83,14 @@ class Request:
     Attributes:
         path: Request path, including optional query.
         headers: Request headers.
+        method: Request method; WebSocket handshake requests use GET.
     """
 
     path: str
     headers: Headers
+    # method comes before path in the HTTP request, but it has a default,
+    # so it must be declared after path and headers which don't have one.
+    method: str = "GET"
     # body isn't useful is the context of this library.
 
     _exception: Exception | None = None
@@ -109,14 +113,12 @@ def parse(
 
         This is a generator-based coroutine.
 
-        The request path isn't URL-decoded or validated in any way.
+        The request method, path, and headers are expected to contain only ASCII
+        characters. The request path isn't URL-decoded or validated in any way.
 
-        The request path and headers are expected to contain only ASCII
-        characters. Other characters are represented with surrogate escapes.
-
-        :meth:`parse` doesn't attempt to read the request body because
-        WebSocket handshake requests don't have one. If the request contains a
-        body, it may be read from the data stream after :meth:`parse` returns.
+        :meth:`parse` doesn't read the request body because WebSocket handshake
+        requests don't have one. If the request contains a body, it may be read
+        from the data stream after :meth:`parse` returns.
 
         Args:
             read_line: Generator-based coroutine that reads a LF-terminated
@@ -130,25 +132,24 @@ def parse(
         """
         # https://datatracker.ietf.org/doc/html/rfc7230#section-3.1.1
 
-        # Parsing is simple because fixed values are expected for method and
-        # version and because path isn't checked. Since WebSocket software tends
-        # to implement HTTP/1.1 strictly, there's little need for lenient parsing.
+        # Parsing is simple because a fixed value is expected for the version
+        # and because path isn't checked. Since WebSocket libraries generally
+        # implement HTTP/1.1 strictly, there's little need for lenient parsing.
 
         try:
             request_line = yield from parse_line(read_line)
         except EOFError as exc:
             raise EOFError("connection closed while reading HTTP request line") from exc
 
         try:
-            method, raw_path, protocol = request_line.split(b" ", 2)
+            raw_method, raw_path, protocol = request_line.split(b" ", 2)
         except ValueError:  # not enough values to unpack (expected 3, got 1-2)
             raise ValueError(f"invalid HTTP request line: {d(request_line)}") from None
         if protocol != b"HTTP/1.1":
             raise ValueError(
                 f"unsupported protocol; expected HTTP/1.1: {d(request_line)}"
             )
-        if method != b"GET":
-            raise ValueError(f"unsupported HTTP method; expected GET; got {d(method)}")
+        method = raw_method.decode("ascii")
 
         # RFC 9110 defers the definition of URIs to RFC 3986, which allows only
         # a subset of ASCII. Non-ASCII IRIs must be UTF-8 then percent-encoded.
@@ -167,7 +168,7 @@ def parse(
             if int(headers["Content-Length"]) != 0:
                 raise ValueError("unsupported request body")
 
-        return cls(path, headers)
+        return cls(path, headers, method)
 
     def serialize(self) -> bytes:
         """
@@ -176,7 +177,7 @@ def serialize(self) -> bytes:
         """
         # Since the request line and headers only contain ASCII characters,
         # we can keep this simple.
-        request = f"GET {self.path} HTTP/1.1\r\n".encode()
+        request = f"{self.method} {self.path} HTTP/1.1\r\n".encode()
         request += self.headers.serialize()
         return request
 

src/websockets/server.py

@@ -15,6 +15,7 @@
     InvalidHeader,
     InvalidHeaderValue,
     InvalidMessage,
+    InvalidMethod,
     InvalidOrigin,
     InvalidUpgrade,
     NegotiationError,
@@ -147,6 +148,17 @@ def accept(self, request: Request) -> Response:
                 http.HTTPStatus.FORBIDDEN,
                 f"Failed to open a WebSocket connection: {exc}.\n",
             )
+        except InvalidMethod as exc:
+            request._exception = exc
+            self.handshake_exc = exc
+            if self.debug:
+                self.logger.debug("! invalid method", exc_info=True)
+            response = self.reject(
+                http.HTTPStatus.METHOD_NOT_ALLOWED,
+                f"Failed to open a WebSocket connection: {exc}.\n",
+            )
+            response.headers["Allow"] = "GET"
+            return response
         except InvalidUpgrade as exc:
             request._exception = exc
             self.handshake_exc = exc
@@ -209,10 +221,9 @@ def process_request(
         """
         Check a handshake request and negotiate extensions and subprotocol.
 
-        This function doesn't verify that the request is an HTTP/1.1 or higher
-        GET request and doesn't check the ``Host`` header. These controls are
-        usually performed earlier in the HTTP request handling code. They're
-        the responsibility of the caller.
+        This function doesn't verify that that it's an HTTP/1.1 request and
+        doesn't check the ``Host`` header. These controls must be performed
+        by the HTTP stack earlier. They're the responsibility of the caller.
 
         Args:
             request: WebSocket handshake request received from the client.
@@ -222,10 +233,15 @@ def process_request(
             ``Sec-WebSocket-Protocol`` headers for the handshake response.
 
         Raises:
+            InvalidMethod: If the request method isn't GET; then the
+                server must return a 405 Method Not Allowed error.
             InvalidHandshake: If the handshake request is invalid;
                 then the server must return 400 Bad Request error.
 
         """
+        if request.method != "GET":
+            raise InvalidMethod(request.method)
+
         headers = request.headers
 
         connection: list[ConnectionOption] = sum(
@@ -558,7 +574,7 @@ def parse(self) -> Generator[None]:
                 yield
 
             if self.debug:
-                self.logger.debug("< GET %s HTTP/1.1", request.path)
+                self.logger.debug("< %s %s HTTP/1.1", request.method, request.path)
                 for key, value in request.headers.raw_items():
                     self.logger.debug("< %s: %s", key, value)
 

tests/test_exceptions.py

@@ -103,6 +103,10 @@ def test_str(self):
                 InvalidMessage("malformed HTTP message"),
                 "malformed HTTP message",
             ),
+            (
+                InvalidMethod("POST"),
+                "unsupported HTTP method: POST",
+            ),
             (
                 InvalidStatus(Response(401, "Unauthorized", Headers())),
                 "server rejected WebSocket connection: HTTP 401",

tests/test_http11.py

@@ -29,9 +29,16 @@ def test_parse(self):
             b"\r\n"
         )
         request = self.assertGeneratorReturns(self.parse())
+        self.assertEqual(request.method, "GET")
         self.assertEqual(request.path, "/chat")
         self.assertEqual(request.headers["Upgrade"], "websocket")
 
+    def test_parse_non_get_method(self):
+        self.reader.feed_data(b"OPTIONS * HTTP/1.1\r\n\r\n")
+        request = self.assertGeneratorReturns(self.parse())
+        self.assertEqual(request.method, "OPTIONS")
+        self.assertEqual(request.path, "*")
+
     def test_parse_empty(self):
         self.reader.feed_eof()
         with self.assertRaises(EOFError) as raised:
@@ -59,15 +66,6 @@ def test_parse_unsupported_protocol(self):
             "unsupported protocol; expected HTTP/1.1: GET /chat HTTP/1.0",
         )
 
-    def test_parse_unsupported_method(self):
-        self.reader.feed_data(b"OPTIONS * HTTP/1.1\r\n\r\n")
-        with self.assertRaises(ValueError) as raised:
-            next(self.parse())
-        self.assertEqual(
-            str(raised.exception),
-            "unsupported HTTP method; expected GET; got OPTIONS",
-        )
-
     def test_parse_invalid_header(self):
         self.reader.feed_data(b"GET /chat HTTP/1.1\r\nOops\r\n")
         with self.assertRaises(ValueError) as raised:

tests/test_server.py

@@ -9,6 +9,7 @@
 from websockets.exceptions import (
     InvalidHeader,
     InvalidMessage,
+    InvalidMethod,
     InvalidOrigin,
     InvalidUpgrade,
     NegotiationError,
@@ -357,6 +358,22 @@ def test_basic(self):
 
         self.assertHandshakeSuccess(server)
 
+    def test_invalid_method(self):
+        """Handshake fails when the request method isn't GET."""
+        server = ServerProtocol()
+        request = make_request()
+        request.method = "POST"
+        response = server.accept(request)
+        server.send_response(response)
+
+        self.assertEqual(response.status_code, 405)
+        self.assertEqual(response.headers["Allow"], "GET")
+        self.assertHandshakeError(
+            server,
+            InvalidMethod,
+            "unsupported HTTP method: POST",
+        )
+
     def test_missing_connection(self):
         """Handshake fails when the Connection header is missing."""
         server = ServerProtocol()