From c07f3d3ff693025fc90a72e9c9947bfd457da11e Mon Sep 17 00:00:00 2001 From: Max Romanov Date: Mon, 20 Sep 2021 09:01:08 +0300 Subject: [PATCH] Fixed WebSocket connection hang issue after listener reconfigure. Because the configuration values were read from the listener's configuration, an established WebSocket connection was unable to work properly (i. e. stuck) if the listener was removed. The correct source of configuration values is the request config joint. This is related to issue #581 on GitHub. --- docs/changes.xml | 7 ++++++ src/nxt_h1proto_websocket.c | 44 ++++++++++++------------------------ test/test_asgi_websockets.py | 17 ++++++++++++++ 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/docs/changes.xml b/docs/changes.xml index 4519f6e9..5855466e 100644 --- a/docs/changes.xml +++ b/docs/changes.xml @@ -37,6 +37,13 @@ fixed building with glibc 2.34, notably Fedora 35. + + +established WebSocket connections could stop reading frames from the client +after the corresponding listener had been reconfigured. + + + diff --git a/src/nxt_h1proto_websocket.c b/src/nxt_h1proto_websocket.c index 42a50a34..7be190f6 100644 --- a/src/nxt_h1proto_websocket.c +++ b/src/nxt_h1proto_websocket.c @@ -73,10 +73,10 @@ void nxt_h1p_websocket_first_frame_start(nxt_task_t *task, nxt_http_request_t *r, nxt_buf_t *ws_frame) { - nxt_conn_t *c; - nxt_timer_t *timer; - nxt_h1proto_t *h1p; - nxt_socket_conf_joint_t *joint; + nxt_conn_t *c; + nxt_timer_t *timer; + nxt_h1proto_t *h1p; + nxt_websocket_conf_t *websocket_conf; nxt_debug(task, "h1p ws first frame start"); @@ -87,11 +87,9 @@ nxt_h1p_websocket_first_frame_start(nxt_task_t *task, nxt_http_request_t *r, nxt_conn_tcp_nodelay_on(task, c); } - joint = c->listen->socket.data; + websocket_conf = &r->conf->socket_conf->websocket_conf; - if (nxt_slow_path(joint != NULL - && joint->socket_conf->websocket_conf.keepalive_interval != 0)) - { + if (nxt_slow_path(websocket_conf->keepalive_interval != 0)) { h1p->websocket_timer = nxt_mp_zget(c->mem_pool, sizeof(nxt_h1p_websocket_timer_t)); if (nxt_slow_path(h1p->websocket_timer == NULL)) { @@ -100,7 +98,7 @@ nxt_h1p_websocket_first_frame_start(nxt_task_t *task, nxt_http_request_t *r, } h1p->websocket_timer->keepalive_interval = - joint->socket_conf->websocket_conf.keepalive_interval; + websocket_conf->keepalive_interval; h1p->websocket_timer->h1p = h1p; timer = &h1p->websocket_timer->timer; @@ -218,14 +216,13 @@ static const nxt_conn_state_t nxt_h1p_read_ws_frame_header_state static void nxt_h1p_conn_ws_frame_header_read(nxt_task_t *task, void *obj, void *data) { - size_t size, hsize, frame_size, max_frame_size; - uint64_t payload_len; - nxt_conn_t *c; - nxt_h1proto_t *h1p; - nxt_http_request_t *r; - nxt_event_engine_t *engine; - nxt_websocket_header_t *wsh; - nxt_socket_conf_joint_t *joint; + size_t size, hsize, frame_size, max_frame_size; + uint64_t payload_len; + nxt_conn_t *c; + nxt_h1proto_t *h1p; + nxt_http_request_t *r; + nxt_event_engine_t *engine; + nxt_websocket_header_t *wsh; c = obj; h1p = data; @@ -265,17 +262,6 @@ nxt_h1p_conn_ws_frame_header_read(nxt_task_t *task, void *obj, void *data) r->ws_frame = c->read; - joint = c->listen->socket.data; - - if (nxt_slow_path(joint == NULL)) { - /* - * Listening socket had been closed while - * connection was in keep-alive state. - */ - c->read_state = &nxt_h1p_idle_close_state; - return; - } - if (nxt_slow_path(wsh->mask == 0)) { hxt_h1p_send_ws_error(task, r, &nxt_ws_err_not_masked); return; @@ -330,7 +316,7 @@ nxt_h1p_conn_ws_frame_header_read(nxt_task_t *task, void *obj, void *data) h1p->websocket_cont_expected = !wsh->fin; } - max_frame_size = joint->socket_conf->websocket_conf.max_frame_size; + max_frame_size = r->conf->socket_conf->websocket_conf.max_frame_size; payload_len = nxt_websocket_frame_payload_len(wsh); diff --git a/test/test_asgi_websockets.py b/test/test_asgi_websockets.py index 140bcb9a..7da2d9ce 100644 --- a/test/test_asgi_websockets.py +++ b/test/test_asgi_websockets.py @@ -1481,3 +1481,20 @@ class TestASGIWebsockets(TestApplicationPython): self.check_frame(frame, True, self.ws.OP_PING, '') # PING frame sock.close() + + def test_asgi_websockets_client_locks_app(self): + self.load('websockets/mirror') + + message = 'blah' + + _, sock, _ = self.ws.upgrade() + + assert 'success' in self.conf({}), 'remove app' + + self.ws.frame_write(sock, self.ws.OP_TEXT, message) + + frame = self.ws.frame_read(sock) + + assert message == frame['data'].decode('utf-8'), 'client' + + sock.close()