From 2766e6cb8113fe778a256fb110ef4222b75b02ea Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Tue, 9 Apr 2013 19:30:34 +0300 Subject: [PATCH 1/3] request: handle purple_account_request_password() This is needed for libpurple plugins with optional password, e.g. SIPE since 1.14.1. That libpurple API call boils down to a purple_request_fields() call. The flagging for --enable-leaky-request-stubs was refactored so that this new code is always compiled in. --- src/connection.c | 45 +++++++++++++++++++++++++++++++++----- src/connection.h | 3 +++ src/main.c | 2 -- src/request.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++------ src/request.h | 3 +++ 5 files changed, 105 insertions(+), 14 deletions(-) diff --git a/src/connection.c b/src/connection.c index 3ec4c8e..c005207 100644 --- a/src/connection.c +++ b/src/connection.c @@ -42,6 +42,7 @@ #include "connection-avatars.h" #include "connection-mail.h" #include "extensions/extensions.h" +#include "request.h" #include "connection-capabilities.h" @@ -135,6 +136,9 @@ struct _HazeConnectionPrivate gchar *prpl_id; PurplePluginProtocolInfo *prpl_info; + /* Set if purple_account_request_password() was called */ + gpointer password_request; + /* Set if purple_account_disconnect has been called or is scheduled to be * called, so should not be called again. */ @@ -443,6 +447,11 @@ _haze_connection_password_manager_prompt_cb (GObject *source, { DEBUG ("Simple password manager failed: %s", error->message); + if (priv->password_request) + { + haze_request_password_cb(priv->password_request, NULL); + } + if (base_conn->status != TP_CONNECTION_STATUS_DISCONNECTED) { tp_base_connection_disconnect_with_dbus_error (base_conn, @@ -451,7 +460,13 @@ _haze_connection_password_manager_prompt_cb (GObject *source, } /* no need to call purple_account_disconnect because _connect - * was never called */ + * was never called ... + * ... unless we had a dynamic password request */ + if (priv->password_request) + { + priv->disconnecting = TRUE; + purple_account_disconnect (self->account); + } g_error_free (error); return; @@ -460,11 +475,17 @@ _haze_connection_password_manager_prompt_cb (GObject *source, g_free (priv->password); priv->password = g_strdup (password->str); - purple_account_set_password (self->account, priv->password); + if (priv->password_request) + { + haze_request_password_cb (priv->password_request, priv->password); + } else + { + purple_account_set_password (self->account, priv->password); - purple_account_set_enabled(self->account, UI_ID, TRUE); - purple_account_connect (self->account); - priv->connect_called = TRUE; + purple_account_set_enabled(self->account, UI_ID, TRUE); + purple_account_connect (self->account); + priv->connect_called = TRUE; + } } static gboolean @@ -513,6 +534,20 @@ _haze_connection_start_connecting (TpBaseConnection *base, return TRUE; } +void haze_connection_request_password (PurpleAccount *account, + void *user_data) +{ + HazeConnection *self = ACCOUNT_GET_HAZE_CONNECTION (account); + HazeConnectionPrivate *priv = self->priv; + + priv->password_request = user_data; + + /* pop up auth channel */ + tp_simple_password_manager_prompt_async (self->password_manager, + _haze_connection_password_manager_prompt_cb, + self); +} + static void _haze_connection_shut_down (TpBaseConnection *base) { diff --git a/src/connection.h b/src/connection.h index a75732a..ae587ab 100644 --- a/src/connection.h +++ b/src/connection.h @@ -109,6 +109,9 @@ const gchar *haze_get_fallback_group (void); const gchar **haze_connection_get_implemented_interfaces (void); const gchar **haze_connection_get_guaranteed_interfaces (void); +void haze_connection_request_password (PurpleAccount *account, + gpointer user_data); + G_END_DECLS #endif /* #ifndef __HAZE_CONNECTION_H__*/ diff --git a/src/main.c b/src/main.c index afa0a02..4cd171c 100644 --- a/src/main.c +++ b/src/main.c @@ -141,9 +141,7 @@ haze_ui_init (void) purple_accounts_set_ui_ops (haze_get_account_ui_ops ()); purple_conversations_set_ui_ops (haze_get_conv_ui_ops ()); purple_connections_set_ui_ops (haze_get_connection_ui_ops ()); -#ifdef ENABLE_LEAKY_REQUEST_STUBS purple_request_set_ui_ops (haze_request_get_ui_ops ()); -#endif purple_notify_set_ui_ops (haze_notify_get_ui_ops ()); purple_privacy_set_ui_ops (haze_get_privacy_ui_ops ()); } diff --git a/src/request.c b/src/request.c index 408678b..dae859d 100644 --- a/src/request.c +++ b/src/request.c @@ -18,6 +18,8 @@ * */ +#include "config.h" + #include #include @@ -25,7 +27,9 @@ #include "debug.h" #include "request.h" +#include "connection.h" +#ifdef ENABLE_LEAKY_REQUEST_STUBS static gpointer haze_request_input (const char *title, const char *primary, @@ -95,6 +99,30 @@ haze_request_action (const char *title, return NULL; } +#endif + +struct password_data { + PurpleRequestFields *fields; + PurpleRequestField *password; + GCallback ok_cb; + GCallback cancel_cb; + void *user_data; +}; + +void haze_request_password_cb (gpointer user_data, + const gchar *password) +{ + struct password_data *pd = user_data; + + if (password) { + purple_request_field_string_set_value(pd->password, password); + ((PurpleRequestFieldsCb)pd->ok_cb)(pd->user_data, pd->fields); + } else { + ((PurpleRequestFieldsCb)pd->cancel_cb)(pd->user_data, pd->fields); + } + + g_free(pd); +} static gpointer haze_request_fields (const char *title, @@ -110,14 +138,36 @@ haze_request_fields (const char *title, PurpleConversation *conv, void *user_data) { - DEBUG ("ignoring request:"); - DEBUG (" title: %s", (title ? title : "(null)")); - DEBUG (" primary: %s", (primary ? primary : "(null)")); - DEBUG (" secondary: %s", (secondary ? secondary : "(null)")); + /* + * We must support purple_account_request_password() which boils down + * to purple_request_fields() with certain parameters. I'm not sure + * if this the best way of doing this, but it works. + */ + if (purple_request_fields_exists(fields, "password") && + purple_request_fields_exists(fields, "remember")) { + struct password_data *pd = g_new0(struct password_data, 1); + + DEBUG ("triggering password request"); + + pd->fields = fields; + pd->password = purple_request_fields_get_field(fields, "password"); + pd->ok_cb = ok_cb; + pd->cancel_cb = cancel_cb; + pd->user_data = user_data; + + haze_connection_request_password(account, pd); + + } else { + DEBUG ("ignoring request:"); + DEBUG (" title: %s", (title ? title : "(null)")); + DEBUG (" primary: %s", (primary ? primary : "(null)")); + DEBUG (" secondary: %s", (secondary ? secondary : "(null)")); + } return NULL; } +#ifdef ENABLE_LEAKY_REQUEST_STUBS static gpointer haze_request_file (const char *title, const char *filename, @@ -152,7 +202,7 @@ haze_request_folder (const char *title, return NULL; } - +#endif /* void (*close_request)(PurpleRequestType type, void *ui_handle); @@ -160,12 +210,14 @@ haze_request_folder (const char *title, static PurpleRequestUiOps request_uiops = { +#ifdef ENABLE_LEAKY_REQUEST_STUBS .request_input = haze_request_input, .request_choice = haze_request_choice, .request_action = haze_request_action, - .request_fields = haze_request_fields, .request_file = haze_request_file, - .request_folder = haze_request_folder + .request_folder = haze_request_folder, +#endif + .request_fields = haze_request_fields }; PurpleRequestUiOps * diff --git a/src/request.h b/src/request.h index d896cc6..cc572d6 100644 --- a/src/request.h +++ b/src/request.h @@ -20,4 +20,7 @@ #include +void haze_request_password_cb (gpointer user_data, + const gchar *password); + PurpleRequestUiOps *haze_request_get_ui_ops (void); -- 1.8.1.4 From bd32b0af473c7e98ca81b8826fbab9c930b0ad9a Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Wed, 10 Apr 2013 16:13:04 +0300 Subject: [PATCH 2/3] request: fix resource leakage It is the responsibility of the UI code to free the "fields" parameter after haze_request_fields() has been called. This has to be done with purple_request_close() and handled in the close_request() UI operation. --- src/request.c | 65 +++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/src/request.c b/src/request.c index dae859d..db54317 100644 --- a/src/request.c +++ b/src/request.c @@ -101,7 +101,7 @@ haze_request_action (const char *title, } #endif -struct password_data { +struct fields_data { PurpleRequestFields *fields; PurpleRequestField *password; GCallback ok_cb; @@ -109,21 +109,40 @@ struct password_data { void *user_data; }; +static void haze_close_request(PurpleRequestType type, void *ui_handle) +{ + struct fields_data *fd = ui_handle; + + purple_request_fields_destroy(fd->fields); + g_free(fd); +} + void haze_request_password_cb (gpointer user_data, const gchar *password) { - struct password_data *pd = user_data; + struct fields_data *fd = user_data; if (password) { - purple_request_field_string_set_value(pd->password, password); - ((PurpleRequestFieldsCb)pd->ok_cb)(pd->user_data, pd->fields); + purple_request_field_string_set_value(fd->password, password); + ((PurpleRequestFieldsCb)fd->ok_cb)(fd->user_data, fd->fields); } else { - ((PurpleRequestFieldsCb)pd->cancel_cb)(pd->user_data, pd->fields); + ((PurpleRequestFieldsCb)fd->cancel_cb)(fd->user_data, fd->fields); } - g_free(pd); + purple_request_close(PURPLE_REQUEST_FIELDS, fd); } +static gboolean haze_request_fields_destroy(gpointer user_data) +{ + purple_request_close(PURPLE_REQUEST_FIELDS, user_data); + return FALSE; +} + +/* + * We must support purple_account_request_password() which boils down + * to purple_request_fields() with certain parameters. I'm not sure + * if this the best way of doing this, but it works. + */ static gpointer haze_request_fields (const char *title, const char *primary, @@ -138,33 +157,34 @@ haze_request_fields (const char *title, PurpleConversation *conv, void *user_data) { - /* - * We must support purple_account_request_password() which boils down - * to purple_request_fields() with certain parameters. I'm not sure - * if this the best way of doing this, but it works. - */ + struct fields_data *fd = g_new0(struct fields_data, 1); + + /* it is our responsibility to destroy this data */ + fd->fields = fields; + if (purple_request_fields_exists(fields, "password") && purple_request_fields_exists(fields, "remember")) { - struct password_data *pd = g_new0(struct password_data, 1); DEBUG ("triggering password request"); - pd->fields = fields; - pd->password = purple_request_fields_get_field(fields, "password"); - pd->ok_cb = ok_cb; - pd->cancel_cb = cancel_cb; - pd->user_data = user_data; + fd->password = purple_request_fields_get_field(fields, "password"); + fd->ok_cb = ok_cb; + fd->cancel_cb = cancel_cb; + fd->user_data = user_data; - haze_connection_request_password(account, pd); + haze_connection_request_password(account, fd); } else { DEBUG ("ignoring request:"); DEBUG (" title: %s", (title ? title : "(null)")); DEBUG (" primary: %s", (primary ? primary : "(null)")); DEBUG (" secondary: %s", (secondary ? secondary : "(null)")); + + /* Avoid leaking of "fields" */ + g_idle_add(haze_request_fields_destroy, fd); } - return NULL; + return fd; } #ifdef ENABLE_LEAKY_REQUEST_STUBS @@ -204,10 +224,6 @@ haze_request_folder (const char *title, } #endif -/* - void (*close_request)(PurpleRequestType type, void *ui_handle); -*/ - static PurpleRequestUiOps request_uiops = { #ifdef ENABLE_LEAKY_REQUEST_STUBS @@ -217,7 +233,8 @@ static PurpleRequestUiOps request_uiops = .request_file = haze_request_file, .request_folder = haze_request_folder, #endif - .request_fields = haze_request_fields + .request_fields = haze_request_fields, + .close_request = haze_close_request }; PurpleRequestUiOps * -- 1.8.1.4 From 3cb78610afdd638cb2ebb9cf907d9acad6f05a8d Mon Sep 17 00:00:00 2001 From: Stefan Becker Date: Thu, 11 Apr 2013 15:45:34 +0300 Subject: [PATCH 3/3] request: fix more resource leaks Triggered by Will's review comments. - check callback pointers to be non-NULL before calling them - call cancel_cb also in destroy idle callback - reset connection's password request field on closing - coding style cleanup --- src/connection.c | 19 ++++++++++--- src/connection.h | 1 + src/request.c | 87 +++++++++++++++++++++++++++++++++++++------------------- 3 files changed, 73 insertions(+), 34 deletions(-) diff --git a/src/connection.c b/src/connection.c index c005207..10450c2 100644 --- a/src/connection.c +++ b/src/connection.c @@ -449,7 +449,7 @@ _haze_connection_password_manager_prompt_cb (GObject *source, if (priv->password_request) { - haze_request_password_cb(priv->password_request, NULL); + haze_request_password_cb (priv->password_request, NULL); } if (base_conn->status != TP_CONNECTION_STATUS_DISCONNECTED) @@ -478,7 +478,8 @@ _haze_connection_password_manager_prompt_cb (GObject *source, if (priv->password_request) { haze_request_password_cb (priv->password_request, priv->password); - } else + } + else { purple_account_set_password (self->account, priv->password); @@ -534,8 +535,9 @@ _haze_connection_start_connecting (TpBaseConnection *base, return TRUE; } -void haze_connection_request_password (PurpleAccount *account, - void *user_data) +void +haze_connection_request_password (PurpleAccount *account, + void *user_data) { HazeConnection *self = ACCOUNT_GET_HAZE_CONNECTION (account); HazeConnectionPrivate *priv = self->priv; @@ -548,6 +550,15 @@ void haze_connection_request_password (PurpleAccount *account, self); } +void +haze_connection_cancel_password_request (PurpleAccount *account) +{ + HazeConnection *self = ACCOUNT_GET_HAZE_CONNECTION (account); + HazeConnectionPrivate *priv = self->priv; + + priv->password_request = NULL; +} + static void _haze_connection_shut_down (TpBaseConnection *base) { diff --git a/src/connection.h b/src/connection.h index ae587ab..0637f6c 100644 --- a/src/connection.h +++ b/src/connection.h @@ -111,6 +111,7 @@ const gchar **haze_connection_get_guaranteed_interfaces (void); void haze_connection_request_password (PurpleAccount *account, gpointer user_data); +void haze_connection_cancel_password_request (PurpleAccount *account); G_END_DECLS diff --git a/src/request.c b/src/request.c index db54317..8aa7bc9 100644 --- a/src/request.c +++ b/src/request.c @@ -102,39 +102,62 @@ haze_request_action (const char *title, #endif struct fields_data { + PurpleAccount *account; PurpleRequestFields *fields; PurpleRequestField *password; - GCallback ok_cb; - GCallback cancel_cb; + PurpleRequestFieldsCb ok_cb; + PurpleRequestFieldsCb cancel_cb; void *user_data; }; -static void haze_close_request(PurpleRequestType type, void *ui_handle) +static void +haze_close_request (PurpleRequestType type, + void *ui_handle) { struct fields_data *fd = ui_handle; - purple_request_fields_destroy(fd->fields); - g_free(fd); + haze_connection_cancel_password_request (fd->account); + purple_request_fields_destroy (fd->fields); + g_slice_free (struct fields_data, fd); } -void haze_request_password_cb (gpointer user_data, - const gchar *password) +void +haze_request_password_cb (gpointer user_data, + const gchar *password) { struct fields_data *fd = user_data; - if (password) { - purple_request_field_string_set_value(fd->password, password); - ((PurpleRequestFieldsCb)fd->ok_cb)(fd->user_data, fd->fields); - } else { - ((PurpleRequestFieldsCb)fd->cancel_cb)(fd->user_data, fd->fields); - } - - purple_request_close(PURPLE_REQUEST_FIELDS, fd); + if (password) + { + purple_request_field_string_set_value (fd->password, password); + if (fd->ok_cb) + { + (fd->ok_cb) (fd->user_data, fd->fields); + } + } + else + { + if (fd->cancel_cb) + { + (fd->cancel_cb) (fd->user_data, fd->fields); + } + } + + purple_request_close (PURPLE_REQUEST_FIELDS, fd); } -static gboolean haze_request_fields_destroy(gpointer user_data) +static gboolean +haze_request_fields_destroy (gpointer user_data) { - purple_request_close(PURPLE_REQUEST_FIELDS, user_data); + struct fields_data *fd = user_data; + + if (fd->cancel_cb) + { + (fd->cancel_cb) (fd->user_data, fd->fields); + } + + purple_request_close (PURPLE_REQUEST_FIELDS, user_data); + return FALSE; } @@ -157,32 +180,36 @@ haze_request_fields (const char *title, PurpleConversation *conv, void *user_data) { - struct fields_data *fd = g_new0(struct fields_data, 1); + struct fields_data *fd = g_slice_new0 (struct fields_data); /* it is our responsibility to destroy this data */ - fd->fields = fields; + fd->account = account; + fd->fields = fields; + fd->cancel_cb = (PurpleRequestFieldsCb) cancel_cb; + fd->user_data = user_data; - if (purple_request_fields_exists(fields, "password") && - purple_request_fields_exists(fields, "remember")) { + if (purple_request_fields_exists (fields, "password") && + purple_request_fields_exists (fields, "remember")) + { DEBUG ("triggering password request"); - fd->password = purple_request_fields_get_field(fields, "password"); - fd->ok_cb = ok_cb; - fd->cancel_cb = cancel_cb; - fd->user_data = user_data; + fd->password = purple_request_fields_get_field (fields, "password"); + fd->ok_cb = (PurpleRequestFieldsCb) ok_cb; - haze_connection_request_password(account, fd); + haze_connection_request_password (account, fd); - } else { + } + else + { DEBUG ("ignoring request:"); DEBUG (" title: %s", (title ? title : "(null)")); DEBUG (" primary: %s", (primary ? primary : "(null)")); DEBUG (" secondary: %s", (secondary ? secondary : "(null)")); - /* Avoid leaking of "fields" */ - g_idle_add(haze_request_fields_destroy, fd); - } + /* Avoid leaking of "fields" and "user_data" */ + g_idle_add (haze_request_fields_destroy, fd); + } return fd; } -- 1.8.1.4