From ac449d494983f7cd3d19626d7ab81f979d0e75c9 Mon Sep 17 00:00:00 2001 From: Eugene K Date: Sat, 9 Sep 2023 21:38:54 -0400 Subject: [PATCH 1/3] do not reference websocket handle after callbacks been called --- src/tcp_src.c | 4 ++-- src/websocket.c | 16 +++++++------- tests/ws_tests.cpp | 55 +++++++++++++++++++--------------------------- 3 files changed, 32 insertions(+), 43 deletions(-) diff --git a/src/tcp_src.c b/src/tcp_src.c index 0efbf321..a11f7de8 100644 --- a/src/tcp_src.c +++ b/src/tcp_src.c @@ -94,6 +94,7 @@ static void tcp_connect_cb(uv_connect_t *req, int status) { static void resolve_cb(uv_getaddrinfo_t *req, int status, struct addrinfo *addr) { tcp_src_t *sl = req->data; + sl->resolve_req = NULL; if (sl != NULL) { UM_LOG(TRACE, "resolved status = %d", status); uv_tcp_t *conn = NULL; @@ -110,7 +111,6 @@ static void resolve_cb(uv_getaddrinfo_t *req, int status, struct addrinfo *addr) if (status != 0) { UM_LOG(ERR, "connect failed: %d(%s)", status, uv_strerror(status)); - sl->connect_cb((tlsuv_src_t *) sl, status, sl->connect_ctx); if (sl->conn_req) { free(sl->conn_req); sl->conn_req = NULL; @@ -118,9 +118,9 @@ static void resolve_cb(uv_getaddrinfo_t *req, int status, struct addrinfo *addr) uv_close((uv_handle_t *) conn, free_handle); } } + sl->connect_cb((tlsuv_src_t *) sl, status, sl->connect_ctx); } - sl->resolve_req = NULL; } uv_freeaddrinfo(addr); diff --git a/src/websocket.c b/src/websocket.c index 7e5683ca..f0998b1e 100644 --- a/src/websocket.c +++ b/src/websocket.c @@ -428,10 +428,6 @@ static void send_pong(tlsuv_websocket_t *ws, const char* ping_data, int len) { static void on_ws_close(tlsuv_websocket_t *ws) { if (ws == NULL) return; - if (!ws->closed && ws->close_cb) { - ws->close_cb((uv_handle_t *) ws); - } - if (ws->req) { http_req_free(ws->req); free(ws->req); @@ -450,20 +446,24 @@ static void on_ws_close(tlsuv_websocket_t *ws) { tcp_src_free((tcp_src_t *) ws->src); ws->src = NULL; } - ws->closed = true; + + if (ws->close_cb) { + ws->close_cb((uv_handle_t *) ws); + } } + static void ws_close_cb(uv_link_t *l) { tlsuv_websocket_t *ws = l->data; - on_ws_close(ws); l->data = NULL; + + on_ws_close(ws); } int tlsuv_websocket_close(tlsuv_websocket_t *ws, uv_close_cb cb) { ws->close_cb = cb; if (ws->ws_link.data != NULL) { uv_link_close(&ws->ws_link, ws_close_cb); - } - else { + } else { on_ws_close(ws); } return 0; diff --git a/tests/ws_tests.cpp b/tests/ws_tests.cpp index 09ed4220..d5237100 100644 --- a/tests/ws_tests.cpp +++ b/tests/ws_tests.cpp @@ -54,6 +54,7 @@ static void on_close_cb(uv_handle_t *h) { auto ws = (tlsuv_websocket_t *)h; auto test = (websocket_test*)ws->data; test->close_cb_called = true; + free(ws); } static void on_connect(uv_connect_t *req, int status) { @@ -89,34 +90,29 @@ static void on_ws_data(uv_stream_t *s, ssize_t nread, const uv_buf_t* buf) { TEST_CASE("websocket fail tests", "[websocket]") { UvLoopTest lt; - tlsuv_websocket_t clt; + auto clt = (tlsuv_websocket_t *)malloc(sizeof(tlsuv_websocket_t)); websocket_test test(0); - WHEN("invalid URL") { - tlsuv_websocket_init(lt.loop, &clt); - test.ws = &clt; - clt.data = &test; + tlsuv_websocket_init(lt.loop, clt); + test.ws = clt; + clt->data = &test; + + uv_connect_t r; + r.data = &test; - uv_connect_t r; - r.data = &test; - int rc = tlsuv_websocket_connect(&r, &clt, "not a real URL", on_connect, on_ws_data); + WHEN("invalid URL") { + int rc = tlsuv_websocket_connect(&r, clt, "not a real URL", on_connect, on_ws_data); lt.run(); CHECK(test.conn_status == 0); CHECK(rc == UV_EINVAL); + tlsuv_websocket_close(clt, on_close_cb); } WHEN("resolve failure ") { - tlsuv_websocket_init(lt.loop, &clt); - test.ws = &clt; - clt.data = &test; - - uv_connect_t r; - r.data = &test; - int rc = tlsuv_websocket_connect(&r, &clt, "ws://not.a.real.host", on_connect, on_ws_data); + int rc = tlsuv_websocket_connect(&r, clt, "ws://not.a.real.host", on_connect, on_ws_data); lt.run(); CHECK((rc == UV_EAI_NONAME || test.conn_status == UV_EAI_NONAME)); } - tlsuv_websocket_close(&clt, on_close_cb); } #define WS_TEST_HOST "echo.websocket.events" @@ -124,17 +120,18 @@ TEST_CASE("websocket fail tests", "[websocket]") { TEST_CASE("websocket echo tests", "[websocket]") { UvLoopTest lt; - tlsuv_websocket_t clt; + auto clt = (tlsuv_websocket_t *)malloc(sizeof (tlsuv_websocket_t)); websocket_test test(2); - WHEN("ws echo test") { - tlsuv_websocket_init(lt.loop, &clt); - test.ws = &clt; - clt.data = &test; + tlsuv_websocket_init(lt.loop, clt); + test.ws = clt; + clt->data = &test; - uv_connect_t r; - r.data = &test; - int rc = tlsuv_websocket_connect(&r, &clt, "ws://" WS_TEST_HOST, on_connect, on_ws_data); + uv_connect_t r; + r.data = &test; + + WHEN("ws echo test") { + int rc = tlsuv_websocket_connect(&r, clt, "ws://" WS_TEST_HOST, on_connect, on_ws_data); lt.run(); CHECK(rc == 0); CHECK(test.conn_status == 0); @@ -145,13 +142,7 @@ TEST_CASE("websocket echo tests", "[websocket]") { } WHEN("wss echo test") { - tlsuv_websocket_init(lt.loop, &clt); - test.ws = &clt; - clt.data = &test; - - uv_connect_t r; - r.data = &test; - int rc = tlsuv_websocket_connect(&r, &clt, "wss://" WS_TEST_HOST, on_connect, on_ws_data); + int rc = tlsuv_websocket_connect(&r, clt, "wss://" WS_TEST_HOST, on_connect, on_ws_data); lt.run(); CHECK(rc == 0); CHECK(test.conn_status == 0); @@ -160,6 +151,4 @@ TEST_CASE("websocket echo tests", "[websocket]") { REQUIRE(test.resp.size() == 2); CHECK_THAT(test.resp[1],Catch::Matches("this is a test")); } - - tlsuv_websocket_close(&clt, on_close_cb); } \ No newline at end of file From 8651ffd82783a968654443783cd410bfa3c7c99a Mon Sep 17 00:00:00 2001 From: Eugene K Date: Sat, 9 Sep 2023 22:08:53 -0400 Subject: [PATCH 2/3] add missing return in the case when server sends invalid/non-parseable HTTP response [fixes #174] --- src/http.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/http.c b/src/http.c index 66350638..5d5873f1 100644 --- a/src/http.c +++ b/src/http.c @@ -94,11 +94,11 @@ static void http_read_cb(uv_link_t *link, ssize_t nread, const uv_buf_t *buf) { fail_active_request(c, UV_EINVAL, "failed to parse HTTP response"); close_connection(c); free(buf->base); - + return; } } - if (c->active->state == completed) { + if (c->active->state == completed) { tlsuv_http_req_t *hr = c->active; c->active = NULL; @@ -119,8 +119,7 @@ static void http_read_cb(uv_link_t *link, ssize_t nread, const uv_buf_t *buf) { if (!keep_alive) { close_connection(c); - } - else { + } else { uv_async_send(&c->proc); } } From 97e42e552ae4c64897ed55d37dcd728d57fa3af1 Mon Sep 17 00:00:00 2001 From: Eugene K Date: Sun, 10 Sep 2023 12:08:26 -0400 Subject: [PATCH 3/3] check for NULL first --- src/tcp_src.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tcp_src.c b/src/tcp_src.c index a11f7de8..d093b558 100644 --- a/src/tcp_src.c +++ b/src/tcp_src.c @@ -94,8 +94,8 @@ static void tcp_connect_cb(uv_connect_t *req, int status) { static void resolve_cb(uv_getaddrinfo_t *req, int status, struct addrinfo *addr) { tcp_src_t *sl = req->data; - sl->resolve_req = NULL; if (sl != NULL) { + sl->resolve_req = NULL; UM_LOG(TRACE, "resolved status = %d", status); uv_tcp_t *conn = NULL; if (status == 0) { @@ -120,7 +120,6 @@ static void resolve_cb(uv_getaddrinfo_t *req, int status, struct addrinfo *addr) } sl->connect_cb((tlsuv_src_t *) sl, status, sl->connect_ctx); } - } uv_freeaddrinfo(addr);