pidgin/pidgin

Don't error out when trying to remove a password that isn't stored.

2021-08-23, Gary Kramlich
2918459dbd63
Parents ec8b76f3bf0a
Children 920f2790dca3
Don't error out when trying to remove a password that isn't stored.

Both wincred and secretservice return an error if you try to delete a password
that they don't know about. We don't care about this scenario and just want to
make sure the password is gone, so we changed the expected behavior to be as
such and updated wincred and secret service for the change.

I also fixed the NULL deference which is how I stumbled across the issue in the
first place.

Testing Done:
* Verfied the issue was fixed in secretservice.
* Verfied the issue didn't exist in kwallet.

Reviewed at https://reviews.imfreedom.org/r/901/
5 files changed, 29 insertions(+), 27 deletions(-)
  • +1 -1
    libpurple/accounts.c
  • +7 -3
    libpurple/plugins/keyrings/secretservice.c
  • +17 -19
    libpurple/plugins/keyrings/wincred.c
  • +2 -2
    libpurple/purplecredentialmanager.h
  • +2 -2
    libpurple/purplecredentialprovider.h
    • --- a/libpurple/accounts.c Mon Aug 16 03:10:08 2021 -0500
    +++ b/libpurple/accounts.c Mon Aug 23 07:16:01 2021 -0500
    @@ -573,7 +573,7 @@
    purple_debug_warning("accounts",
    "Failed to remove password for account %s: %s",
    purple_account_get_name_for_display(account),
    - error->message);
    + (error != NULL) ? error->message : "Unknown error");
    g_clear_error(&error);
    }
    --- a/libpurple/plugins/keyrings/secretservice.c Mon Aug 16 03:10:08 2021 -0500
    +++ b/libpurple/plugins/keyrings/secretservice.c Mon Aug 23 07:16:01 2021 -0500
    @@ -123,14 +123,18 @@
    {
    GTask *task = G_TASK(data);
    GError *error = NULL;
    - gboolean ret = FALSE;
    - ret = secret_password_clear_finish(result, &error);
    + /* This returns whether a password was removed or not. Which means that it
    + * can return FALSE with error unset. This would complicate all of the other
    + * credential API and we don't need to make this distinction, so we just
    + * return TRUE unless error is set.
    + */
    + secret_password_clear_finish(result, &error);
    if(error != NULL) {
    g_task_return_error(task, error);
    } else {
    - g_task_return_boolean(task, ret);
    + g_task_return_boolean(task, TRUE);
    }
    g_object_unref(G_OBJECT(task));
    --- a/libpurple/plugins/keyrings/wincred.c Mon Aug 16 03:10:08 2021 -0500
    +++ b/libpurple/plugins/keyrings/wincred.c Mon Aug 23 07:16:01 2021 -0500
    @@ -292,28 +292,26 @@
    DWORD error_code = GetLastError();
    if (error_code == ERROR_NOT_FOUND) {
    - if (purple_debug_is_verbose()) {
    - purple_debug_misc(
    + /* If there was no password we just return TRUE. */
    + g_task_return_boolean(task, TRUE);
    + } else {
    + if (error_code == ERROR_NO_SUCH_LOGON_SESSION) {
    + purple_debug_error(
    "keyring-wincred",
    - "Password for account %s was already removed.",
    - purple_account_get_username(account));
    + "Cannot remove password, no valid logon session");
    + error = g_error_new(
    + PURPLE_WINCRED_ERROR, error_code,
    + _("Cannot remove password, no valid logon session."));
    + } else {
    + purple_debug_error("keyring-wincred",
    + "Cannot remove password, error %lx", error_code);
    + error = g_error_new(
    + PURPLE_WINCRED_ERROR, error_code,
    + _("Cannot remove password (error %lx)."), error_code);
    }
    - } else if (error_code == ERROR_NO_SUCH_LOGON_SESSION) {
    - purple_debug_error(
    - "keyring-wincred",
    - "Cannot remove password, no valid logon session");
    - error = g_error_new(
    - PURPLE_WINCRED_ERROR, error_code,
    - _("Cannot remove password, no valid logon session."));
    - } else {
    - purple_debug_error("keyring-wincred",
    - "Cannot remove password, error %lx", error_code);
    - error = g_error_new(
    - PURPLE_WINCRED_ERROR, error_code,
    - _("Cannot remove password (error %lx)."), error_code);
    +
    + g_task_return_error(task, error);
    }
    -
    - g_task_return_error(task, error);
    }
    g_object_unref(G_OBJECT(task));
    --- a/libpurple/purplecredentialmanager.h Mon Aug 16 03:10:08 2021 -0500
    +++ b/libpurple/purplecredentialmanager.h Mon Aug 23 07:16:01 2021 -0500
    @@ -270,8 +270,8 @@
    * Finishes a previous call to
    * purple_credential_provider_clear_password_async().
    *
    - * Returns: %TRUE if the password was cleared successfully, otherwise %FALSE
    - * with @error set.
    + * Returns: %TRUE if the password didn't exist or was cleared successfully,
    + * otherwise %FALSE with @error set.
    *
    * Since: 3.0.0
    */
    --- a/libpurple/purplecredentialprovider.h Mon Aug 16 03:10:08 2021 -0500
    +++ b/libpurple/purplecredentialprovider.h Mon Aug 23 07:16:01 2021 -0500
    @@ -269,8 +269,8 @@
    * Finishes a previous call to
    * purple_credential_provider_clear_password_async().
    *
    - * Returns: %TRUE if the password was cleared successfully, otherwise %FALSE
    - * with @error set.
    + * Returns: %TRUE if the password didn't exist or was cleared successfully,
    + * otherwise %FALSE with @error set.
    *
    * Since: 3.0.0
    */