From 46ba7572c0d73d35ce39a0163ff71c9a377f93e5 Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Thu, 21 Apr 2022 05:34:17 +0000 Subject: [PATCH 01/13] feat(websocket): add support for reason in WebSocket.close() --- falcon/asgi/app.py | 9 ++++++++- falcon/asgi/ws.py | 27 ++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/falcon/asgi/app.py b/falcon/asgi/app.py index 5f8cc4c90..3b4c159cd 100644 --- a/falcon/asgi/app.py +++ b/falcon/asgi/app.py @@ -976,7 +976,13 @@ async def _handle_websocket(self, ver, scope, receive, send): # we don't support, so bail out. This also fulfills the ASGI # spec requirement to only process the request after # receiving and verifying the first event. - await send({'type': EventType.WS_CLOSE, 'code': WSCloseCode.SERVER_ERROR}) + await send( + { + 'type': EventType.WS_CLOSE, + 'code': WSCloseCode.SERVER_ERROR, + 'reason': 'Internal Server Error', + } + ) return req = self._request_type(scope, receive, options=self.req_options) @@ -988,6 +994,7 @@ async def _handle_websocket(self, ver, scope, receive, send): send, self.ws_options.media_handlers, self.ws_options.max_receive_queue, + self.ws_options.default_close_reasons, ) on_websocket = None diff --git a/falcon/asgi/ws.py b/falcon/asgi/ws.py index 19dd2f2f1..d3414aeb0 100644 --- a/falcon/asgi/ws.py +++ b/falcon/asgi/ws.py @@ -49,6 +49,7 @@ class WebSocket: '_asgi_send', '_buffered_receiver', '_close_code', + '_close_reasons', '_supports_accept_headers', '_mh_bin_deserialize', '_mh_bin_serialize', @@ -69,6 +70,7 @@ def __init__( Union[media.BinaryBaseHandlerWS, media.TextBaseHandlerWS], ], max_receive_queue: int, + default_close_reasons: Dict[Optional[int], str], ): self._supports_accept_headers = ver != '2.0' @@ -94,6 +96,7 @@ def __init__( self._mh_bin_serialize = mh_bin.serialize self._mh_bin_deserialize = mh_bin.deserialize + self._close_reasons = default_close_reasons self._state = _WebSocketState.HANDSHAKE self._close_code = None # type: Optional[int] @@ -257,10 +260,18 @@ async def close(self, code: Optional[int] = None) -> None: if self.closed: return + if code in self._close_reasons: + reason = self._close_reasons[code] + elif 3100 <= code <= 3999: + reason = falcon.util.code_to_http_status(code - 3000) + else: + reason = '' + await self._asgi_send( { 'type': EventType.WS_CLOSE, 'code': code, + 'reason': reason, } ) @@ -512,6 +523,9 @@ class WebSocketOptions: unhandled error is raised while handling a WebSocket connection (default ``1011``). For a list of valid close codes and ranges, see also: https://tools.ietf.org/html/rfc6455#section-7.4 + default_close_reasons (dict): A default mapping between the Websocket + close code and the reason why the connection is close. HTTPerrors + are not included and will be rendered automatically with HTTP status. media_handlers (dict): A dict-like object for configuring media handlers according to the WebSocket payload type (TEXT vs. BINARY) of a given message. See also: :ref:`ws_media_handlers`. @@ -528,7 +542,12 @@ class WebSocketOptions: """ - __slots__ = ['error_close_code', 'max_receive_queue', 'media_handlers'] + __slots__ = [ + 'error_close_code', + 'default_close_reasons', + 'max_receive_queue', + 'media_handlers', + ] def __init__(self): try: @@ -557,6 +576,12 @@ def __init__(self): # self.error_close_code: int = WSCloseCode.SERVER_ERROR + self.default_close_reasons: Dict[int, str] = { + 1000: 'Normal Closure', + 1011: 'Internal Server Error', + 3011: 'Internal Server Error', + } + # NOTE(kgriffs): The websockets library itself will buffer, so we keep # this value fairly small by default to mitigate buffer bloat. But in # the case that we have a large spillover from the websocket server's From 59698fde70a6717fba44c9fe397bd5df9bcb1a6f Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Thu, 21 Apr 2022 05:40:43 +0000 Subject: [PATCH 02/13] test(Websocket): modify testing module to support reason --- falcon/testing/helpers.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/falcon/testing/helpers.py b/falcon/testing/helpers.py index 15678e439..fc75e33c3 100644 --- a/falcon/testing/helpers.py +++ b/falcon/testing/helpers.py @@ -408,6 +408,7 @@ def __init__(self): self._state = _WebSocketState.CONNECT self._disconnect_emitted = False self._close_code = None + self._close_reason = None self._accepted_subprotocol = None self._accepted_headers = None self._collected_server_events = deque() @@ -427,6 +428,10 @@ def closed(self) -> bool: def close_code(self) -> int: return self._close_code + @property + def close_reason(self) -> str: + return self._close_reason + @property def subprotocol(self) -> str: return self._accepted_subprotocol @@ -464,12 +469,14 @@ async def wait_ready(self, timeout: Optional[int] = 5): # NOTE(kgriffs): This is a coroutine just in case we need it to be # in a future code revision. It also makes it more consistent # with the other methods. - async def close(self, code: Optional[int] = None): + async def close(self, code: Optional[int] = None, reason: Optional[str] = None): """Close the simulated connection. Keyword Args: code (int): The WebSocket close code to send to the application per the WebSocket spec (default: ``1000``). + reason (str): The WebSocket close reason to send to the application + per the WebSocket spec (default: empty string). """ # NOTE(kgriffs): Give our collector a chance in case the @@ -488,8 +495,12 @@ async def close(self, code: Optional[int] = None): if code is None: code = WSCloseCode.NORMAL + if reason is None: + reason = '' + self._state = _WebSocketState.CLOSED self._close_code = code + self._close_reason = reason async def send_text(self, payload: str): """Send a message to the app with a Unicode string payload. @@ -727,6 +738,7 @@ async def _collect(self, event: Dict[str, Any]): self._state = _WebSocketState.DENIED desired_code = event.get('code', WSCloseCode.NORMAL) + reason = event.get('reason', '') if desired_code == WSCloseCode.SERVER_ERROR or ( 3000 <= desired_code < 4000 ): @@ -735,12 +747,16 @@ async def _collect(self, event: Dict[str, Any]): # different raised error types or to pass through a # raised HTTPError status code. self._close_code = desired_code + self._close_reason = reason else: # NOTE(kgriffs): Force the close code to this since it is # similar to what happens with a real web server (the HTTP # connection is closed with a 403 and there is no websocket # close code). self._close_code = WSCloseCode.FORBIDDEN + self._close_reason = falcon.util.code_to_http_status( + WSCloseCode.FORBIDDEN - 3000 + ) self._event_handshake_complete.set() @@ -755,6 +771,7 @@ async def _collect(self, event: Dict[str, Any]): if event_type == EventType.WS_CLOSE: self._state = _WebSocketState.CLOSED self._close_code = event.get('code', WSCloseCode.NORMAL) + self._close_reason = event.get('reason', '') else: assert event_type == EventType.WS_SEND self._collected_server_events.append(event) @@ -780,7 +797,11 @@ def _create_checked_disconnect(self) -> Dict[str, Any]: ) self._disconnect_emitted = True - return {'type': EventType.WS_DISCONNECT, 'code': self._close_code} + return { + 'type': EventType.WS_DISCONNECT, + 'code': self._close_code, + 'reason': self._close_reason, + } # get_encoding_from_headers() is Copyright 2016 Kenneth Reitz, and is From 3fab9937e5591d1a791ca916584d5e6b048855e9 Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Thu, 21 Apr 2022 05:42:14 +0000 Subject: [PATCH 03/13] test(Websocket): add tests for reason in Websocket.close() --- tests/asgi/test_ws.py | 71 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/asgi/test_ws.py b/tests/asgi/test_ws.py index dd327b405..af2de37e9 100644 --- a/tests/asgi/test_ws.py +++ b/tests/asgi/test_ws.py @@ -1093,3 +1093,74 @@ def test_msgpack_missing(): with pytest.raises(RuntimeError): handler.deserialize(b'{}') + + +@pytest.mark.asyncio +@pytest.mark.parametrize('reason', ['Client closing connection', '', None]) +async def test_client_close_with_reason(reason, conductor): + class Resource: + def __init__(self): + pass + + async def on_websocket(self, req, ws): + await ws.accept() + while True: + try: + await ws.receive_data() + + except falcon.WebSocketDisconnected: + break + + resource = Resource() + conductor.app.add_route('/', resource) + + async with conductor as c: + async with c.simulate_ws('/') as ws: + await ws.close(4099, reason) + + assert ws.close_code == 4099 + if reason: + assert ws.close_reason == reason + else: + assert ws.close_reason == '' + + +@pytest.mark.asyncio +@pytest.mark.parametrize('no_default', [True, False]) +@pytest.mark.parametrize('code', [None, 1011, 4099, 4042, 3405]) +async def test_no_reason_mapping(no_default, code, conductor): + class Resource: + def __init__(self): + pass + + async def on_websocket(self, req, ws): + await ws.accept() + await ws.close(code) + + resource = Resource() + conductor.app.add_route('/', resource) + if no_default: + conductor.app.ws_options.default_close_reasons = {} + else: + conductor.app.ws_options.default_close_reasons[4099] = '4099 reason' + + async with conductor as c: + with pytest.raises(falcon.WebSocketDisconnected): + async with c.simulate_ws('/') as ws: + await ws.receive_data() + + if code: + assert ws.close_code == code + else: + assert ws.close_code == CloseCode.NORMAL + + if 3100 <= ws.close_code <= 3999: + assert ws.close_reason == falcon.util.code_to_http_status(ws.close_code - 3000) + elif ( + no_default + or ws.close_code not in conductor.app.ws_options.default_close_reasons + ): + assert ws.close_reason == '' + else: + reason = conductor.app.ws_options.default_close_reasons[ws.close_code] + assert ws.close_reason == reason From ca2b6b0f345333cd976b6bf9a13d84e464b72cb1 Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Sat, 7 May 2022 04:54:05 +0000 Subject: [PATCH 04/13] verify asgi spec version before sending reason --- falcon/asgi/ws.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/falcon/asgi/ws.py b/falcon/asgi/ws.py index d3414aeb0..8007af916 100644 --- a/falcon/asgi/ws.py +++ b/falcon/asgi/ws.py @@ -51,6 +51,7 @@ class WebSocket: '_close_code', '_close_reasons', '_supports_accept_headers', + '_supports_reason', '_mh_bin_deserialize', '_mh_bin_serialize', '_mh_text_deserialize', @@ -73,6 +74,7 @@ def __init__( default_close_reasons: Dict[Optional[int], str], ): self._supports_accept_headers = ver != '2.0' + self._supports_reason = self._check_support_reason(ver) # NOTE(kgriffs): Normalize the iterable to a stable tuple; note that # ordering is significant, and so we preserve it here. @@ -260,20 +262,15 @@ async def close(self, code: Optional[int] = None) -> None: if self.closed: return - if code in self._close_reasons: - reason = self._close_reasons[code] - elif 3100 <= code <= 3999: - reason = falcon.util.code_to_http_status(code - 3000) - else: - reason = '' + response = {'type': EventType.WS_CLOSE, 'code': code} - await self._asgi_send( - { - 'type': EventType.WS_CLOSE, - 'code': code, - 'reason': reason, - } - ) + if self._supports_reason: + if code in self._close_reasons: + response['reason'] = self._close_reasons[code] + elif 3100 <= code <= 3999: + response['reason'] = falcon.util.code_to_http_status(code - 3000) + + await self._asgi_send(response) self._state = _WebSocketState.CLOSED self._close_code = code @@ -511,6 +508,16 @@ def _translate_webserver_error(self, ex): return None + def _check_support_reason(self, asgi_ver): + target_ver = [2, 3] + current_ver = asgi_ver.split('.') + + for i in range(2): + if int(current_ver[i]) < target_ver[i]: + return False + + return True + class WebSocketOptions: """Defines a set of configurable WebSocket options. From ba03ce0f35f4128e9af54d247791be3a57bc2188 Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Sat, 7 May 2022 04:57:09 +0000 Subject: [PATCH 05/13] specify asgi spec version --- tests/asgi/test_ws.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/asgi/test_ws.py b/tests/asgi/test_ws.py index af2de37e9..fe0aeadce 100644 --- a/tests/asgi/test_ws.py +++ b/tests/asgi/test_ws.py @@ -1115,7 +1115,7 @@ async def on_websocket(self, req, ws): conductor.app.add_route('/', resource) async with conductor as c: - async with c.simulate_ws('/') as ws: + async with c.simulate_ws('/', spec_version='2.3') as ws: await ws.close(4099, reason) assert ws.close_code == 4099 @@ -1146,7 +1146,7 @@ async def on_websocket(self, req, ws): async with conductor as c: with pytest.raises(falcon.WebSocketDisconnected): - async with c.simulate_ws('/') as ws: + async with c.simulate_ws('/', spec_version='2.10.3') as ws: await ws.receive_data() if code: From 37b3b21018b37247994ac2820517ed4a0a5b0850 Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Mon, 16 May 2022 06:33:04 +0000 Subject: [PATCH 06/13] make check_support_reason() a function --- falcon/asgi/ws.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/falcon/asgi/ws.py b/falcon/asgi/ws.py index 8007af916..36af56eb6 100644 --- a/falcon/asgi/ws.py +++ b/falcon/asgi/ws.py @@ -74,7 +74,7 @@ def __init__( default_close_reasons: Dict[Optional[int], str], ): self._supports_accept_headers = ver != '2.0' - self._supports_reason = self._check_support_reason(ver) + self._supports_reason = check_support_reason(ver) # NOTE(kgriffs): Normalize the iterable to a stable tuple; note that # ordering is significant, and so we preserve it here. @@ -508,16 +508,6 @@ def _translate_webserver_error(self, ex): return None - def _check_support_reason(self, asgi_ver): - target_ver = [2, 3] - current_ver = asgi_ver.split('.') - - for i in range(2): - if int(current_ver[i]) < target_ver[i]: - return False - - return True - class WebSocketOptions: """Defines a set of configurable WebSocket options. @@ -733,3 +723,14 @@ async def _pump(self): if self._pop_message_waiter is not None: self._pop_message_waiter.set_result(None) self._pop_message_waiter = None + + +def check_support_reason(asgi_ver): + target_ver = [2, 3] + current_ver = asgi_ver.split('.') + + for i in range(2): + if int(current_ver[i]) < target_ver[i]: + return False + + return True From 0d01d7603938566af9ceddc88d6506464de1fccd Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Mon, 16 May 2022 06:39:31 +0000 Subject: [PATCH 07/13] Modify doc string --- falcon/asgi/ws.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/falcon/asgi/ws.py b/falcon/asgi/ws.py index 36af56eb6..ee543b532 100644 --- a/falcon/asgi/ws.py +++ b/falcon/asgi/ws.py @@ -521,8 +521,9 @@ class WebSocketOptions: (default ``1011``). For a list of valid close codes and ranges, see also: https://tools.ietf.org/html/rfc6455#section-7.4 default_close_reasons (dict): A default mapping between the Websocket - close code and the reason why the connection is close. HTTPerrors - are not included and will be rendered automatically with HTTP status. + close code and the reason why the connection is close. Close codes + corresponding to HTTPerrors are not included as they will be rendered + automatically using HTTP status. media_handlers (dict): A dict-like object for configuring media handlers according to the WebSocket payload type (TEXT vs. BINARY) of a given message. See also: :ref:`ws_media_handlers`. From dc30b1091e05273b9554464f1f4015d8d97628c8 Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Sun, 17 Jul 2022 02:50:13 +0000 Subject: [PATCH 08/13] Omit reason if empty string --- falcon/testing/helpers.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/falcon/testing/helpers.py b/falcon/testing/helpers.py index 628488367..aafc351ed 100644 --- a/falcon/testing/helpers.py +++ b/falcon/testing/helpers.py @@ -796,11 +796,12 @@ def _create_checked_disconnect(self) -> Dict[str, Any]: ) self._disconnect_emitted = True - return { - 'type': EventType.WS_DISCONNECT, - 'code': self._close_code, - 'reason': self._close_reason, - } + response = {'type': EventType.WS_DISCONNECT, 'code': self._close_code} + + if self._close_reason: + response['reason'] = self._close_reason + + return response # get_encoding_from_headers() is Copyright 2016 Kenneth Reitz, and is From 5d6f70e6b759ab31439278dcee71064e4d3e355b Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Sun, 17 Jul 2022 03:01:29 +0000 Subject: [PATCH 09/13] verify asgi spec version before sending reason --- falcon/asgi/app.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/falcon/asgi/app.py b/falcon/asgi/app.py index 3b4c159cd..7079a32fc 100644 --- a/falcon/asgi/app.py +++ b/falcon/asgi/app.py @@ -45,6 +45,7 @@ from .request import Request from .response import Response from .structures import SSEvent +from .ws import check_support_reason from .ws import WebSocket from .ws import WebSocketOptions @@ -976,13 +977,11 @@ async def _handle_websocket(self, ver, scope, receive, send): # we don't support, so bail out. This also fulfills the ASGI # spec requirement to only process the request after # receiving and verifying the first event. - await send( - { - 'type': EventType.WS_CLOSE, - 'code': WSCloseCode.SERVER_ERROR, - 'reason': 'Internal Server Error', - } - ) + response = {'type': EventType.WS_CLOSE, 'code': WSCloseCode.SERVER_ERROR} + if check_support_reason(ver): + response['reason'] = 'Internal Server Error' + + await send(response) return req = self._request_type(scope, receive, options=self.req_options) From c0af178416b725a48df5301cb9f58a20e893ef10 Mon Sep 17 00:00:00 2001 From: wendy5667 Date: Sun, 17 Jul 2022 03:02:51 +0000 Subject: [PATCH 10/13] Adding test case for function check_support_reason() --- tests/asgi/test_ws.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/asgi/test_ws.py b/tests/asgi/test_ws.py index 7fd771771..516a22cc7 100644 --- a/tests/asgi/test_ws.py +++ b/tests/asgi/test_ws.py @@ -889,11 +889,11 @@ async def test_bad_http_version(version, conductor): @pytest.mark.asyncio -async def test_bad_first_event(): +@pytest.mark.parametrize('version', ['2.1', '2.3', '2.10.3']) +async def test_bad_first_event(version): app = App() - scope = testing.create_scope_ws() - del scope['asgi']['spec_version'] + scope = testing.create_scope_ws(spec_version=version) ws = testing.ASGIWebSocketSimulator() wrapped_emit = ws._emit @@ -913,6 +913,10 @@ async def _emit(): assert ws.closed assert ws.close_code == CloseCode.SERVER_ERROR + if version != '2.1': + assert ws.close_reason == 'Internal Server Error' + else: + assert ws.close_reason == '' @pytest.mark.asyncio From 70e1d309d994e4337602a91fc1b75bf232bd8ba5 Mon Sep 17 00:00:00 2001 From: Federico Caselli Date: Sat, 13 Jul 2024 17:08:48 +0200 Subject: [PATCH 11/13] docs: fix docs --- falcon/asgi/ws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/falcon/asgi/ws.py b/falcon/asgi/ws.py index e9922b2b5..b1221b80e 100644 --- a/falcon/asgi/ws.py +++ b/falcon/asgi/ws.py @@ -522,7 +522,7 @@ class WebSocketOptions: see also: https://tools.ietf.org/html/rfc6455#section-7.4 default_close_reasons (dict): A default mapping between the Websocket close code and the reason why the connection is close. Close codes - corresponding to HTTPerrors are not included as they will be rendered + corresponding to HTTPErrors are not included as they will be rendered automatically using HTTP status. media_handlers (dict): A dict-like object for configuring media handlers according to the WebSocket payload type (TEXT vs. BINARY) of a From 0cd0307ade0f1d49d0de4df78fbc4a5bb2c2401b Mon Sep 17 00:00:00 2001 From: Federico Caselli Date: Sun, 14 Jul 2024 11:43:57 +0200 Subject: [PATCH 12/13] docs: add news fragment and missing attribute docs --- docs/_newsfragments/2025.newandimproved.rst | 1 + falcon/testing/helpers.py | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 docs/_newsfragments/2025.newandimproved.rst diff --git a/docs/_newsfragments/2025.newandimproved.rst b/docs/_newsfragments/2025.newandimproved.rst new file mode 100644 index 000000000..e06826b19 --- /dev/null +++ b/docs/_newsfragments/2025.newandimproved.rst @@ -0,0 +1 @@ +Support closing a :class:`falcon.asgi.WebSocket` with a reason. diff --git a/falcon/testing/helpers.py b/falcon/testing/helpers.py index 4044dfe9b..2afee75c5 100644 --- a/falcon/testing/helpers.py +++ b/falcon/testing/helpers.py @@ -389,6 +389,8 @@ class ASGIWebSocketSimulator: denied or closed by the app, or the client has disconnected. close_code (int): The WebSocket close code provided by the app if the connection is closed, or ``None`` if the connection is open. + close_reason (str): The WebSocket close reason provided by the app if + the connection is closed, or ``None`` if the connection is open. subprotocol (str): The subprotocol the app wishes to accept, or ``None`` if not specified. headers (Iterable[Iterable[bytes]]): An iterable of ``[name, value]`` From ac75eb2f9550a47bef97a205d8bd45015fc65b51 Mon Sep 17 00:00:00 2001 From: Federico Caselli Date: Sun, 14 Jul 2024 12:23:09 +0200 Subject: [PATCH 13/13] docs: fix the style of the doc string --- falcon/asgi/ws.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/falcon/asgi/ws.py b/falcon/asgi/ws.py index 0e3e4aafa..05e9a6fed 100644 --- a/falcon/asgi/ws.py +++ b/falcon/asgi/ws.py @@ -730,7 +730,7 @@ async def _pump(self): def check_support_reason(asgi_ver): - """Checks if the websocket version support a close reason.""" + """Check if the websocket version support a close reason.""" target_ver = [2, 3] current_ver = asgi_ver.split('.')