From d2aa894dc3378e9ce7a324e45e4f20fef53da8ba Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 3 Aug 2011 08:32:16 +0100 Subject: [PATCH] MUC: don't forget password when handling nick conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WockyMuc has a property, :password, representing the current password being used to join the MUC. GabbleMuc previously had a private variable, 'password', which was used for this. In the port to WockyMuc, setting the private variable was removed, but it was still used when re-attempting to join after a nick conflict. (I think the password should be a parameter to a hypothetical wocky_muc_join_async() which does all the nick conflict crap for you. Having this as state that kicks around on the WockyMuc for ever is bizarre—once you're in the room, you usually don't use the password, unless you're the owner, in which case you can retrieve the current password *which may be different*!) This patch expunges the zombie private variable, and ensures WockyMuc:password is only set when the user provides a password, not at every join attempt. It adds a test for this case, and some of the basic functionality of Password (which subsumes some incidental testing of the Password interface in muc/presence-before-closing: the only test that touched Password at all!). Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=39790 --- src/muc-channel.c | 18 +++------- tests/twisted/Makefile.am | 1 + tests/twisted/muc/presence-before-closing.py | 46 ++++++++++++------------- 3 files changed, 28 insertions(+), 37 deletions(-) diff --git a/src/muc-channel.c b/src/muc-channel.c index ad91639..ebb829f 100644 --- a/src/muc-channel.c +++ b/src/muc-channel.c @@ -230,7 +230,6 @@ struct _GabbleMucChannelPrivate TpChannelPasswordFlags password_flags; DBusGMethodInvocation *password_ctx; - gchar *password; const gchar *jid; gboolean requested; @@ -803,12 +802,10 @@ create_room_identity (GabbleMucChannel *chan) } static void -send_join_request (GabbleMucChannel *gmuc, - const gchar *password) +send_join_request (GabbleMucChannel *gmuc) { GabbleMucChannelPrivate *priv = gmuc->priv; - g_object_set (priv->wmuc, "password", password, NULL); wocky_muc_join (priv->wmuc, NULL); } @@ -1194,8 +1191,6 @@ gabble_muc_channel_finalize (GObject *object) g_string_free (priv->self_jid, TRUE); } - g_free (priv->password); - if (priv->initial_channels != NULL) { g_boxed_free (TP_ARRAY_TYPE_OBJECT_PATH_LIST, priv->initial_channels); @@ -1365,10 +1360,6 @@ channel_state_changed (GabbleMucChannel *chan, priv->poll_timer_id = g_timeout_add_seconds (interval, timeout_poll, chan); - - /* no need to keep this around any longer, if it's set */ - g_free (priv->password); - priv->password = NULL; } else if (new_state == MUC_STATE_ENDED) { @@ -1518,7 +1509,7 @@ handle_nick_conflict (GabbleMucChannel *chan, tp_handle_unref (contact_repo, self_handle); priv->nick_retry_count++; - send_join_request (chan, priv->password); + send_join_request (chan); return TRUE; } @@ -2941,7 +2932,8 @@ gabble_muc_channel_provide_password (TpSvcChannelInterfacePassword *iface, } else { - send_join_request (self, password); + g_object_set (priv->wmuc, "password", password, NULL); + send_join_request (self); priv->password_ctx = context; } } @@ -3074,7 +3066,7 @@ gabble_muc_channel_add_member (GObject *obj, tp_intset_destroy (set_remote_pending); /* seek to enter the room */ - send_join_request (self, NULL); + send_join_request (self); g_object_set (obj, "state", MUC_STATE_INITIATED, NULL); /* deny adding */ diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am index 311deb7..3bd42c7 100644 --- a/tests/twisted/Makefile.am +++ b/tests/twisted/Makefile.am @@ -25,6 +25,7 @@ TWISTED_TESTS = \ muc/chat-states.py \ muc/kicked.py \ muc/name-conflict.py \ + muc/password.py \ muc/presence-before-closing.py \ muc/renamed.py \ muc/roomlist.py \ diff --git a/tests/twisted/muc/presence-before-closing.py b/tests/twisted/muc/presence-before-closing.py index c221ca2..b89abef 100644 --- a/tests/twisted/muc/presence-before-closing.py +++ b/tests/twisted/muc/presence-before-closing.py @@ -6,7 +6,9 @@ import dbus from twisted.words.xish import domish -from gabbletest import exec_test, make_result_iq, request_muc_handle, wrap_channel +from gabbletest import ( + exec_test, make_result_iq, request_muc_handle, wrap_channel, elem, +) from servicetest import (EventPattern, assertEquals, assertLength, assertContains, sync_dbus, call_async) import constants as cs @@ -111,31 +113,27 @@ def test_with_password(q, bus, conn, stream): cs.TARGET_HANDLE_TYPE: cs.HT_ROOM, cs.TARGET_HANDLE: handle}) - q.expect('stream-presence', to='chat@conf.localhost/test') + expected_muc_jid = '%s/%s' % (room, 'test') + q.expect('stream-presence', to=expected_muc_jid) # tell gabble the room needs a password - presence = domish.Element(('jabber:client', 'presence')) - presence['from'] = '%s/%s' % (room, 'test') - presence['type'] = 'error' - x = presence.addElement((ns.MUC, 'x')) - error = presence.addElement('error') - error['type'] = 'auth' - not_authorized = error.addElement((ns.STANZA, 'not-authorized')) - stream.send(presence) - - cc, _, _ = q.expect_many(EventPattern('dbus-return', method='CreateChannel'), - EventPattern('dbus-signal', signal='NewChannels'), - EventPattern('dbus-signal', signal='PasswordFlagsChanged', - args=[cs.PASSWORD_FLAG_PROVIDE, 0])) - - chan = wrap_channel(bus.get_object(conn.bus_name, cc.value[0]), - 'Text', ['Password']) - - # ensure gabble knows we need a password - flags = chan.Password.GetPasswordFlags() - assertEquals(cs.PASSWORD_FLAG_PROVIDE, flags) - - forbidden = [EventPattern('stream-presence', to=room + '/test')] + stream.send( + elem('jabber:client', 'presence', from_=expected_muc_jid, + type='error')( + elem(ns.MUC, 'x'), + elem('error', type='auth')( + elem(ns.STANZA, 'not-authorized'), + ), + )) + + cc, _ = q.expect_many( + EventPattern('dbus-return', method='CreateChannel'), + EventPattern('dbus-signal', signal='PasswordFlagsChanged', + args=[cs.PASSWORD_FLAG_PROVIDE, 0])) + + chan = wrap_channel(bus.get_object(conn.bus_name, cc.value[0]), 'Text') + + forbidden = [EventPattern('stream-presence', to=expected_muc_jid)] q.forbid_events(forbidden) # we call Close... -- 1.7.5.4