/cpw/tomkiewicz/masterpassword: 52b91b4b8314: Proper error handl...
Tomasz Wasilczyk
tomkiewicz at cpw.pidgin.im
Fri Mar 22 17:39:25 EDT 2013
Changeset: 52b91b4b8314938f4f05e332ddc2b2c2dd5bff45
Author: Tomasz Wasilczyk <tomkiewicz at cpw.pidgin.im>
Date: 2013-03-22 22:39 +0100
Branch: soc.2008.masterpassword
URL: https://hg.pidgin.im/cpw/tomkiewicz/masterpassword/rev/52b91b4b8314
Description:
Proper error handling when doing a password migration, fixes reverting it.
diffstat:
libpurple/keyring.c | 31 +++++++++++++--------------
libpurple/plugins/keyrings/gnomekeyring.c | 20 +++++------------
libpurple/plugins/keyrings/internalkeyring.c | 30 ++++++++++++++++++--------
libpurple/plugins/keyrings/kwallet.cpp | 14 ++---------
libpurple/plugins/keyrings/secretservice.c | 19 ++++------------
5 files changed, 50 insertions(+), 64 deletions(-)
diffs (truncated from 343 to 300 lines):
diff --git a/libpurple/keyring.c b/libpurple/keyring.c
--- a/libpurple/keyring.c
+++ b/libpurple/keyring.c
@@ -419,8 +419,6 @@ purple_keyring_set_inuse_check_error_cb(
tracker = (PurpleKeyringChangeTracker *)data;
- g_return_if_fail(tracker->abort == FALSE);
-
tracker->read_outstanding--;
name = purple_account_get_username(account);
@@ -431,34 +429,37 @@ purple_keyring_set_inuse_check_error_cb(
switch(error->code) {
case PURPLE_KEYRING_ERROR_NOCAP:
purple_debug_info("keyring",
- "Keyring could not save password for account %s: %s.\n",
+ "Keyring does not support saving a password for account %s: %s.\n",
name, error->message);
break;
case PURPLE_KEYRING_ERROR_NOPASSWD:
- purple_debug_info("keyring",
- "No password found while changing keyring for account %s: %s.\n",
- name, error->message);
+ if (purple_debug_is_verbose()) {
+ purple_debug_misc("keyring",
+ "No password found while changing keyring for account %s: %s.\n",
+ name, error->message);
+ }
break;
case PURPLE_KEYRING_ERROR_NOCHANNEL:
- purple_debug_info("keyring",
+ purple_debug_error("keyring",
"Failed to communicate with backend while changing keyring for account %s: %s. Aborting changes.\n",
name, error->message);
tracker->abort = TRUE;
break;
default:
- purple_debug_info("keyring",
- "Unknown error while changing keyring for account %s: %s.\n",
+ purple_debug_error("keyring",
+ "Unknown error while changing keyring for account %s: %s. Aborting changes.\n",
name, error->message);
+ tracker->abort = TRUE;
break;
}
}
/* if this was the last one */
- if (tracker->finished == TRUE && tracker->read_outstanding == 0) {
- if (tracker->abort == TRUE && tracker->force == FALSE) {
+ if ((tracker->finished && tracker->read_outstanding == 0) || (tracker->abort && !tracker->force)) {
+ if (tracker->abort && !tracker->force) {
if (tracker->cb != NULL)
tracker->cb(tracker->old, FALSE, tracker->error, tracker->data);
@@ -468,7 +469,7 @@ purple_keyring_set_inuse_check_error_cb(
if (close != NULL)
close(NULL);
- purple_debug_info("keyring",
+ purple_debug_error("keyring",
"Failed to change keyring, aborting.\n");
purple_notify_error(NULL, _("Keyrings"), _("Failed to change the keyring."),
@@ -526,14 +527,12 @@ purple_keyring_set_inuse_got_pw_cb(Purpl
if (error->code == PURPLE_KEYRING_ERROR_NOPASSWD ||
error->code == PURPLE_KEYRING_ERROR_NOACCOUNT ||
tracker->force == TRUE) {
- /* don't save password, and directly trigger callback */
- purple_keyring_set_inuse_check_error_cb(account, error, data);
-
+ /* don't save password, and ignore it */
} else {
/* fatal error, abort all */
tracker->abort = TRUE;
}
-
+ purple_keyring_set_inuse_check_error_cb(account, error, data);
} else {
save = purple_keyring_get_save_password(new);
diff --git a/libpurple/plugins/keyrings/gnomekeyring.c b/libpurple/plugins/keyrings/gnomekeyring.c
--- a/libpurple/plugins/keyrings/gnomekeyring.c
+++ b/libpurple/plugins/keyrings/gnomekeyring.c
@@ -41,8 +41,6 @@
#define GNOMEKEYRING_AUTHOR "Scrouaf (scrouaf[at]soc.pidgin.im)"
#define GNOMEKEYRING_ID "keyring-gnomekeyring"
-#define ERR_GNOMEKEYRINGPLUGIN gkp_error_domain()
-
static PurpleKeyring *keyring_handler = NULL;
typedef struct _InfoStorage InfoStorage;
@@ -54,12 +52,6 @@ struct _InfoStorage
gpointer user_data;
};
-static GQuark gkp_error_domain(void)
-{
- return g_quark_from_static_string("Gnome-Keyring plugin");
-}
-
-
/***********************************************/
/* Keyring interface */
/***********************************************/
@@ -77,7 +69,7 @@ gkp_read_continue(GnomeKeyringResult res
if (result != GNOME_KEYRING_RESULT_OK) {
switch(result) {
case GNOME_KEYRING_RESULT_NO_MATCH:
- error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+ error = g_error_new(PURPLE_KEYRING_ERROR,
PURPLE_KEYRING_ERROR_NOPASSWD,
"No password found for account : %s",
purple_account_get_username(account));
@@ -88,7 +80,7 @@ gkp_read_continue(GnomeKeyringResult res
case GNOME_KEYRING_RESULT_NO_KEYRING_DAEMON:
case GNOME_KEYRING_RESULT_IO_ERROR:
- error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+ error = g_error_new(PURPLE_KEYRING_ERROR,
PURPLE_KEYRING_ERROR_NOCHANNEL,
"Failed to communicate with GNOME Keyring (account : %s).",
purple_account_get_username(account));
@@ -98,7 +90,7 @@ gkp_read_continue(GnomeKeyringResult res
return;
default:
- error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+ error = g_error_new(PURPLE_KEYRING_ERROR,
PURPLE_KEYRING_ERROR_NOCHANNEL,
"Unknown error (account : %s).",
purple_account_get_username(account));
@@ -154,7 +146,7 @@ gkp_save_continue(GnomeKeyringResult res
"Could not update password for %s (%s) : not found.\n",
purple_account_get_username(account),
purple_account_get_protocol_id(account));
- error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+ error = g_error_new(PURPLE_KEYRING_ERROR,
PURPLE_KEYRING_ERROR_NOPASSWD,
"Could not update password for %s : not found",
purple_account_get_username(account));
@@ -169,7 +161,7 @@ gkp_save_continue(GnomeKeyringResult res
"Failed to communicate with GNOME Keyring (account : %s (%s)).\n",
purple_account_get_username(account),
purple_account_get_protocol_id(account));
- error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+ error = g_error_new(PURPLE_KEYRING_ERROR,
PURPLE_KEYRING_ERROR_NOCHANNEL,
"Failed to communicate with GNOME Keyring (account : %s).",
purple_account_get_username(account));
@@ -183,7 +175,7 @@ gkp_save_continue(GnomeKeyringResult res
"Unknown error (account : %s (%s)).\n",
purple_account_get_username(account),
purple_account_get_protocol_id(account));
- error = g_error_new(ERR_GNOMEKEYRINGPLUGIN,
+ error = g_error_new(PURPLE_KEYRING_ERROR,
PURPLE_KEYRING_ERROR_NOCHANNEL,
"Unknown error (account : %s).",
purple_account_get_username(account));
diff --git a/libpurple/plugins/keyrings/internalkeyring.c b/libpurple/plugins/keyrings/internalkeyring.c
--- a/libpurple/plugins/keyrings/internalkeyring.c
+++ b/libpurple/plugins/keyrings/internalkeyring.c
@@ -79,10 +79,12 @@ internal_keyring_read(PurpleAccount *acc
if (cb != NULL)
cb(account, password, NULL, data);
} else {
- purple_debug_misc("keyring-internal",
- "No password for account %s (%s).\n",
- purple_account_get_username(account),
- purple_account_get_protocol_id(account));
+ if (purple_debug_is_verbose()) {
+ purple_debug_misc("keyring-internal",
+ "No password for account %s (%s).\n",
+ purple_account_get_username(account),
+ purple_account_get_protocol_id(account));
+ }
error = g_error_new(PURPLE_KEYRING_ERROR,
PURPLE_KEYRING_ERROR_NOPASSWD, "Password not found.");
if (cb != NULL)
@@ -95,8 +97,11 @@ static void
internal_keyring_save(PurpleAccount *account, const gchar *password,
PurpleKeyringSaveCallback cb, gpointer data)
{
+ void *old_password;
internal_keyring_open();
+ old_password = g_hash_table_lookup(internal_keyring_passwords, account);
+
if (password == NULL)
g_hash_table_remove(internal_keyring_passwords, account);
else {
@@ -104,11 +109,18 @@ internal_keyring_save(PurpleAccount *acc
g_strdup(password));
}
- purple_debug_misc("keyring-internal",
- "Password %s for account %s (%s).\n",
- (password == NULL ? "removed" : "saved"),
- purple_account_get_username(account),
- purple_account_get_protocol_id(account));
+ if (!(password == NULL && old_password == NULL)) {
+ purple_debug_misc("keyring-internal",
+ "Password %s for account %s (%s).\n",
+ (password == NULL ? "removed" : (old_password == NULL ? "saved" : "updated")),
+ purple_account_get_username(account),
+ purple_account_get_protocol_id(account));
+ } else if (purple_debug_is_verbose()) {
+ purple_debug_misc("keyring-internal",
+ "Password for account %s (%s) was already removed.\n",
+ purple_account_get_username(account),
+ purple_account_get_protocol_id(account));
+ }
if (cb != NULL)
cb(account, NULL, data);
diff --git a/libpurple/plugins/keyrings/kwallet.cpp b/libpurple/plugins/keyrings/kwallet.cpp
--- a/libpurple/plugins/keyrings/kwallet.cpp
+++ b/libpurple/plugins/keyrings/kwallet.cpp
@@ -50,8 +50,6 @@
PurpleKeyring *keyring_handler = NULL;
QCoreApplication *qCoreApp = NULL;
-#define ERR_KWALLETPLUGIN kwallet_plugin_error_domain()
-
namespace KWalletPlugin {
class request
@@ -124,12 +122,6 @@ class read_request : public request
PurpleKeyringReadCallback callback;
};
-static GQuark
-kwallet_plugin_error_domain(void)
-{
- return g_quark_from_static_string("KWallet keyring");
-}
-
}
static gboolean
@@ -147,7 +139,7 @@ KWalletPlugin::request::~request()
void
KWalletPlugin::request::abort()
{
- detailedAbort(PURPLE_KEYRING_ERROR_UNKNOWN);
+ detailedAbort(PURPLE_KEYRING_ERROR_NOCHANNEL);
}
KWalletPlugin::engine::engine()
@@ -342,7 +334,7 @@ KWalletPlugin::save_request::detailedAbo
if (callback == NULL)
return;
- gerror = g_error_new(ERR_KWALLETPLUGIN, error,
+ gerror = g_error_new(PURPLE_KEYRING_ERROR, error,
"Failed to save password");
callback(account, gerror, data);
g_error_free(gerror);
@@ -355,7 +347,7 @@ KWalletPlugin::read_request::detailedAbo
if (callback == NULL)
return;
- gerror = g_error_new(ERR_KWALLETPLUGIN, error,
+ gerror = g_error_new(PURPLE_KEYRING_ERROR, error,
"Failed to read password");
callback(account, NULL, gerror, data);
g_error_free(gerror);
diff --git a/libpurple/plugins/keyrings/secretservice.c b/libpurple/plugins/keyrings/secretservice.c
--- a/libpurple/plugins/keyrings/secretservice.c
+++ b/libpurple/plugins/keyrings/secretservice.c
@@ -33,8 +33,6 @@
#define SECRETSERVICE_NAME N_("Secret Service")
#define SECRETSERVICE_ID "keyring-libsecret"
-#define ERR_SECRETSERVICEPLUGIN (ss_error_domain())
-
static PurpleKeyring *keyring_handler = NULL;
static const SecretSchema purple_schema = {
@@ -57,13 +55,6 @@ struct _InfoStorage
gpointer user_data;
};
-static GQuark
-ss_error_domain(void)
-{
- return g_quark_from_static_string("SecretService plugin");
-}
-
-
/***********************************************/
/* Keyring interface */
/***********************************************/
@@ -85,7 +76,7 @@ ss_read_continue(GObject *object, GAsync
switch (code) {
More information about the Commits
mailing list