From 3e3c09c6692b526c3ed7ca94bab99a2f7ca2c887 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 25 Sep 2013 19:38:05 +0100 Subject: [PATCH 3/5] McdConnection: refuse to deal with pre-Requests connections If it implements Requests, all is good; if it doesn't, just disconnect it before it can cause trouble (e.g. new channels turning up and not being dispatched correctly). --- src/mcd-connection.c | 143 ++++++------------- tests/twisted/account-manager/device-idle.py | 2 +- tests/twisted/constants.py | 1 + tests/twisted/dispatcher/already-has-obsolete.py | 107 +-------------- tests/twisted/dispatcher/connect-for-request.py | 2 +- .../dispatcher/dispatch-before-connected.py | 2 +- tests/twisted/dispatcher/dispatch-obsolete.py | 152 +-------------------- tests/twisted/mctest.py | 9 -- 8 files changed, 55 insertions(+), 363 deletions(-) diff --git a/src/mcd-connection.c b/src/mcd-connection.c index fd582ec..c04b63c 100644 --- a/src/mcd-connection.c +++ b/src/mcd-connection.c @@ -174,7 +174,8 @@ static const gchar * const *presence_fallbacks[] = { }; static void _mcd_connection_release_tp_connection (McdConnection *connection, - McdInhibit *inhibit); + McdInhibit *inhibit, + gboolean already_signalled); static gboolean request_channel_new_iface (McdConnection *connection, McdChannel *channel); @@ -488,40 +489,6 @@ _mcd_connection_request_presence (McdConnection *self, } static void -on_new_channel (TpConnection *proxy, const gchar *chan_obj_path, - const gchar *chan_type, guint handle_type, guint handle, - gboolean suppress_handler, gpointer user_data, - GObject *weak_object) -{ - McdConnection *connection = MCD_CONNECTION (weak_object); - McdConnectionPrivate *priv = user_data; - McdChannel *channel; - - DEBUG ("%s (t=%s, ht=%u, h=%u, suppress=%c)", - chan_obj_path, chan_type, handle_type, handle, - suppress_handler ? 'T' : 'F'); - - if (priv->dispatched_initial_channels) - { - channel = mcd_channel_new_from_path (proxy, - chan_obj_path, - chan_type, handle, handle_type); - if (G_UNLIKELY (!channel)) return; - mcd_operation_take_mission (MCD_OPERATION (connection), - MCD_MISSION (channel)); - - /* MC no longer calls RequestChannel. As a result, if suppress_handler - * is TRUE, we know that this channel was requested "behind our back", - * therefore we should call ObserveChannels, but refrain from calling - * AddDispatchOperation or HandleChannels. - * - * We assume that channels without suppress_handler are incoming. */ - _mcd_dispatcher_add_channel (priv->dispatcher, channel, - suppress_handler, suppress_handler); - } -} - -static void _foreach_channel_remove (McdMission * mission, McdOperation * operation) { g_assert (MCD_IS_MISSION (mission)); @@ -739,7 +706,7 @@ mcd_connection_invalidated_cb (TpConnection *tp_conn, DEBUG ("Proxy destroyed (%s)!", message); - _mcd_connection_release_tp_connection (connection, NULL); + _mcd_connection_release_tp_connection (connection, NULL, FALSE); if (priv->connected && priv->abort_reason != TP_CONNECTION_STATUS_REASON_REQUESTED && @@ -1084,65 +1051,6 @@ mcd_connection_setup_requests (McdConnection *connection) } static void -list_channels_cb (TpConnection *connection, - const GPtrArray *structs, - const GError *error, - gpointer user_data, - GObject *weak_object) -{ - McdConnection *self = MCD_CONNECTION (weak_object); - guint i; - - if (error) - { - g_warning ("ListChannels got error: %s", error->message); - return; - } - - for (i = 0; i < structs->len; i++) - { - GValueArray *va = g_ptr_array_index (structs, i); - const gchar *object_path; - GHashTable *channel_props; - - object_path = g_value_get_boxed (va->values + 0); - - DEBUG ("%s (t=%s, ht=%u, h=%u)", - object_path, - g_value_get_string (va->values + 1), - g_value_get_uint (va->values + 2), - g_value_get_uint (va->values + 3)); - - /* this is not the most efficient thing we could possibly do, but - * we're on a fallback path so it's OK to be a bit slow */ - channel_props = g_hash_table_new (g_str_hash, g_str_equal); - g_hash_table_insert (channel_props, TP_IFACE_CHANNEL ".ChannelType", - va->values + 1); - g_hash_table_insert (channel_props, TP_IFACE_CHANNEL ".TargetHandleType", - va->values + 2); - g_hash_table_insert (channel_props, TP_IFACE_CHANNEL ".TargetHandle", - va->values + 3); - mcd_connection_found_channel (self, object_path, channel_props); - g_hash_table_unref (channel_props); - } - - self->priv->dispatched_initial_channels = TRUE; -} - -static void -mcd_connection_setup_pre_requests (McdConnection *connection) -{ - McdConnectionPrivate *priv = connection->priv; - - tp_cli_connection_connect_to_new_channel - (priv->tp_conn, on_new_channel, priv, NULL, - (GObject *)connection, NULL); - - tp_cli_connection_call_list_channels (priv->tp_conn, -1, - list_channels_cb, priv, NULL, (GObject *) connection); -} - -static void on_connection_ready (GObject *source_object, GAsyncResult *result, gpointer user_data) { @@ -1162,6 +1070,28 @@ on_connection_ready (GObject *source_object, GAsyncResult *result, if (!connection) goto finally; + if (!tp_proxy_has_interface_by_id (tp_conn, + TP_IFACE_QUARK_CONNECTION_INTERFACE_REQUESTS)) + { + GHashTable *asv; + + DEBUG ("%s: connection manager is too old", + tp_proxy_get_object_path (tp_conn)); + connection->priv->abort_reason = + TP_CONNECTION_STATUS_REASON_NONE_SPECIFIED; + asv = tp_asv_new ( + "debug-message", G_TYPE_STRING, + "Connection manager does not implement Requests interface", + NULL); + g_signal_emit (connection, signals[CONNECTION_STATUS_CHANGED], 0, + TP_CONNECTION_STATUS_DISCONNECTED, + connection->priv->abort_reason, + tp_conn, TP_ERROR_STR_SOFTWARE_UPGRADE_REQUIRED, asv); + g_hash_table_unref (asv); + _mcd_connection_release_tp_connection (connection, NULL, TRUE); + goto finally; + } + DEBUG ("connection is ready"); priv = MCD_CONNECTION_PRIV (connection); @@ -1203,11 +1133,9 @@ _mcd_connection_start_dispatching (McdConnection *self, self->priv->dispatching_started = TRUE; - if (tp_proxy_has_interface_by_id (self->priv->tp_conn, - TP_IFACE_QUARK_CONNECTION_INTERFACE_REQUESTS)) - mcd_connection_setup_requests (self); - else - mcd_connection_setup_pre_requests (self); + g_return_if_fail (tp_proxy_has_interface_by_id (self->priv->tp_conn, + TP_IFACE_QUARK_CONNECTION_INTERFACE_REQUESTS)); + mcd_connection_setup_requests (self); /* FIXME: why is this here? if we need to update caps before and after * * connected, it should be in the call_when_ready callback. */ @@ -1557,13 +1485,18 @@ _mcd_connection_finalize (GObject * object) static void _mcd_connection_release_tp_connection (McdConnection *connection, - McdInhibit *inhibit) + McdInhibit *inhibit, + gboolean already_signalled) { McdConnectionPrivate *priv = MCD_CONNECTION_PRIV (connection); DEBUG ("%p", connection); - if (priv->abort_reason == TP_CONNECTION_STATUS_REASON_REQUESTED) + if (already_signalled) + { + DEBUG ("already emitted connection-status-changed"); + } + else if (priv->abort_reason == TP_CONNECTION_STATUS_REASON_REQUESTED) { g_signal_emit (connection, signals[CONNECTION_STATUS_CHANGED], 0, TP_CONNECTION_STATUS_DISCONNECTED, @@ -1676,7 +1609,7 @@ _mcd_connection_dispose (GObject * object) mcd_operation_foreach (MCD_OPERATION (connection), (GFunc) _foreach_channel_remove, connection); - _mcd_connection_release_tp_connection (connection, NULL); + _mcd_connection_release_tp_connection (connection, NULL, FALSE); g_assert (priv->tp_conn == NULL); if (priv->account) @@ -2124,7 +2057,7 @@ mcd_connection_close (McdConnection *connection, connection->priv->closed = TRUE; connection->priv->abort_reason = TP_CONNECTION_STATUS_REASON_REQUESTED; - _mcd_connection_release_tp_connection (connection, inhibit); + _mcd_connection_release_tp_connection (connection, inhibit, FALSE); mcd_mission_abort (MCD_MISSION (connection)); } @@ -2258,7 +2191,7 @@ _mcd_connection_set_tp_connection (McdConnection *connection, } DEBUG ("releasing old connection first"); - _mcd_connection_release_tp_connection (connection, NULL); + _mcd_connection_release_tp_connection (connection, NULL, FALSE); } g_assert (priv->tp_conn == NULL); diff --git a/tests/twisted/account-manager/device-idle.py b/tests/twisted/account-manager/device-idle.py index 196b6a0..434544a 100644 --- a/tests/twisted/account-manager/device-idle.py +++ b/tests/twisted/account-manager/device-idle.py @@ -83,7 +83,7 @@ def _create_and_enable(q, bus, mc, account_name, power_saving_supported, params = dbus.Dictionary({"account": account_name, "password": "secrecy"}, signature='sv') cm_name_ref, account = create_fakecm_account(q, bus, mc, params) - conn = enable_fakecm_account(q, bus, mc, account, params, has_requests=False, + conn = enable_fakecm_account(q, bus, mc, account, params, extra_interfaces=extra_interfaces, expect_after_connect=expect_after_connect) diff --git a/tests/twisted/constants.py b/tests/twisted/constants.py index 9976110..6bb50c7 100644 --- a/tests/twisted/constants.py +++ b/tests/twisted/constants.py @@ -239,6 +239,7 @@ INVALID_HANDLE = ERROR + '.InvalidHandle' CERT_UNTRUSTED = ERROR + '.Cert.Untrusted' SERVICE_BUSY = ERROR + '.ServiceBusy' SERVICE_CONFUSED = ERROR + '.ServiceConfused' +SOFTWARE_UPGRADE_REQUIRED = ERROR + '.SoftwareUpgradeRequired' BANNED = ERROR + '.Channel.Banned' diff --git a/tests/twisted/dispatcher/already-has-obsolete.py b/tests/twisted/dispatcher/already-has-obsolete.py index 8788169..abc3a3c 100644 --- a/tests/twisted/dispatcher/already-has-obsolete.py +++ b/tests/twisted/dispatcher/already-has-obsolete.py @@ -113,109 +113,16 @@ def test(q, bus, mc): # Now reply to GetInterfaces and say we don't have Requests conn.GetInterfaces(get_interfaces_call) - q.expect('dbus-method-call', - interface=cs.CONN, method='ListChannels', args=[], - path=conn.object_path, handled=True) - - # A channel dispatch operation is created for the channel we already had - - e = q.expect('dbus-signal', - path=cs.CD_PATH, - interface=cs.CD_IFACE_OP_LIST, - signal='NewDispatchOperation') - - cdo_path = e.args[0] - cdo_properties = e.args[1] - - assert cdo_properties[cs.CDO + '.Account'] == account.object_path - assert cdo_properties[cs.CDO + '.Connection'] == conn.object_path - - handlers = cdo_properties[cs.CDO + '.PossibleHandlers'][:] - handlers.sort() - assert handlers == [cs.tp_name_prefix + '.Client.Empathy', - cs.tp_name_prefix + '.Client.Kopete'], handlers - - assert cs.CD_IFACE_OP_LIST in cd_props.Get(cs.CD, 'Interfaces') - assert cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations') ==\ - [(cdo_path, cdo_properties)] - - cdo = bus.get_object(cs.CD, cdo_path) - cdo_iface = dbus.Interface(cdo, cs.CDO) - - # Both Observers are told about the new channel - - e, k = q.expect_many( - EventPattern('dbus-method-call', - path=empathy.object_path, - interface=cs.OBSERVER, method='ObserveChannels', - handled=False), - EventPattern('dbus-method-call', - path=kopete.object_path, - interface=cs.OBSERVER, method='ObserveChannels', - handled=False), - ) - assert e.args[0] == account.object_path, e.args - assert e.args[1] == conn.object_path, e.args - assert e.args[3] == cdo_path, e.args - assert e.args[4] == [], e.args # no requests satisfied - channels = e.args[2] - assert len(channels) == 1, channels - assert channels[0][0] == chan.object_path, channels - - assert k.args == e.args - - # Both Observers indicate that they are ready to proceed - q.dbus_return(k.message, signature='') - q.dbus_return(e.message, signature='') - - # The Approvers are next - - e, k = q.expect_many( - EventPattern('dbus-method-call', - path=empathy.object_path, - interface=cs.APPROVER, method='AddDispatchOperation', - handled=False), - EventPattern('dbus-method-call', - path=kopete.object_path, - interface=cs.APPROVER, method='AddDispatchOperation', - handled=False), - ) - - assert len(e.args) == 3 - assert len(e.args[0]) == 1 - assert e.args[0][0][0] == chan.object_path - assert e.args[1] == cdo_path - assert e.args[2] == cdo_properties - assert k.args == e.args - - q.dbus_return(e.message, signature='') - q.dbus_return(k.message, signature='') - - # Both Approvers now have a flashing icon or something, trying to get the - # user's attention - - # The user responds to Empathy first - call_async(q, cdo_iface, 'HandleWith', - cs.tp_name_prefix + '.Client.Empathy') - - # Empathy is asked to handle the channels - e = q.expect('dbus-method-call', - path=empathy.object_path, - interface=cs.HANDLER, method='HandleChannels', - handled=False) - - # Empathy accepts the channels - q.dbus_return(e.message, signature='') + # MC shoots down the connection. Goodbye! q.expect_many( - EventPattern('dbus-return', method='HandleWith'), - EventPattern('dbus-signal', interface=cs.CDO, signal='Finished'), - EventPattern('dbus-signal', interface=cs.CD_IFACE_OP_LIST, - signal='DispatchOperationFinished'), + EventPattern('dbus-signal', signal='AccountPropertyChanged', + predicate=lambda e: + e.args[0].get('ConnectionError') == + cs.SOFTWARE_UPGRADE_REQUIRED), + EventPattern('dbus-method-call', method='Disconnect', + handled=True), ) - # Now there are no more active channel dispatch operations - assert cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations') == [] - if __name__ == '__main__': exec_test(test, {}) diff --git a/tests/twisted/dispatcher/connect-for-request.py b/tests/twisted/dispatcher/connect-for-request.py index e32b520..1163752 100644 --- a/tests/twisted/dispatcher/connect-for-request.py +++ b/tests/twisted/dispatcher/connect-for-request.py @@ -102,7 +102,7 @@ def test(q, bus, mc): handled=False) conn = SimulatedConnection(q, bus, 'fakecm', 'fakeprotocol', '_', - 'myself', has_requests=True, has_presence=True) + 'myself', has_presence=True) q.dbus_return(e.message, conn.bus_name, conn.object_path, signature='so') diff --git a/tests/twisted/dispatcher/dispatch-before-connected.py b/tests/twisted/dispatcher/dispatch-before-connected.py index a668c19..26cfa99 100644 --- a/tests/twisted/dispatcher/dispatch-before-connected.py +++ b/tests/twisted/dispatcher/dispatch-before-connected.py @@ -73,7 +73,7 @@ def test(q, bus, mc): handled=False) conn = SimulatedConnection(q, bus, 'fakecm', 'fakeprotocol', '_', - 'myself', has_requests=True, has_presence=True) + 'myself', has_presence=True) q.dbus_return(e.message, conn.bus_name, conn.object_path, signature='so') diff --git a/tests/twisted/dispatcher/dispatch-obsolete.py b/tests/twisted/dispatcher/dispatch-obsolete.py index 4431631..afe5df6 100644 --- a/tests/twisted/dispatcher/dispatch-obsolete.py +++ b/tests/twisted/dispatcher/dispatch-obsolete.py @@ -38,154 +38,14 @@ def test(q, bus, mc): conn = enable_fakecm_account(q, bus, mc, account, params, has_requests=False) - text_fixed_properties = dbus.Dictionary({ - cs.CHANNEL + '.TargetHandleType': cs.HT_CONTACT, - cs.CHANNEL + '.ChannelType': cs.CHANNEL_TYPE_TEXT, - }, signature='sv') - - # Two clients want to observe, approve and handle channels - empathy = SimulatedClient(q, bus, 'Empathy', - observe=[text_fixed_properties], approve=[text_fixed_properties], - handle=[text_fixed_properties], bypass_approval=False) - kopete = SimulatedClient(q, bus, 'Kopete', - observe=[text_fixed_properties], approve=[text_fixed_properties], - handle=[text_fixed_properties], bypass_approval=False) - - # wait for MC to download the properties - expect_client_setup(q, [empathy, kopete]) - - # subscribe to the OperationList interface (MC assumes that until this - # property has been retrieved once, nobody cares) - - cd = bus.get_object(cs.CD, cs.CD_PATH) - cd_props = dbus.Interface(cd, cs.PROPERTIES_IFACE) - assert cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations') == [] - - channel_properties = dbus.Dictionary(text_fixed_properties, - signature='sv') - channel_properties[cs.CHANNEL + '.TargetID'] = 'juliet' - channel_properties[cs.CHANNEL + '.TargetHandle'] = \ - conn.ensure_handle(cs.HT_CONTACT, 'juliet') - channel_properties[cs.CHANNEL + '.InitiatorID'] = 'juliet' - channel_properties[cs.CHANNEL + '.InitiatorHandle'] = \ - conn.ensure_handle(cs.HT_CONTACT, 'juliet') - channel_properties[cs.CHANNEL + '.Requested'] = False - channel_properties[cs.CHANNEL + '.Interfaces'] = dbus.Array(signature='s') - - chan = SimulatedChannel(conn, channel_properties) - chan.announce() - - # A channel dispatch operation is created - - e = q.expect('dbus-signal', - path=cs.CD_PATH, - interface=cs.CD_IFACE_OP_LIST, - signal='NewDispatchOperation') - - cdo_path = e.args[0] - cdo_properties = e.args[1] - - assert cdo_properties[cs.CDO + '.Account'] == account.object_path - assert cdo_properties[cs.CDO + '.Connection'] == conn.object_path - assert cs.CDO + '.Interfaces' in cdo_properties - - handlers = cdo_properties[cs.CDO + '.PossibleHandlers'][:] - handlers.sort() - assert handlers == [cs.tp_name_prefix + '.Client.Empathy', - cs.tp_name_prefix + '.Client.Kopete'], handlers - - assert cs.CD_IFACE_OP_LIST in cd_props.Get(cs.CD, 'Interfaces') - assert cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations') ==\ - [(cdo_path, cdo_properties)] - - cdo = bus.get_object(cs.CD, cdo_path) - cdo_iface = dbus.Interface(cdo, cs.CDO) - - # Both Observers are told about the new channel - - e, k = q.expect_many( - EventPattern('dbus-method-call', - path=empathy.object_path, - interface=cs.OBSERVER, method='ObserveChannels', - handled=False), - EventPattern('dbus-method-call', - path=kopete.object_path, - interface=cs.OBSERVER, method='ObserveChannels', - handled=False), - ) - assert e.args[0] == account.object_path, e.args - assert e.args[1] == conn.object_path, e.args - assert e.args[3] == cdo_path, e.args - assert e.args[4] == [], e.args # no requests satisfied - channels = e.args[2] - assert len(channels) == 1, channels - assert channels[0][0] == chan.object_path, channels - # the announced channel properties are some subset of what it actually has - for key in channels[0][1]: - assert channel_properties[key] == channels[0][1][key], key - assert cs.CHANNEL + '.TargetHandleType' in channels[0][1] - assert cs.CHANNEL + '.ChannelType' in channels[0][1] - assert cs.CHANNEL + '.TargetHandle' in channels[0][1] - - assert k.args == e.args - - # Both Observers indicate that they are ready to proceed - q.dbus_return(k.message, signature='') - q.dbus_return(e.message, signature='') - - # The Approvers are next - - e, k = q.expect_many( - EventPattern('dbus-method-call', - path=empathy.object_path, - interface=cs.APPROVER, method='AddDispatchOperation', - handled=False), - EventPattern('dbus-method-call', - path=kopete.object_path, - interface=cs.APPROVER, method='AddDispatchOperation', - handled=False), - ) - - assert e.args[1:] == [cdo_path, cdo_properties] - channels = e.args[0] - assert len(channels) == 1, channels - assert channels[0][0] == chan.object_path, channels - # the announced channel properties are some subset of what it actually has - for key in channels[0][1]: - assert channel_properties[key] == channels[0][1][key], key - assert cs.CHANNEL + '.TargetHandleType' in channels[0][1] - assert cs.CHANNEL + '.ChannelType' in channels[0][1] - assert cs.CHANNEL + '.TargetHandle' in channels[0][1] - assert k.args == e.args - - q.dbus_return(e.message, signature='') - q.dbus_return(k.message, signature='') - - # Both Approvers now have a flashing icon or something, trying to get the - # user's attention - - # The user responds to Empathy first - call_async(q, cdo_iface, 'HandleWith', - cs.tp_name_prefix + '.Client.Empathy') - - # Empathy is asked to handle the channels - e = q.expect('dbus-method-call', - path=empathy.object_path, - interface=cs.HANDLER, method='HandleChannels', - handled=False) - - # Empathy accepts the channels - q.dbus_return(e.message, signature='') - q.expect_many( - EventPattern('dbus-return', method='HandleWith'), - EventPattern('dbus-signal', interface=cs.CDO, signal='Finished'), - EventPattern('dbus-signal', interface=cs.CD_IFACE_OP_LIST, - signal='DispatchOperationFinished'), + EventPattern('dbus-signal', signal='AccountPropertyChanged', + predicate=lambda e: + e.args[0].get('ConnectionError') == + cs.SOFTWARE_UPGRADE_REQUIRED), + EventPattern('dbus-method-call', method='Disconnect', + handled=True), ) - # Now there are no more active channel dispatch operations - assert cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations') == [] - if __name__ == '__main__': exec_test(test, {}) diff --git a/tests/twisted/mctest.py b/tests/twisted/mctest.py index de75aba..0d1bd03 100644 --- a/tests/twisted/mctest.py +++ b/tests/twisted/mctest.py @@ -1047,17 +1047,8 @@ def expect_fakecm_connection(q, bus, mc, account, expected_params, expect_after_connect = list(expect_after_connect) - if not has_requests: - expect_after_connect.append( - servicetest.EventPattern('dbus-method-call', - interface=cs.CONN, method='ListChannels', args=[], - path=conn.object_path, handled=True)) - events = events + list(q.expect_many(*expect_after_connect)) - if not has_requests: - del events[-1] - if events: return (conn,) + tuple(events) -- 1.8.4.rc3