Skip to content

Commit

Permalink
Fix recv() errors when closing during handshake
Browse files Browse the repository at this point in the history
Fixes encode#244. The unit test already existed, but it logged exceptions that
weren't being noticed.

* websockets_impl: `transfer_data_task` is unset if the handshake fails,
  so we can't call recv().
* wsproto_impl: `h11.Reject` is invalid during handshake; send
  `RejectConnection` instead.
  • Loading branch information
adamhooper committed Jun 19, 2020
1 parent 6757386 commit f608081
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 5 deletions.
7 changes: 6 additions & 1 deletion tests/protocols/test_websocket.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import asyncio
import functools
import logging
import threading
import time
from contextlib import contextmanager
Expand Down Expand Up @@ -118,7 +119,9 @@ async def open_connection(url):


@pytest.mark.parametrize("protocol_cls", WS_PROTOCOLS)
def test_close_connection(protocol_cls):
def test_close_connection(protocol_cls, caplog):
caplog.set_level(logging.ERROR)

class App(WebSocketResponse):
async def websocket_connect(self, message):
await self.send({"type": "websocket.close"})
Expand All @@ -136,6 +139,8 @@ async def open_connection(url):
assert not is_open
loop.close()

assert not [r.message for r in caplog.records]


@pytest.mark.parametrize("protocol_cls", WS_PROTOCOLS)
def test_headers(protocol_cls):
Expand Down
7 changes: 6 additions & 1 deletion uvicorn/protocols/websockets/websockets_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ def __init__(self, config, server_state, _loop=None):
self.initial_response = None
self.connect_sent = False
self.accepted_subprotocol = None
self.transfer_data_task = None

self.ws_server = Server()

Expand Down Expand Up @@ -230,6 +229,12 @@ async def asgi_receive(self):
return {"type": "websocket.connect"}

await self.handshake_completed_event.wait()
if not hasattr(self, "transfer_data_task"):
# If the handshake completed and there's no data-transfer task,
# that means the handshake failed.
assert self.closed_event.is_set()
return {"type": "websocket.disconnect", "code": self.initial_response[0]}

try:
data = await self.recv()
except websockets.ConnectionClosed as exc:
Expand Down
4 changes: 1 addition & 3 deletions uvicorn/protocols/websockets/wsproto_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,8 @@ async def send(self, message):
)
self.handshake_complete = True
self.close_sent = True
msg = h11.Response(status_code=403, headers=[])
msg = events.RejectConnection(status_code=403, headers=[])
output = self.conn.send(msg)
msg = h11.EndOfMessage()
output += self.conn.send(msg)
self.transport.write(output)
self.transport.close()

Expand Down

0 comments on commit f608081

Please sign in to comment.