From 783540deafd6a93fc3e316164667676f2ea708b0 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Sun, 2 Sep 2018 19:32:23 +0800 Subject: [PATCH] client: libuv: fix close handling during redirect During client redirect we "reset" the wsi to the redirect address, involving closing the current fd that was told to redirect (it will usually be a completely different server or port). With libuv and its two-stage close that's not trivial. This solves the problem we will "reset" (overwrite) where the handle lives in the wsi with new a new connection / handle by having it copied out into an allocated watcher struct, which is freed in the uv close callback. To confirm it the minimal ws client example gets some new options, the original problem was replicated with this $ lws-minimal-ws-client-echo -s invalid.url.com -p 80 https://github.com/warmcat/libwebsockets/issues/1390 --- lib/core/context.c | 2 +- lib/event-libs/libuv/libuv.c | 10 +++++--- lib/roles/http/client/client-handshake.c | 4 ++-- .../minimal-ws-client-echo.c | 23 ++++++++++++------- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/core/context.c b/lib/core/context.c index c36244566..7be004df3 100644 --- a/lib/core/context.c +++ b/lib/core/context.c @@ -134,7 +134,7 @@ lws_protocol_vh_priv_get(struct lws_vhost *vhost, { int n = 0; - if (!vhost || !vhost->protocol_vh_privs) + if (!vhost || !vhost->protocol_vh_privs || !prot) return NULL; while (n < vhost->count_protocols && &vhost->protocols[n] != prot) diff --git a/lib/event-libs/libuv/libuv.c b/lib/event-libs/libuv/libuv.c index 0abd834b9..ee104d60d 100644 --- a/lib/event-libs/libuv/libuv.c +++ b/lib/event-libs/libuv/libuv.c @@ -317,6 +317,7 @@ lws_libuv_closewsi_m(uv_handle_t* handle) lws_sockfd_type sockfd = (lws_sockfd_type)(lws_intptr_t)handle->data; compatible_close(sockfd); + lws_free(handle); } int @@ -593,12 +594,15 @@ elops_check_client_connect_ok_uv(struct lws *wsi) static void elops_close_handle_manually_uv(struct lws *wsi) { - uv_handle_t *h = (void *)&wsi->w_read.uv.watcher; + struct lws_io_watcher *h = (void *)&wsi->w_read.uv.watcher, *nh; + + nh = lws_malloc(sizeof(*h), __func__); + *nh = *h; lwsl_debug("%s: lws_libuv_closehandle: wsi %p\n", __func__, wsi); - h->data = (void *)(lws_intptr_t)wsi->desc.sockfd; + ((uv_handle_t *)nh)->data = (void *)(lws_intptr_t)wsi->desc.sockfd; /* required to defer actual deletion until libuv has processed it */ - uv_close((uv_handle_t*)&wsi->w_read.uv.watcher, lws_libuv_closewsi_m); + uv_close((uv_handle_t *)nh, lws_libuv_closewsi_m); } static void diff --git a/lib/roles/http/client/client-handshake.c b/lib/roles/http/client/client-handshake.c index 58a672062..0095c79a6 100644 --- a/lib/roles/http/client/client-handshake.c +++ b/lib/roles/http/client/client-handshake.c @@ -654,13 +654,13 @@ lws_client_reset(struct lws **pwsi, int ssl, const char *address, int port, lws_ssl_close(wsi); #endif + __remove_wsi_socket_from_fds(wsi); + if (wsi->context->event_loop_ops->close_handle_manually) wsi->context->event_loop_ops->close_handle_manually(wsi); else compatible_close(wsi->desc.sockfd); - __remove_wsi_socket_from_fds(wsi); - #if defined(LWS_WITH_TLS) wsi->tls.use_ssl = ssl; #else diff --git a/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c b/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c index cb8ce0524..c28686c33 100644 --- a/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c +++ b/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c @@ -22,6 +22,7 @@ static struct lws_protocols protocols[] = { { NULL, NULL, 0, 0 } /* terminator */ }; +static struct lws_context *context; static int interrupted, port = 7681, options = 0; static const char *url = "/", *ads = "localhost"; @@ -87,9 +88,8 @@ void sigint_handler(int sig) int main(int argc, const char **argv) { struct lws_context_creation_info info; - struct lws_context *context; const char *p; - int n = 0, logs = LLL_USER | LLL_ERR | LLL_WARN | LLL_NOTICE + int logs = LLL_USER | LLL_ERR | LLL_WARN | LLL_NOTICE /* for LLL_ verbosity above NOTICE to be built into lws, * lws must have been configured and built with * -DCMAKE_BUILD_TYPE=DEBUG instead of =RELEASE */ @@ -97,8 +97,6 @@ int main(int argc, const char **argv) /* | LLL_EXT */ /* | LLL_CLIENT */ /* | LLL_LATENCY */ /* | LLL_DEBUG */; - signal(SIGINT, sigint_handler); - if ((p = lws_cmdline_option(argc, argv, "-d"))) logs = atoi(p); @@ -115,7 +113,10 @@ int main(int argc, const char **argv) if (lws_cmdline_option(argc, argv, "-o")) options |= 1; - lwsl_user("options %d\n", options); + if ((p = lws_cmdline_option(argc, argv, "-s"))) + ads = p; + + lwsl_user("options %d, ads %s\n", options, ads); memset(&info, 0, sizeof info); /* otherwise uninitialized garbage */ info.port = CONTEXT_PORT_NO_LISTEN; @@ -124,7 +125,13 @@ int main(int argc, const char **argv) if (!lws_cmdline_option(argc, argv, "-n")) info.extensions = extensions; info.pt_serv_buf_size = 32 * 1024; - info.options = LWS_SERVER_OPTION_VALIDATE_UTF8; + info.options = LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT | + LWS_SERVER_OPTION_VALIDATE_UTF8; + + if (lws_cmdline_option(argc, argv, "--libuv")) + info.options |= LWS_SERVER_OPTION_LIBUV; + else + signal(SIGINT, sigint_handler); context = lws_create_context(&info); if (!context) { @@ -132,8 +139,8 @@ int main(int argc, const char **argv) return 1; } - while (n >= 0 && !interrupted) - n = lws_service(context, 1000); + while (!lws_service(context, 1000) && !interrupted) + ; lws_context_destroy(context);