diff --git a/CHANGELOG.md b/CHANGELOG.md index ec4e3f5cd..1e385397e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ SPDX-License-Identifier: GPL-3.0-or-later ### Fixed - core: display an error message in case of invalid parameters in commands /bar, /buffer, /cursor, /print and /window +- api: fix file descriptor leak in hook_url when a timeout occurs or if the hook is removed during the transfer ([#2284](https://github.com/weechat/weechat/issues/2284)) - api: fix parsing of date/times with timezone offset in function util_parse_time - irc: fix warning on creation of irc.msgbuffer option when the server name contains upper case letters ([#2281](https://github.com/weechat/weechat/issues/2281)) - irc: display a warning for each unknown or invalid server option in commands /connect and /server diff --git a/src/core/core-url.c b/src/core/core-url.c index 665941ba6..fcb76e256 100644 --- a/src/core/core-url.c +++ b/src/core/core-url.c @@ -37,6 +37,7 @@ #include "core-infolist.h" #include "core-proxy.h" #include "core-string.h" +#include "core-util.h" #include "../plugins/plugin.h" @@ -978,31 +979,46 @@ weeurl_set_proxy (CURL *curl, struct t_proxy *proxy) * output | stdout (set only if "file_out" was not set in options) * error | error message (set only in case of error) * + * If timeout is 0, the function blocks until the end of the transfer. + * If timeout (in milliseconds) is > 0, the function returns an error in the + * output hashtable if the timeout is reached while the transfer is still + * active. + * + * If stop_download is not NULL, it is checked regularly, and as soon as the + * pointed integer becomes different from 0 (set by the caller of this function), + * the download is immediately stopped with an error. + * * Returns: * 0: OK * 1: invalid URL * 2: error downloading URL * 3: not enough memory * 4: file error + * 5: transfer stopped by the called + * 6: transfer timeout */ int weeurl_download (const char *url, struct t_hashtable *options, - struct t_hashtable *output) + long timeout, struct t_hashtable *output, + int *stop_transfer) { CURL *curl; + CURLM *multi; + CURLMcode curl_mc; struct t_url_file url_file[2]; char *url_file_option[2] = { "file_in", "file_out" }; char *url_file_mode[2] = { "rb", "wb" }; - char url_error[CURL_ERROR_SIZE + 1], url_error_code[12]; + char url_error[4096], url_error_code[12]; char **string_headers, **string_output; char str_response_code[32]; CURLoption url_file_opt_func[2] = { CURLOPT_READFUNCTION, CURLOPT_WRITEFUNCTION }; CURLoption url_file_opt_data[2] = { CURLOPT_READDATA, CURLOPT_WRITEDATA }; void *url_file_opt_cb[2] = { &weeurl_read_stream, &weeurl_write_stream }; struct t_proxy *ptr_proxy; - int rc, curl_rc, i, output_to_file; + int rc, i, output_to_file, still_running; long response_code; + struct timeval tv_now, tv_end; rc = 0; @@ -1101,44 +1117,120 @@ weeurl_download (const char *url, struct t_hashtable *options, /* set error buffer */ curl_easy_setopt (curl, CURLOPT_ERRORBUFFER, url_error); - /* perform action! */ - curl_rc = curl_easy_perform (curl); - if (curl_rc == CURLE_OK) + /* compute end time for transfer, according to the timeout */ + if (timeout > 0) { - if (output) - { - curl_easy_getinfo (curl, CURLINFO_RESPONSE_CODE, &response_code); - snprintf (str_response_code, sizeof (str_response_code), - "%ld", response_code); - hashtable_set (output, "response_code", str_response_code); - } + gettimeofday (&tv_end, NULL); + util_timeval_add (&tv_end, ((long long)timeout) * 1000); } else { - if (output) + tv_end.tv_sec = 0; + tv_end.tv_usec = 0; + } + + /* start the transfer */ + multi = curl_multi_init (); + curl_multi_add_handle (multi, curl); + while (1) + { + curl_mc = curl_multi_perform (multi, &still_running); + + if ((curl_mc == CURLM_OK) && still_running) + curl_mc = curl_multi_poll (multi, NULL, 0, 5, NULL); + + if (curl_mc != CURLM_OK) { - snprintf (url_error_code, sizeof (url_error_code), "%d", curl_rc); - if (!url_error[0]) + if (output) { - snprintf (url_error, sizeof (url_error), - "%s", _("transfer error")); + snprintf (url_error_code, sizeof (url_error_code), "%d", curl_mc); + if (!url_error[0]) + { + snprintf (url_error, sizeof (url_error), + "%s", _("transfer error")); + } + } + else + { + /* + * URL transfer done in a forked process: display error on stderr, + * which will be sent to the hook_process callback + */ + fprintf (stderr, + _("curl error %d (%s) (URL: \"%s\")\n"), + curl_mc, url_error, url); + } + rc = 2; + break; + } + if (!still_running) + { + /* transfer OK */ + if (output) + { + curl_easy_getinfo (curl, CURLINFO_RESPONSE_CODE, &response_code); + snprintf (str_response_code, sizeof (str_response_code), + "%ld", response_code); + hashtable_set (output, "response_code", str_response_code); + } + break; + } + if (stop_transfer && *stop_transfer) + { + /* transfer stopped by the caller */ + if (output) + { + if (!url_error[0]) + snprintf (url_error, sizeof (url_error), "transfer stopped"); + } + else + { + /* + * URL transfer done in a forked process: display error on stderr, + * which will be sent to the hook_process callback + */ + fprintf (stderr, + _("transfer stopped (URL: \"%s\")\n"), + url); + } + rc = 5; + break; + } + if (tv_end.tv_sec > 0) + { + /* timeout reached? */ + gettimeofday (&tv_now, NULL); + if (util_timeval_cmp (&tv_now, &tv_end) >= 0) + { + if (output) + { + if (!url_error[0]) + { + snprintf (url_error, sizeof (url_error), + URL_ERROR_TIMEOUT " (%.3fs)", + ((float)timeout) / 1000); + } + } + else + { + /* + * URL transfer done in a forked process: display error on stderr, + * which will be sent to the hook_process callback + */ + fprintf (stderr, + _("transfer timeout reached (%.3fs) (URL: \"%s\")\n"), + ((float)timeout) / 1000, url); + } + rc = 6; + break; } } - else - { - /* - * URL transfer done in a forked process: display error on stderr, - * which will be sent to the hook_process callback - */ - fprintf (stderr, - _("curl error %d (%s) (URL: \"%s\")\n"), - curl_rc, url_error, url); - } - rc = 2; } /* cleanup */ + curl_multi_remove_handle (multi, curl); curl_easy_cleanup (curl); + curl_multi_cleanup (multi); end: for (i = 0; i < 2; i++) diff --git a/src/core/core-url.h b/src/core/core-url.h index bcfaa592c..3e494723d 100644 --- a/src/core/core-url.h +++ b/src/core/core-url.h @@ -24,6 +24,8 @@ #include +#define URL_ERROR_TIMEOUT "transfer timeout reached" + struct t_hashtable; struct t_infolist; @@ -61,7 +63,8 @@ extern char *url_type_string[]; extern struct t_url_option url_options[]; extern int weeurl_download (const char *url, struct t_hashtable *options, - struct t_hashtable *output); + long timeout, struct t_hashtable *output, + int *stop_transfer); extern int weeurl_option_add_to_infolist (struct t_infolist *infolist, struct t_url_option *option); extern void weeurl_init (void); diff --git a/src/core/hook/hook-process.c b/src/core/hook/hook-process.c index 3421885e7..0ea7c1f1c 100644 --- a/src/core/hook/hook-process.c +++ b/src/core/hook/hook-process.c @@ -280,7 +280,9 @@ hook_process_child (struct t_hook *hook_process) } rc = weeurl_download (ptr_url, HOOK_PROCESS(hook_process, options), - NULL); /* output */ + 0, /* timeout */ + NULL, /* output */ + NULL); /* stop_download */ } else if (strncmp (HOOK_PROCESS(hook_process, command), "func:", 5) == 0) { diff --git a/src/core/hook/hook-url.c b/src/core/hook/hook-url.c index 018444c8a..5406978c6 100644 --- a/src/core/hook/hook-url.c +++ b/src/core/hook/hook-url.c @@ -138,7 +138,9 @@ hook_url_transfer_thread (void *hook_pointer) url_rc = weeurl_download (HOOK_URL(hook, url), HOOK_URL(hook, options), - HOOK_URL(hook, output)); + HOOK_URL(hook, timeout), + HOOK_URL(hook, output), + &(HOOK_URL(hook, stop_transfer))); if (url_rc != 0) { @@ -160,7 +162,7 @@ hook_url_timer_cb (const void *pointer, void *data, int remaining_calls) { struct t_hook *hook; const char *ptr_error; - char str_error[1024], str_error_code[12]; + char str_error[1024]; /* make C compiler happy */ (void) data; @@ -193,11 +195,10 @@ hook_url_timer_cb (const void *pointer, void *data, int remaining_calls) if (!hashtable_has_key (HOOK_URL(hook, output), "error_code")) { snprintf (str_error, sizeof (str_error), - "transfer timeout reached (%.3fs)", + URL_ERROR_TIMEOUT " (%.3fs)", ((float)HOOK_URL(hook, timeout)) / 1000); - snprintf (str_error_code, sizeof (str_error_code), "6"); hashtable_set (HOOK_URL(hook, output), "error", str_error); - hashtable_set (HOOK_URL(hook, output), "error_code", str_error_code); + hashtable_set (HOOK_URL(hook, output), "error_code", "6"); } hook_url_run_callback (hook); if (weechat_debug_core >= 1) @@ -225,7 +226,7 @@ hook_url_transfer (struct t_hook *hook) { int rc, timeout, max_calls; long interval; - char str_error[1024], str_error_code[12], str_error_code_pthread[12]; + char str_error[1024], str_error_code_pthread[12]; HOOK_URL(hook, thread_running) = 1; @@ -236,11 +237,10 @@ hook_url_transfer (struct t_hook *hook) { snprintf (str_error, sizeof (str_error), "error calling pthread_create (%d)", rc); - snprintf (str_error_code, sizeof (str_error_code), "5"); snprintf (str_error_code_pthread, sizeof (str_error_code_pthread), "%d", rc); hashtable_set (HOOK_URL(hook, output), "error", str_error); - hashtable_set (HOOK_URL(hook, output), "error_code", str_error_code); + hashtable_set (HOOK_URL(hook, output), "error_code", "100"); hashtable_set (HOOK_URL(hook, output), "error_code_pthread", str_error_code_pthread); hook_url_run_callback (hook); @@ -266,7 +266,7 @@ hook_url_transfer (struct t_hook *hook) { if (timeout <= 100) { - interval = timeout; + interval = timeout + 50; max_calls = 1; } else @@ -275,6 +275,7 @@ hook_url_transfer (struct t_hook *hook) max_calls = timeout / 100; if (timeout % 100 == 0) max_calls++; + max_calls++; } } HOOK_URL(hook, hook_timer) = hook_timer (hook->plugin, @@ -324,6 +325,7 @@ hook_url (struct t_weechat_plugin *plugin, new_hook_url->url = strdup (url); new_hook_url->options = (options) ? hashtable_dup (options) : NULL; new_hook_url->timeout = timeout; + new_hook_url->stop_transfer = 0; new_hook_url->thread_id = 0; new_hook_url->thread_created = 0; new_hook_url->thread_running = 0; @@ -367,6 +369,26 @@ hook_url_free_data (struct t_hook *hook) if (!hook || !hook->hook_data) return; + /* stop transfer if it's still active */ + if (HOOK_URL(hook, thread_created) && HOOK_URL(hook, thread_running)) + { + HOOK_URL(hook, stop_transfer) = 1; + usleep (10000); + if (!hashtable_has_key (HOOK_URL(hook, output), "error_code")) + { + hashtable_set (HOOK_URL(hook, output), "error", "transfer stopped"); + hashtable_set (HOOK_URL(hook, output), "error_code", "5"); + } + hook_url_run_callback (hook); + if (weechat_debug_core >= 1) + { + gui_chat_printf ( + NULL, + _("End of URL transfer '%s', transfer stopped"), + HOOK_URL(hook, url)); + } + } + if (HOOK_URL(hook, url)) { free (HOOK_URL(hook, url)); @@ -452,6 +474,8 @@ hook_url_add_to_infolist (struct t_infolist_item *item, return 0; if (!infolist_new_var_integer (item, "timeout", (int)(HOOK_URL(hook, timeout)))) return 0; + if (!infolist_new_var_integer (item, "stop_transfer", HOOK_URL(hook, stop_transfer))) + return 0; if (!infolist_new_var_integer (item, "thread_created", (int)(HOOK_URL(hook, thread_created)))) return 0; if (!infolist_new_var_integer (item, "thread_running", (int)(HOOK_URL(hook, thread_running)))) @@ -482,6 +506,7 @@ hook_url_print_log (struct t_hook *hook) hashtable_get_string (HOOK_URL(hook, options), "keys_values")); log_printf (" timeout . . . . . . . : %ld", HOOK_URL(hook, timeout)); + log_printf (" stop_transfer . . . . : %d", HOOK_URL(hook, stop_transfer)); log_printf (" thread_created. . . . : %d", (int)HOOK_URL(hook, thread_created)); log_printf (" thread_running. . . . : %d", (int)HOOK_URL(hook, thread_running)); log_printf (" hook_timer. . . . . . : %p", HOOK_URL(hook, hook_timer)); diff --git a/src/core/hook/hook-url.h b/src/core/hook/hook-url.h index b8fd8eefd..4f6a81179 100644 --- a/src/core/hook/hook-url.h +++ b/src/core/hook/hook-url.h @@ -41,6 +41,7 @@ struct t_hook_url char *url; /* URL */ struct t_hashtable *options; /* URL options (see doc) */ long timeout; /* timeout (ms) (0 = no timeout) */ + int stop_transfer; /* 1 is used to stop transfer now */ pthread_t thread_id; /* thread id */ int thread_created; /* thread created */ int thread_running; /* 1 if thread is running */