From 701ab8129ba9ea64f569daedca9a8603abad740f Mon Sep 17 00:00:00 2001 From: dequis Date: Sun, 13 Nov 2016 16:52:43 -0300 Subject: imcb_file_send_start: handle ft attempts from non-existing users --- protocols/bee_ft.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'protocols') diff --git a/protocols/bee_ft.c b/protocols/bee_ft.c index 27fd4eac..916b2e88 100644 --- a/protocols/bee_ft.c +++ b/protocols/bee_ft.c @@ -30,7 +30,7 @@ file_transfer_t *imcb_file_send_start(struct im_connection *ic, char *handle, ch bee_t *bee = ic->bee; bee_user_t *bu = bee_user_by_handle(bee, ic, handle); - if (bee->ui->ft_in_start) { + if (bee->ui->ft_in_start && bu) { return bee->ui->ft_in_start(bee, bu, file_name, file_size); } else { return NULL; -- cgit v1.2.3 From ea902752503fc5b356d6513911081ec932d804f2 Mon Sep 17 00:00:00 2001 From: dequis Date: Sun, 13 Nov 2016 17:00:04 -0300 Subject: purple: fix file transfer memory management This means cancelling transfers on logout to avoid crashes, keeping track of timeouts, reffing and unreffing the xfers, listening to the callbacks from UI and purple more carefully and using the correct functions to free the correct things at the correct moments. Originally intended to fix a crash triggered when the dcc stall timeout kicks in after the account is offline, which is apparently very frequent with telegram (it sends file transfers while fetching history, and randomly disconnects a while later). Trying to fix that meant opening a can of worms, but after three days of work on this bug I'm pretty sure I've finished dealing with the resulting mess and tested all the typical edge cases. --- protocols/purple/bpurple.h | 1 + protocols/purple/ft.c | 79 +++++++++++++++++++++++++++++++++++++++------- protocols/purple/purple.c | 6 ++++ 3 files changed, 75 insertions(+), 11 deletions(-) (limited to 'protocols') diff --git a/protocols/purple/bpurple.h b/protocols/purple/bpurple.h index 8225f0b8..81be2575 100644 --- a/protocols/purple/bpurple.h +++ b/protocols/purple/bpurple.h @@ -13,6 +13,7 @@ struct purple_data GHashTable *input_requests; guint next_request_id; char *chat_list_server; + GSList *filetransfers; }; #endif /* !BPURPLE_H */ diff --git a/protocols/purple/ft.c b/protocols/purple/ft.c index 320fc887..81fee8d0 100644 --- a/protocols/purple/ft.c +++ b/protocols/purple/ft.c @@ -41,6 +41,7 @@ struct prpl_xfer_data { int fd; char *fn, *handle; gboolean ui_wants_data; + int timeout; }; static file_transfer_t *next_ft; @@ -63,13 +64,47 @@ static void prpl_xfer_canceled(struct file_transfer *ft, char *reason) { struct prpl_xfer_data *px = ft->data; - purple_xfer_request_denied(px->xfer); + if (px->xfer) { + if (!purple_xfer_is_completed(px->xfer) && !purple_xfer_is_canceled(px->xfer)) { + purple_xfer_cancel_local(px->xfer); + } + px->xfer->ui_data = NULL; + purple_xfer_unref(px->xfer); + px->xfer = NULL; + } +} + +static void prpl_xfer_free(struct file_transfer *ft) +{ + struct prpl_xfer_data *px = ft->data; + struct purple_data *pd = px->ic->proto_data; + + pd->filetransfers = g_slist_remove(pd->filetransfers, px); + + if (px->xfer) { + px->xfer->ui_data = NULL; + purple_xfer_unref(px->xfer); + } + + if (px->timeout) { + b_event_remove(px->timeout); + } + + g_free(px->fn); + g_free(px->handle); + if (px->fd >= 0) { + close(px->fd); + } + g_free(px); } static void prplcb_xfer_new(PurpleXfer *xfer) { + purple_xfer_ref(xfer); + if (purple_xfer_get_type(xfer) == PURPLE_XFER_RECEIVE) { struct prpl_xfer_data *px = g_new0(struct prpl_xfer_data, 1); + struct purple_data *pd; xfer->ui_data = px; px->xfer = xfer; @@ -77,6 +112,9 @@ static void prplcb_xfer_new(PurpleXfer *xfer) px->fd = -1; px->ic = purple_ic_by_pa(xfer->account); + pd = px->ic->proto_data; + pd->filetransfers = g_slist_prepend(pd->filetransfers, px); + purple_xfer_set_local_filename(xfer, px->fn); /* Sadly the xfer struct is still empty ATM so come back after @@ -111,6 +149,7 @@ static gboolean prplcb_xfer_new_send_cb(gpointer data, gint fd, b_input_conditio px->ft->accept = prpl_xfer_accept; px->ft->canceled = prpl_xfer_canceled; + px->ft->free = prpl_xfer_free; px->ft->write_request = prpl_xfer_write_request; return FALSE; @@ -163,17 +202,13 @@ static gboolean prpl_xfer_write_request(struct file_transfer *ft) } -/* Generic (IM<>UI): */ static void prplcb_xfer_destroy(PurpleXfer *xfer) { struct prpl_xfer_data *px = xfer->ui_data; - g_free(px->fn); - g_free(px->handle); - if (px->fd >= 0) { - close(px->fd); + if (px) { + px->xfer = NULL; } - g_free(px); } static void prplcb_xfer_progress(PurpleXfer *xfer, double percent) @@ -223,9 +258,9 @@ static void prplcb_xfer_cancel_remote(PurpleXfer *xfer) { struct prpl_xfer_data *px = xfer->ui_data; - if (px->ft) { + if (px && px->ft) { imcb_file_canceled(px->ic, px->ft, "Canceled by remote end"); - } else { + } else if (px) { /* px->ft == NULL for sends, because of the two stages. :-/ */ imcb_error(px->ic, "File transfer cancelled by remote end"); } @@ -239,10 +274,12 @@ static gboolean purple_transfer_request_cb(gpointer data, gint fd, b_input_condi void purple_transfer_request(struct im_connection *ic, file_transfer_t *ft, char *handle) { struct prpl_xfer_data *px = g_new0(struct prpl_xfer_data, 1); + struct purple_data *pd; char *dir, *basename; ft->data = px; px->ft = ft; + px->ft->free = prpl_xfer_free; dir = g_strdup("/tmp/bitlbee-purple-ft.XXXXXX"); if (!mkdtemp(dir)) { @@ -271,10 +308,13 @@ void purple_transfer_request(struct im_connection *ic, file_transfer_t *ft, char px->ic = ic; px->handle = g_strdup(handle); + pd = px->ic->proto_data; + pd->filetransfers = g_slist_prepend(pd->filetransfers, px); + imcb_log(ic, "Due to libpurple limitations, the file has to be cached locally before proceeding with the actual file transfer. Please wait..."); - b_timeout_add(0, purple_transfer_request_cb, ft); + px->timeout = b_timeout_add(0, purple_transfer_request_cb, ft); } static void purple_transfer_forward(struct file_transfer *ft) @@ -294,6 +334,8 @@ static gboolean purple_transfer_request_cb(gpointer data, gint fd, b_input_condi file_transfer_t *ft = data; struct prpl_xfer_data *px = ft->data; + px->timeout = 0; + if (ft->write == NULL) { ft->write = prpl_xfer_write; imcb_file_recv_start(px->ic, ft); @@ -321,12 +363,27 @@ static gboolean prpl_xfer_write(struct file_transfer *ft, char *buffer, unsigned imcb_file_finished(px->ic, ft); px->ft = NULL; } else { - b_timeout_add(0, purple_transfer_request_cb, ft); + px->timeout = b_timeout_add(0, purple_transfer_request_cb, ft); } return TRUE; } +void purple_transfer_cancel_all(struct im_connection *ic) +{ + struct purple_data *pd = ic->proto_data; + + while (pd->filetransfers) { + struct prpl_xfer_data *px = pd->filetransfers->data; + + if (px->ft) { + imcb_file_canceled(ic, px->ft, "Logging out"); + } + + pd->filetransfers = g_slist_remove(pd->filetransfers, px); + } +} + PurpleXferUiOps bee_xfer_uiops = diff --git a/protocols/purple/purple.c b/protocols/purple/purple.c index d476afba..4ee41d62 100644 --- a/protocols/purple/purple.c +++ b/protocols/purple/purple.c @@ -42,6 +42,8 @@ static char *set_eval_display_name(set_t *set, char *value); void purple_request_input_callback(guint id, struct im_connection *ic, const char *message, const char *who); +void purple_transfer_cancel_all(struct im_connection *ic); + /* purple_request_input specific stuff */ typedef void (*ri_callback_t)(gpointer, const gchar *); @@ -384,6 +386,10 @@ static void purple_logout(struct im_connection *ic) imcb_chat_free(ic->groupchats->data); } + if (pd->filetransfers) { + purple_transfer_cancel_all(ic); + } + purple_account_set_enabled(pd->account, "BitlBee", FALSE); purple_connections = g_slist_remove(purple_connections, ic); purple_accounts_remove(pd->account); -- cgit v1.2.3 From 9f03c472fef309878ff2f3bc720d51e6d03077f1 Mon Sep 17 00:00:00 2001 From: dequis Date: Sun, 13 Nov 2016 21:37:14 -0300 Subject: Improve support for protocols which don't require a password This adds a prpl_options_t enum with flags, which mostly just brings OPT_PROTO_{NO_PASSWORD,PASSWORD_OPTIONAL} from libpurple as PRPL_OPT_{NO_PASSWORD,PASSWORD_OPTIONAL} --- protocols/nogaim.h | 21 +++++++++++++++++++++ protocols/purple/purple.c | 10 ++++++++++ protocols/twitter/twitter.c | 3 ++- 3 files changed, 33 insertions(+), 1 deletion(-) (limited to 'protocols') diff --git a/protocols/nogaim.h b/protocols/nogaim.h index 4cba2174..b5a46524 100644 --- a/protocols/nogaim.h +++ b/protocols/nogaim.h @@ -139,6 +139,27 @@ struct buddy_action { char *description; }; +/* This enum takes a few things from libpurple and a few things from old OPT_ flags. + * The only flag that was used before this struct was PRPL_OPT_NOOTR. + * + * The libpurple ones only use the same values as the PurpleProtocolOptions + * enum for convenience, but there's no promise of direct compatibility with + * those values. As of libpurple 2.8.0 they use up to 0x800 (1 << 11), which is + * a nice coincidence. + */ +typedef enum { + /* The protocol doesn't use passwords + * Mirrors libpurple's OPT_PROTO_NO_PASSWORD */ + PRPL_OPT_NO_PASSWORD = 1 << 4, + + /* The protocol doesn't require passwords, but may use them + * Mirrors libpurple's OPT_PROTO_PASSWORD_OPTIONAL */ + PRPL_OPT_PASSWORD_OPTIONAL = 1 << 7, + + /* The protocol is not suitable for OTR, see OPT_NOOTR */ + PRPL_OPT_NOOTR = 1 << 12, +} prpl_options_t; + struct prpl { int options; /* You should set this to the name of your protocol. diff --git a/protocols/purple/purple.c b/protocols/purple/purple.c index 4ee41d62..c7123798 100644 --- a/protocols/purple/purple.c +++ b/protocols/purple/purple.c @@ -1724,6 +1724,7 @@ void purple_initmodule() supported by this libpurple instance. */ for (prots = purple_plugins_get_protocols(); prots; prots = prots->next) { PurplePlugin *prot = prots->data; + PurplePluginProtocolInfo *pi = prot->info->extra_info; struct prpl *ret; /* If we already have this one (as a native module), don't @@ -1737,6 +1738,15 @@ void purple_initmodule() if (strncmp(ret->name, "prpl-", 5) == 0) { ret->name += 5; } + + if (pi->options & OPT_PROTO_NO_PASSWORD) { + ret->options |= PRPL_OPT_NO_PASSWORD; + } + + if (pi->options & OPT_PROTO_PASSWORD_OPTIONAL) { + ret->options |= PRPL_OPT_PASSWORD_OPTIONAL; + } + register_protocol(ret); g_string_append_printf(help, "\n* %s (%s)", ret->name, prot->info->name); diff --git a/protocols/twitter/twitter.c b/protocols/twitter/twitter.c index b2039171..8bc6140a 100644 --- a/protocols/twitter/twitter.c +++ b/protocols/twitter/twitter.c @@ -1091,7 +1091,7 @@ void twitter_initmodule() { struct prpl *ret = g_new0(struct prpl, 1); - ret->options = OPT_NOOTR; + ret->options = PRPL_OPT_NOOTR | PRPL_OPT_NO_PASSWORD; ret->name = "twitter"; ret->login = twitter_login; ret->init = twitter_init; @@ -1118,5 +1118,6 @@ void twitter_initmodule() /* And an identi.ca variant: */ ret = g_memdup(ret, sizeof(struct prpl)); ret->name = "identica"; + ret->options = PRPL_OPT_NOOTR; register_protocol(ret); } -- cgit v1.2.3