Hung connection in failed upgrade edge case

[Dreamsorcerer]

Describe the bug

This was submitted as a security issue, but after investigation is not a security issue.

The result here is that if a client pipelines another request after a websocket upgrade request, and the upgrade fails in the handler, then the second request gets silently discarded and the client connection will end up hanging.

In reality, a client should never pipeline a second request as they need to wait for the response to know if the second request should be HTTP or WebSocket. So, this is a weird edge case that probably never actually happens.

Will need to figure out a test that reproduces the issue first.

To Reproduce

Step 1: Initial parse stashes pipelined bytes (web_protocol.py:439-470)

aiohttp/web_protocol.py:439-466

def data_received(self, data: bytes) -> None:
if self._force_close or self._close:
return
messages: Sequence[_MsgType]
if self._payload_parser is None and not self._upgraded:
assert self._parser is not None
try:
messages, upgraded, tail = self._parser.feed_data(data)
except HttpProcessingError as exc:
messages = [(_ErrInfo(status=400, exc=exc, message=exc.message),
EMPTY_PAYLOAD)]
upgraded = False
tail = b""

    for msg, payload in messages or ():
        self._request_count += 1
        self._messages.append((msg, payload))

    waiter = self._waiter
    if messages and waiter is not None and not waiter.done():
        waiter.set_result(None)

    self._upgraded = upgraded
    if upgraded and tail:
        self._message_tail = tail        # <-- pipelined bytes parked

On a request that the parser detects as an Upgrade (e.g. Connection: Upgrade + Upgrade: websocket), feed_data returns upgraded=True. Any bytes after the upgrade request's \r\n\r\n come back in tail, which is stashed in self._message_tail for replay after the handler runs.

Step 2: Handler fails the upgrade (web_protocol.py:550-572, web_ws.py:275-319)

The user handler — typically WebSocketResponse.prepare() — raises HTTPBadRequest when handshake validation fails: bad Sec-WebSocket-Version, missing/invalid Sec-WebSocket-Key, bad subprotocol, or any custom validation. The exception is caught in _handle_request:

aiohttp/web_protocol.py:557-562 (abridged)

except HTTPException as exc:
resp = exc
resp, reset = await self.finish_response(request, resp, start_time)

The response is built with default keep_alive=True (only the generic 500/504 path in handle_error sets force_close()), so the connection loop continues.

Step 3: finish_response replays _message_tail and discards the result — THE BUG (web_protocol.py:730-735)

aiohttp/web_protocol.py:730-735

if self._parser is not None:
self._parser.set_upgraded(False)
self._upgraded = False
if self._message_tail:
self._parser.feed_data(self._message_tail) # <-- return value DISCARDED
self._message_tail = b""

Contrast with data_received at line 447, which captures the same three-tuple messages, upgraded, tail and appends messages to self._messages. Here, the pipelined request inside _message_tail is parsed into a RawRequestMessage + StreamReader — but the tuple is dropped on the floor. The StreamReader is no longer reachable; the message never reaches self._messages; the parser's internal payload state may still be set (if the pipelined request has a body), but no consumer exists to drain it.

Step 4: start() blocks on _waiter forever (web_protocol.py:638-712 — main loop)

aiohttp/web_protocol.py (abridged main loop)

while not self._force_close:
if not self._messages:
try:
await self._waiter # <-- blocked
except asyncio.CancelledError:
break
msg, payload = self._messages.popleft()
...

_waiter is resolved only when data_received queues a new message. The pipelined message was already consumed (Step 3 dropped it), and data_received will only fire again on new bytes from the peer. A peer that simply stops writing keeps the loop blocked until _keepalive_timeout (default 3630 seconds ≈ 1 hour) closes the connection via _process_keepalive.