/cpw/tomkiewicz/masterpassword: 544f05bc883d: Don't allow regist...

Tomasz Wasilczyk tomkiewicz at cpw.pidgin.im
Tue Mar 26 11:08:26 EDT 2013


Changeset: 544f05bc883dd22cbe82163a63a6f2567de1216f
Author:	 Tomasz Wasilczyk <tomkiewicz at cpw.pidgin.im>
Date:	 2013-03-26 16:08 +0100
Branch:	 soc.2008.masterpassword
URL: https://hg.pidgin.im/cpw/tomkiewicz/masterpassword/rev/544f05bc883d

Description:

Don't allow registering invalid keyring

diffstat:

 libpurple/keyring.c |  161 ++++++++++++++++++---------------------------------
 1 files changed, 56 insertions(+), 105 deletions(-)

diffs (260 lines):

diff --git a/libpurple/keyring.c b/libpurple/keyring.c
--- a/libpurple/keyring.c
+++ b/libpurple/keyring.c
@@ -1,8 +1,6 @@
 /**
  * @file keyring.c Keyring plugin API
  * @ingroup core
- * @todo ? : add dummy callback to all calls to prevent poorly written
- * plugins from segfaulting on NULL callback ?
  */
 
 /* purple
@@ -188,6 +186,7 @@ void
 purple_keyring_set_name(PurpleKeyring *keyring, const char *name)
 {
 	g_return_if_fail(keyring != NULL);
+	g_return_if_fail(name != NULL);
 
 	g_free(keyring->name);
 	keyring->name = g_strdup(name);
@@ -197,6 +196,7 @@ void
 purple_keyring_set_id(PurpleKeyring *keyring, const char *id)
 {
 	g_return_if_fail(keyring != NULL);
+	g_return_if_fail(id != NULL);
 
 	g_free(keyring->id);
 	keyring->id = g_strdup(id);
@@ -206,6 +206,7 @@ void
 purple_keyring_set_read_password(PurpleKeyring *keyring, PurpleKeyringRead read)
 {
 	g_return_if_fail(keyring != NULL);
+	g_return_if_fail(read != NULL);
 
 	keyring->read_password = read;
 }
@@ -214,6 +215,7 @@ void
 purple_keyring_set_save_password(PurpleKeyring *keyring, PurpleKeyringSave save)
 {
 	g_return_if_fail(keyring != NULL);
+	g_return_if_fail(save != NULL);
 
 	keyring->save_password = save;
 }
@@ -490,11 +492,7 @@ purple_keyring_drop_passwords(const Purp
 	g_return_if_fail(keyring != NULL);
 
 	save = purple_keyring_get_save_password(keyring);
-	if (save == NULL) {
-		if (cb)
-			cb(data);
-		return;
-	}
+	g_return_if_fail(save != NULL);
 
 	tracker = g_new0(PurpleKeyringDropTracker, 1);
 	tracker->cb = cb;
@@ -658,21 +656,10 @@ purple_keyring_set_inuse_got_pw_cb(Purpl
 		purple_keyring_set_inuse_check_error_cb(account, error, data);
 	} else {
 		save = purple_keyring_get_save_password(new);
+		g_return_if_fail(save != NULL);
 
-		if (save != NULL) {
-			/* this test is probably totally useless, since there's no point
-			 * in having a keyring that can't store passwords, but it
-			 * will prevent crash with invalid keyrings
-			 */
-			save(account, password,
-			     purple_keyring_set_inuse_check_error_cb, tracker);
-
-		} else {
-			error = g_error_new(PURPLE_KEYRING_ERROR, PURPLE_KEYRING_ERROR_NOCAP,
-				"cannot store passwords in new keyring");
-			purple_keyring_set_inuse_check_error_cb(account, error, data);
-			g_error_free(error);
-		}
+		save(account, password, purple_keyring_set_inuse_check_error_cb,
+			tracker);
 	}
 }
 
@@ -685,7 +672,6 @@ purple_keyring_set_inuse(const PurpleKey
 	GList *cur;
 	const PurpleKeyring *oldkeyring;
 	PurpleKeyringRead read = NULL;
-	PurpleKeyringClose close;
 	PurpleKeyringChangeTracker *tracker;
 
 	oldkeyring = purple_keyring_get_inuse();
@@ -718,62 +704,38 @@ purple_keyring_set_inuse(const PurpleKey
 
 	if (oldkeyring != NULL) {
 		read = purple_keyring_get_read_password(oldkeyring);
+		g_return_if_fail(read != NULL);
 
-		if (read == NULL) { /* TODO: don't allow registering a keyring without read/save callback */
-			/*
-			error = g_error_new(PURPLE_KEYRING_ERROR, PURPLE_KEYRING_ERROR_NOCAP,
-				"Existing keyring cannot read passwords");
-			*/
-			purple_debug_info("keyring", "Existing keyring cannot read passwords.\n");
+		purple_debug_misc("keyring", "Starting migration from: %s.\n",
+			oldkeyring->id);
 
-			/* at this point, we know the keyring won't let us
-			 * read passwords, so there no point in copying them.
-			 * therefore we just cleanup the old and setup the new
-			 * one later.
-			 */
+		tracker = g_new(PurpleKeyringChangeTracker, 1);
+		current_change_tracker = tracker;
 
-			purple_keyring_drop_passwords(oldkeyring, NULL, NULL);
+		tracker->cb = cb;
+		tracker->data = data;
+		tracker->new = newkeyring;
+		tracker->old = oldkeyring;
+		tracker->read_outstanding = 0;
+		tracker->finished = FALSE;
+		tracker->abort = FALSE;
+		tracker->force = force;
+		tracker->error = NULL;
 
-			close = purple_keyring_get_close_keyring(oldkeyring);
+		for (cur = purple_accounts_get_all(); cur != NULL;
+			cur = cur->next) {
 
-			if (close != NULL)
-				close(NULL);  /* we can't do much about errors at this point */
+			if (tracker->abort) {
+				tracker->finished = TRUE;
+				break;
+			}
+			tracker->read_outstanding++;
 
-		} else {
-			purple_debug_misc("keyring",
-				"Starting migration from: %s.\n",
-				oldkeyring->id);
+			if (cur->next == NULL)
+				tracker->finished = TRUE;
 
-			tracker = g_new(PurpleKeyringChangeTracker, 1);
-			current_change_tracker = tracker;
-
-			tracker->cb = cb;
-			tracker->data = data;
-			tracker->new = newkeyring;
-			tracker->old = oldkeyring;
-			tracker->read_outstanding = 0;
-			tracker->finished = FALSE;
-			tracker->abort = FALSE;
-			tracker->force = force;
-			tracker->error = NULL;
-
-			for (cur = purple_accounts_get_all();
-			    cur != NULL;
-			    cur = cur->next) {
-				
-				if (tracker->abort) {
-					tracker->finished = TRUE;
-					break;
-				}
-				tracker->read_outstanding++;
-
-				if (cur->next == NULL)
-					tracker->finished = TRUE;
-
-				read(cur->data, purple_keyring_set_inuse_got_pw_cb, tracker);
-			}
+			read(cur->data, purple_keyring_set_inuse_got_pw_cb, tracker);
 		}
-
 	} else { /* no keyring was set before. */
 		if (purple_debug_is_verbose()) {
 			purple_debug_misc("keyring", "Setting keyring for the "
@@ -823,11 +785,18 @@ purple_keyring_register(PurpleKeyring *k
 
 	keyring_id = purple_keyring_get_id(keyring);
 
-	/* keyring with no ID. Add error handling ? */
-	g_return_if_fail(keyring_id != NULL);
+	purple_debug_info("keyring", "Registering keyring: %s.\n",
+		keyring->id ? keyring->id : "(null)");
 
-	purple_debug_info("keyring", "Registering keyring: %s.\n",
-		keyring->id);
+	if (purple_keyring_get_id(keyring) == NULL ||
+		purple_keyring_get_name(keyring) == NULL ||
+		purple_keyring_get_read_password(keyring) == NULL ||
+		purple_keyring_get_save_password(keyring) == NULL) {
+		purple_debug_error("keyring", "Invalid keyring %s, some "
+			"required fields are missing.\n",
+			keyring->id ? keyring->id : "(null)");
+		return;
+	}
 
 	/* If this is the configured keyring, use it. */
 	if (purple_keyring_inuse == NULL &&
@@ -1057,23 +1026,12 @@ purple_keyring_get_password(PurpleAccoun
 
 		} else {
 			read = purple_keyring_get_read_password(inuse);
+			g_return_if_fail(read != NULL);
 
-			if (read == NULL) {
-				purple_debug_warning("keyring", "Keyring cannot read password.\n");
-				error = g_error_new(PURPLE_KEYRING_ERROR, PURPLE_KEYRING_ERROR_NOCAP,
-					"Keyring cannot read password.");
-
-				if (cb != NULL)
-					cb(account, NULL, error, data);
-
-				g_error_free(error);
-
-			} else {
-				purple_debug_info("keyring", "Reading password for account %s (%s)...\n",
-					purple_account_get_username(account),
-					purple_account_get_protocol_id(account));
-				read(account, cb, data);
-			}
+			purple_debug_info("keyring", "Reading password for account %s (%s)...\n",
+				purple_account_get_username(account),
+				purple_account_get_protocol_id(account));
+			read(account, cb, data);
 		}
 	}
 }
@@ -1158,22 +1116,15 @@ purple_keyring_set_password(PurpleAccoun
 
 	} else {
 		save = purple_keyring_get_save_password(inuse);
-		if (save == NULL) {
-			error = g_error_new(PURPLE_KEYRING_ERROR, PURPLE_KEYRING_ERROR_NOCAP,
-				"Keyring cannot save password.");
-			if (cb != NULL)
-				cb(account, error, data);
-			g_error_free(error);
+		g_return_if_fail(save != NULL);
 
-		} else {
-			cbinfo = g_new(PurpleKeyringCbInfo, 1);
-			cbinfo->cb = cb;
-			cbinfo->data = data;
-			purple_debug_info("keyring", "Saving password for account %s (%s)...\n",
-				purple_account_get_username(account),
-				purple_account_get_protocol_id(account));
-			save(account, password, purple_keyring_set_password_async_cb, cbinfo);
-		}
+		cbinfo = g_new(PurpleKeyringCbInfo, 1);
+		cbinfo->cb = cb;
+		cbinfo->data = data;
+		purple_debug_info("keyring", "Saving password for account %s (%s)...\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+		save(account, password, purple_keyring_set_password_async_cb, cbinfo);
 	}
 }
 



More information about the Commits mailing list