Entire hypercorn server crashes when receiving trailing data on a websocket upgrade request

This script sets up a trivial hypercorn server that just sleeps when a request arrives, and a client that sends a Upgrade: websocket request that has an extra byte of data (x) after the request. Receiving this request causes the server to crash, i.e. the entire server process exits immediately.

import sys
import socket
import trio
from hypercorn.config import Config
from hypercorn.trio import serve

def server():
    async def app(scope, receive, send) -> None:
        print(f"{scope=}")
        print(f"{await receive()=}")
        if scope["type"] == "websocket":
            await trio.sleep(float("inf"))

    async def main():
        await serve(app, Config())

    trio.run(main, strict_exception_groups=True)

def client():
    s = socket.create_connection(("127.0.0.1", 8000))
    s.sendall(b"""GET / HTTP/1.1
Host: 127.0.0.1:8000
Connection: Upgrade
Upgrade: websocket
Sec-WebSocket-Version: 13
Sec-WebSocket-Key: bKdPyn3u98cTfZJSh4TNeQ==

x
""")

if __name__ == "__main__":
    globals()[sys.argv[1]]()

(py311) sodium-a njs@Nathaniel-Smith-MacBook aws-crash % python t.py server
scope={'type': 'lifespan', 'asgi': {'spec_version': '2.0', 'version': '3.0'}}
await receive()={'type': 'lifespan.startup'}
[2024-05-08 16:23:42 -0700] [46547] [INFO] Running on http://127.0.0.1:8000 (CTRL + C to quit)

[... run 'python t.py client' in another terminal ...]

scope={'type': 'websocket', 'asgi': {'spec_version': '2.3', 'version': '3.0'}, 'scheme': 'ws', 'http_version': '1.1', 'path': '/', 'raw_path': b'/', 'query_string': b'', 'root_path': '', 'headers': [(b'host', b'127.0.0.1:8000'), (b'connection', b'Upgrade'), (b'upgrade', b'websocket'), (b'sec-websocket-version', b'13'), (b'sec-websocket-key', b'bKdPyn3u98cTfZJSh4TNeQ==')], 'client': ('127.0.0.1', 58161), 'server': ('127.0.0.1', 8000), 'subprotocols': [], 'extensions': {'websocket.http.response': {}}}
  + Exception Group Traceback (most recent call last):
  |   File "/Users/njs/aws-crash/t.py", line 33, in <module>
  |     globals()[sys.argv[1]]()
  |   File "/Users/njs/aws-crash/t.py", line 17, in server
  |     trio.run(main, strict_exception_groups=True)
  |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/trio/_core/_run.py", line 2093, in run
  |     raise runner.main_task_outcome.error
  |   File "/Users/njs/aws-crash/t.py", line 15, in main
  |     await serve(app, Config())
  |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/trio/__init__.py", line 47, in serve
  |     await worker_serve(
  |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/trio/run.py", line 46, in worker_serve
  |     async with trio.open_nursery() as lifespan_nursery:
  |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/trio/_core/_run.py", line 881, in __aexit__
  |     raise combined_error_from_nursery
  | trio.NonBaseMultiError: <MultiError: <MultiError: AttributeError("'WSStream' object has no attribute 'connection'")>>
  +-+---------------- 1 ----------------
    | Exception Group Traceback (most recent call last):
    |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/trio/run.py", line 50, in worker_serve
    |     async with trio.open_nursery() as server_nursery:
    |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/trio/_core/_run.py", line 881, in __aexit__
    |     raise combined_error_from_nursery
    | trio.NonBaseMultiError: <MultiError: AttributeError("'WSStream' object has no attribute 'connection'")>
    +-+---------------- 1 ----------------
      | Exception Group Traceback (most recent call last):
      |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/trio/_highlevel_serve_listeners.py", line 25, in _run_handler
      |     await handler(stream)
      |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/trio/tcp_server.py", line 55, in run
      |     async with TaskGroup() as task_group:
      |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/trio/task_group.py", line 76, in __aexit__
      |     await self._nursery_manager.__aexit__(exc_type, exc_value, tb)
      |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/trio/_core/_run.py", line 881, in __aexit__
      |     raise combined_error_from_nursery
      | trio.NonBaseMultiError: AttributeError("'WSStream' object has no attribute 'connection'")
      +-+---------------- 1 ----------------
        | Traceback (most recent call last):
        |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/trio/tcp_server.py", line 70, in run
        |     await self._read_data()
        |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/trio/tcp_server.py", line 109, in _read_data
        |     await self.protocol.handle(RawData(data))
        |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/protocol/__init__.py", line 62, in handle
        |     return await self.protocol.handle(event)
        |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/protocol/h11.py", line 115, in handle
        |     await self._handle_events()
        |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/protocol/h11.py", line 184, in _handle_events
        |     await self.stream.handle(event)
        |   File "/opt/homebrew/Caskroom/miniforge/base/envs/py311/lib/python3.11/site-packages/hypercorn/protocol/ws_stream.py", line 236, in handle
        |     self.connection.receive_data(event.data)
        |     ^^^^^^^^^^^^^^^
        | AttributeError: 'WSStream' object has no attribute 'connection'
        +------------------------------------

So IMO there's really two issues here:

A bug in the websocket state machine where it switches to websocket mode as soon as the request arrives, but it doesn't finish setting up the websocket mode until the response is sent, so any data that arrives in the mean time gets delivered to a half-initialized state machine that crashes.
That wouldn't be so bad -- it'd be nice to handle it more gracefully, but this isn't really a thing that happens in real life and it's fine to error out the connection -- but there's a lack of worst-case error handling in the latest hypercorn release that causes this exception to propagate all the way out and crash the entire server, not just the offending connection handler.

pgjones / hypercorn

Entire hypercorn server crashes when receiving trailing data on a websocket upgrade request #225