Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

do not reference websocket handle after callbacks been called #181

Merged
merged 3 commits into from
Sep 10, 2023
Merged

do not reference websocket handle after callbacks been called #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac449d4
do not reference websocket handle after callbacks been called
ekoby Sep 10, 2023
8651ffd
add missing return in the case when server sends invalid/non-parseabl…
ekoby Sep 10, 2023
97e42e5
check for NULL first
ekoby Sep 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/http.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@
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;

Expand All @@ -119,8 +119,7 @@

if (!keep_alive) {
close_connection(c);
}
else {
} else {
uv_async_send(&c->proc);
}
}
Expand Down Expand Up @@ -280,12 +279,12 @@
clt->src->cancel(clt->src);
}

static void req_write_cb(uv_link_t *source, int status, void *arg) {

Check warning on line 282 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, openssl) 

 'source': unreferenced formal parameter

Check warning on line 282 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, mbedtls) 

 'source': unreferenced formal parameter
UM_LOG(VERB, "request write completed: %d", status);
free(arg);
}

static void req_write_body_cb(uv_link_t *source, int status, void *arg) {

Check warning on line 287 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, openssl) 

 'source': unreferenced formal parameter

Check warning on line 287 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, mbedtls) 

 'source': unreferenced formal parameter
UM_LOG(VERB, "request body write completed: %d", status);
struct body_chunk_s *chunk = arg;
if (chunk->cb) {
Expand All @@ -294,7 +293,7 @@
free(chunk);
}

static void chunk_hdr_wcb(uv_link_t *l, int status, void *arg) {

Check warning on line 296 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, openssl) 

 'status': unreferenced formal parameter

Check warning on line 296 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, openssl) 

 'l': unreferenced formal parameter

Check warning on line 296 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, mbedtls) 

 'status': unreferenced formal parameter

Check warning on line 296 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, mbedtls) 

 'l': unreferenced formal parameter
if (arg != NULL) {
free(arg);
}
Expand All @@ -320,7 +319,7 @@
uv_link_write((uv_link_t *) &clt->http_link, &buf, 1, NULL, chunk_hdr_wcb, buf.base);

buf.base = (char*)b->chunk;
buf.len = b->len;

Check warning on line 322 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, openssl) 

 '=': conversion from 'size_t' to 'ULONG', possible loss of data

Check warning on line 322 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, mbedtls) 

 '=': conversion from 'size_t' to 'ULONG', possible loss of data
uv_link_write((uv_link_t *) &clt->http_link, &buf, 1, NULL, req_write_body_cb, b);

buf.base = "\r\n";
Expand All @@ -337,12 +336,12 @@
else {
buf = uv_buf_init((char*)b->chunk, (unsigned int)b->len);
uv_link_write((uv_link_t *) &clt->http_link, &buf, 1, NULL, req_write_body_cb, b);
if (req->body_sent_size > req->req_body_size) {

Check warning on line 339 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, openssl) 

 '>': signed/unsigned mismatch

Check warning on line 339 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, mbedtls) 

 '>': signed/unsigned mismatch
UM_LOG(WARN, "Supplied data[%ld] is larger than provided Content-Length[%ld]",
req->body_sent_size, req->req_body_size);
}

if (req->body_sent_size >= req->req_body_size) {

Check warning on line 344 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, openssl) 

 '>=': signed/unsigned mismatch

Check warning on line 344 in src/http.c

View workflow job for this annotation

GitHub Actions / build (windows, mbedtls) 

 '>=': signed/unsigned mismatch
req->state = body_sent;
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/tcp_src.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ static void resolve_cb(uv_getaddrinfo_t *req, int status, struct addrinfo *addr)
tcp_src_t *sl = req->data;

if (sl != NULL) {
sl->resolve_req = NULL;
UM_LOG(TRACE, "resolved status = %d", status);
uv_tcp_t *conn = NULL;
if (status == 0) {
Expand All @@ -110,17 +111,15 @@ 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;
if (conn) {
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);
Expand Down
16 changes: 8 additions & 8 deletions src/websocket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down
55 changes: 22 additions & 33 deletions tests/ws_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -89,52 +90,48 @@ 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"

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);
Expand All @@ -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);
Expand All @@ -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);
}
Loading