diff options
author | Will Thompson <will.thompson@collabora.co.uk> | 2013-03-01 15:09:04 +0000 |
---|---|---|
committer | Will Thompson <will.thompson@collabora.co.uk> | 2013-03-01 15:09:04 +0000 |
commit | 73c80443fd8594212f1a0f9fb8a99a0a4d691dbc (patch) | |
tree | 7f6111c6735bf414a11fb926b24061fde5eb29e8 | |
parent | 0bffd2d8302d81a804f5186f17445abae16b1d85 (diff) | |
parent | fa8be7c05185ce92696a5771b556e70b8eb99a00 (diff) |
Merge branch '43166-muc-rate-limit'
m--------- | lib/ext/wocky | 0 | ||||
-rw-r--r-- | src/message-util.c | 1 | ||||
-rw-r--r-- | src/muc-channel.c | 91 | ||||
-rw-r--r-- | tests/twisted/constants.py | 14 | ||||
-rw-r--r-- | tests/twisted/muc/chat-states.py | 73 | ||||
-rw-r--r-- | tests/twisted/muc/send-error.py | 122 |
6 files changed, 219 insertions, 82 deletions
diff --git a/lib/ext/wocky b/lib/ext/wocky -Subproject a38dbe13ebf4eef59fbd1a868464987e204ea60 +Subproject 0fc8f511683054634d7ba5d447e38a634400eeb diff --git a/src/message-util.c b/src/message-util.c index 49f726c3d..01bb71735 100644 --- a/src/message-util.c +++ b/src/message-util.c @@ -241,6 +241,7 @@ gabble_tp_send_error_from_wocky_xmpp_error (WockyXmppError err) case WOCKY_XMPP_ERROR_FORBIDDEN: case WOCKY_XMPP_ERROR_NOT_AUTHORIZED: + case WOCKY_XMPP_ERROR_POLICY_VIOLATION: return TP_CHANNEL_TEXT_SEND_ERROR_PERMISSION_DENIED; case WOCKY_XMPP_ERROR_RESOURCE_CONSTRAINT: diff --git a/src/muc-channel.c b/src/muc-channel.c index 74cde99a1..15c337b07 100644 --- a/src/muc-channel.c +++ b/src/muc-channel.c @@ -214,6 +214,8 @@ struct _GabbleMucChannelPrivate GPtrArray *initial_channels; GArray *initial_handles; char **initial_ids; + + gboolean have_received_error_type_wait; }; typedef struct { @@ -272,8 +274,8 @@ static void handle_renamed (GObject *source, static void handle_error (GObject *source, WockyStanza *stanza, - WockyXmppError errnum, - const gchar *message, + WockyXmppErrorType errtype, + const GError *error, gpointer data); static void handle_join (WockyMuc *muc, @@ -330,8 +332,8 @@ static void handle_errmsg (GObject *source, GDateTime *datetime, WockyMucMember *who, const gchar *text, - WockyXmppError error, WockyXmppErrorType etype, + const GError *error, gpointer data); /* Signatures for some other stuff. */ @@ -339,11 +341,12 @@ static void handle_errmsg (GObject *source, static void _gabble_muc_channel_handle_subject (GabbleMucChannel *chan, TpHandleType handle_type, TpHandle sender, GDateTime *datetime, const gchar *subject, - WockyStanza *msg); + WockyStanza *msg, const GError *error); static void _gabble_muc_channel_receive (GabbleMucChannel *chan, TpChannelTextMessageType msg_type, TpHandleType handle_type, TpHandle sender, GDateTime *datetime, const gchar *id, const gchar *text, - WockyStanza *msg, TpChannelTextSendError send_error, + WockyStanza *msg, + const GError *send_error, TpDeliveryStatus delivery_status); static void @@ -1882,8 +1885,8 @@ update_permissions (GabbleMucChannel *chan) static void handle_error (GObject *source, WockyStanza *stanza, - WockyXmppError errnum, - const gchar *message, + WockyXmppErrorType errtype, + const GError *error, gpointer data) { GabbleMucChannel *gmuc = GABBLE_MUC_CHANNEL (data); @@ -1898,7 +1901,7 @@ handle_error (GObject *source, return; } - if (errnum == WOCKY_XMPP_ERROR_NOT_AUTHORIZED) + if (error->code == WOCKY_XMPP_ERROR_NOT_AUTHORIZED) { /* channel can sit requiring a password indefinitely */ clear_join_timer (gmuc); @@ -1918,7 +1921,7 @@ handle_error (GObject *source, { GError *tp_error = NULL; - switch (errnum) + switch (error->code) { case WOCKY_XMPP_ERROR_FORBIDDEN: tp_error = g_error_new (TP_ERROR, TP_ERROR_CHANNEL_BANNED, @@ -1943,7 +1946,7 @@ handle_error (GObject *source, default: tp_error = g_error_new (TP_ERROR, TP_ERROR_NOT_AVAILABLE, - "%s", wocky_xmpp_error_description (errnum)); + "%s", wocky_xmpp_error_description (error->code)); break; } @@ -2808,7 +2811,8 @@ handle_message (GObject *source, if (text != NULL) _gabble_muc_channel_receive (gmuc, msg_type, handle_type, from, datetime, xmpp_id, text, stanza, - GABBLE_TEXT_CHANNEL_SEND_NO_ERROR, TP_DELIVERY_STATUS_DELIVERED); + NULL, + TP_DELIVERY_STATUS_DELIVERED); if (from_member && state != WOCKY_MUC_MSG_STATE_NONE) { @@ -2836,7 +2840,7 @@ handle_message (GObject *source, if (subject != NULL) _gabble_muc_channel_handle_subject (gmuc, handle_type, from, - datetime, subject, stanza); + datetime, subject, stanza, NULL); } static void @@ -2847,8 +2851,8 @@ handle_errmsg (GObject *source, GDateTime *datetime, WockyMucMember *who, const gchar *text, - WockyXmppError error, WockyXmppErrorType etype, + const GError *error, gpointer data) { GabbleMucChannel *gmuc = GABBLE_MUC_CHANNEL (data); @@ -2856,7 +2860,6 @@ handle_errmsg (GObject *source, TpBaseChannel *base = TP_BASE_CHANNEL (gmuc); TpBaseConnection *conn = tp_base_channel_get_connection (base); gboolean from_member = (who != NULL); - TpChannelTextSendError tp_err = TP_CHANNEL_TEXT_SEND_ERROR_UNKNOWN; TpDeliveryStatus ds = TP_DELIVERY_STATUS_DELIVERED; TpHandleRepoIface *repo = NULL; TpHandleType handle_type; @@ -2883,16 +2886,30 @@ handle_errmsg (GObject *source, from = tp_base_channel_get_target_handle (base); } - tp_err = gabble_tp_send_error_from_wocky_xmpp_error (error); - if (etype == WOCKY_XMPP_ERROR_TYPE_WAIT) - ds = TP_DELIVERY_STATUS_TEMPORARILY_FAILED; + { + ds = TP_DELIVERY_STATUS_TEMPORARILY_FAILED; + /* Some MUCs have very strict rate limiting like "at most one stanza per + * second". Since chat state notifications count towards this, if the + * user types a message very quickly then the typing notification is + * accepted but then the stanza containing the actual message is + * rejected. + * + * So: if we ever get rate-limited, let's just stop sending chat states. + * + * https://bugs.freedesktop.org/show_bug.cgi?id=43166 + */ + DEBUG ("got <error type='wait'>, disabling chat state notifications"); + priv->have_received_error_type_wait = TRUE; + } else - ds = TP_DELIVERY_STATUS_PERMANENTLY_FAILED; + { + ds = TP_DELIVERY_STATUS_PERMANENTLY_FAILED; + } if (text != NULL) _gabble_muc_channel_receive (gmuc, TP_CHANNEL_TEXT_MESSAGE_TYPE_NOTICE, - handle_type, from, datetime, xmpp_id, text, stanza, tp_err, ds); + handle_type, from, datetime, xmpp_id, text, stanza, error, ds); /* FIXME: this is stupid. WockyMuc gives us the subject for non-errors, but * doesn't bother for errors. @@ -2908,7 +2925,7 @@ handle_errmsg (GObject *source, (priv->set_subject_stanza_id != NULL && !tp_strdiff (xmpp_id, priv->set_subject_stanza_id))) _gabble_muc_channel_handle_subject (gmuc, - handle_type, from, datetime, subject, stanza); + handle_type, from, datetime, subject, stanza, error); } /* ************************************************************************* */ @@ -2921,11 +2938,11 @@ _gabble_muc_channel_handle_subject (GabbleMucChannel *chan, TpHandle sender, GDateTime *datetime, const gchar *subject, - WockyStanza *msg) + WockyStanza *msg, + const GError *error) { GabbleMucChannelPrivate *priv; const gchar *actor; - GError *error = NULL; gint64 timestamp = datetime != NULL ? g_date_time_to_unix (datetime) : G_MAXINT64; @@ -2933,7 +2950,7 @@ _gabble_muc_channel_handle_subject (GabbleMucChannel *chan, priv = chan->priv; - if (wocky_stanza_extract_errors (msg, NULL, &error, NULL, NULL)) + if (error != NULL) { if (priv->set_subject_context != NULL) { @@ -2950,7 +2967,6 @@ _gabble_muc_channel_handle_subject (GabbleMucChannel *chan, room_properties_update (chan); } - g_clear_error (&error); return; } @@ -2988,7 +3004,7 @@ _gabble_muc_channel_handle_subject (GabbleMucChannel *chan, /** * _gabble_muc_channel_receive: receive MUC messages */ -void +static void _gabble_muc_channel_receive (GabbleMucChannel *chan, TpChannelTextMessageType msg_type, TpHandleType sender_handle_type, @@ -2997,7 +3013,7 @@ _gabble_muc_channel_receive (GabbleMucChannel *chan, const gchar *id, const gchar *text, WockyStanza *msg, - TpChannelTextSendError send_error, + const GError *send_error, TpDeliveryStatus error_status) { TpBaseChannel *base; @@ -3016,7 +3032,7 @@ _gabble_muc_channel_receive (GabbleMucChannel *chan, muc_self_handle = chan->group.self_handle; /* Is this an error report? */ - is_error = (send_error != GABBLE_TEXT_CHANNEL_SEND_NO_ERROR); + is_error = (send_error != NULL); if (is_error && sender == muc_self_handle) { @@ -3092,9 +3108,21 @@ _gabble_muc_channel_receive (GabbleMucChannel *chan, if (id != NULL) tp_message_set_string (delivery_report, 0, "delivery-token", id); - if (is_error) - tp_message_set_uint32 (delivery_report, 0, "delivery-error", - send_error); + if (send_error != NULL) + { + tp_message_set_uint32 (delivery_report, 0, "delivery-error", + gabble_tp_send_error_from_wocky_xmpp_error (send_error->code)); + + if (!tp_str_empty (send_error->message)) + { + guint body_part_number = tp_message_append_part (delivery_report); + + tp_message_set_string (delivery_report, body_part_number, + "content-type", "text/plain"); + tp_message_set_string (delivery_report, body_part_number, + "content", send_error->message); + } + } /* We do not set a message-sender on the report: the intended recipient * of the original message was the MUC, so the spec says we should omit @@ -3821,6 +3849,9 @@ gabble_muc_channel_send_chat_state (GObject *object, GabbleMucChannelPrivate *priv = self->priv; TpBaseChannel *base = TP_BASE_CHANNEL (self); + if (priv->have_received_error_type_wait) + return TRUE; + return gabble_message_util_send_chat_state (G_OBJECT (self), GABBLE_CONNECTION (tp_base_channel_get_connection (base)), WOCKY_STANZA_SUB_TYPE_GROUPCHAT, state, priv->jid, error); diff --git a/tests/twisted/constants.py b/tests/twisted/constants.py index 0ea9d5b7f..d541c75fe 100644 --- a/tests/twisted/constants.py +++ b/tests/twisted/constants.py @@ -463,6 +463,20 @@ MT_NOTICE = 2 MT_AUTO_REPLY = 3 MT_DELIVERY_REPORT = 4 +class MessageFlag(object): + TRUNCATED = 1 + NON_TEXT_CONTENT = 2 + SCROLLBACK = 4 + RESCUED = 8 + +class SendError(object): + UNKNOWN = 0 + OFFLINE = 1 + INVALID_CONTACT = 2 + PERMISSION_DENIED = 3 + TOO_LONG = 4 + NOT_IMPLEMENTED = 5 + PROTOCOL = 'org.freedesktop.Telepathy.Protocol' PROTOCOL_IFACE_PRESENCES = PROTOCOL + '.Interface.Presence' PROTOCOL_IFACE_ADDRESSING = PROTOCOL + '.Interface.Addressing' diff --git a/tests/twisted/muc/chat-states.py b/tests/twisted/muc/chat-states.py index eaa5c65e2..4be5f50b2 100644 --- a/tests/twisted/muc/chat-states.py +++ b/tests/twisted/muc/chat-states.py @@ -3,8 +3,8 @@ Regression test for <https://bugs.freedesktop.org/show_bug.cgi?id=32952>, wherein chat states in MUCs were misparsed, and MUC chat states in general. """ -from servicetest import assertEquals -from gabbletest import exec_test, elem +from servicetest import assertEquals, assertLength, EventPattern +from gabbletest import exec_test, elem, make_muc_presence, sync_stream from mucutil import join_muc_and_check import ns import constants as cs @@ -12,16 +12,23 @@ import constants as cs MUC = 'ohai@groupchat.google.com' BOB = MUC + '/bob' +def get_state_notification(stanza): + for x in stanza.elements(): + if x.uri == ns.CHAT_STATES: + return x + + return None + def check_state_notification(elem, name, allow_body=False): assertEquals('message', elem.name) assertEquals('groupchat', elem['type']) - children = list(elem.elements()) - notification = [x for x in children if x.uri == ns.CHAT_STATES][0] + notification = get_state_notification(elem) + assert notification is not None, elem.toXml() assert notification.name == name, notification.toXml() if not allow_body: - assert len(children) == 1, elem.toXml() + assert len(elem.children) == 1, elem.toXml() def test(q, bus, conn, stream): (muc_handle, chan, user, bob) = join_muc_and_check(q, bus, conn, stream, @@ -71,21 +78,21 @@ def test(q, bus, conn, stream): assertEquals(cs.CHAT_STATE_PAUSED, states.get(bob, cs.CHAT_STATE_INACTIVE)) -# # Bob leaves -# stream.send( -# elem('presence', from_=BOB, to='test@localhost/Resource', -# type='unavailable')) + # Bob leaves + presence = make_muc_presence('owner', 'none', MUC, 'bob') + presence['type'] = 'unavailable' + stream.send(presence) -# e = q.expect('dbus-signal', signal='ChatStateChanged') -# contact, state = e.args -# assertEquals(bob, contact) -# assertEquals(cs.CHAT_STATE_GONE, state) + e = q.expect('dbus-signal', signal='ChatStateChanged') + contact, state = e.args + assertEquals(bob, contact) + assertEquals(cs.CHAT_STATE_GONE, state) -# states = chan.Properties.Get(cs.CHANNEL_IFACE_CHAT_STATE, 'ChatStates') -# assertEquals(cs.CHAT_STATE_INACTIVE, -# states.get(user, cs.CHAT_STATE_INACTIVE)) -# # Bob no longer has any chat state at all -# assertEquals(None, states.get(bob, None)) + states = chan.Properties.Get(cs.CHANNEL_IFACE_CHAT_STATE, 'ChatStates') + assertEquals(cs.CHAT_STATE_INACTIVE, + states.get(user, cs.CHAT_STATE_INACTIVE)) + # Bob no longer has any chat state at all + assertEquals(None, states.get(bob, None)) # Sending chat states: @@ -111,13 +118,31 @@ def test(q, bus, conn, stream): assertEquals(cs.CHAT_STATE_ACTIVE, states.get(user, cs.CHAT_STATE_INACTIVE)) - def is_body(e): - if e.name == 'body': - assert e.children[0] == u'hi.', e.toXml() - return True - return False + bodies = list(stanza.elements(uri=ns.CLIENT, name='body')) + assertLength(1, bodies) + assertEquals(u'hi.', bodies[0].children[0]) + + # If we get an error with type='wait', stop sending chat states. + stanza['type'] = 'error' + stanza['from'] = MUC + stanza['to'] = 'test@localhost/Resource' + + error = stanza.addElement('error') + error['type'] = 'wait' + error.addElement((ns.STANZA, 'resource-constraint')) + stream.send(stanza) - assert len([x for x in stanza.elements() if is_body(x)]) == 1, stanza.toXml() + q.expect('dbus-signal', signal='MessageReceived', + predicate=lambda e: e.args[0][0]['message-type'] == cs.MT_DELIVERY_REPORT) + + q.forbid_events([ + EventPattern('stream-message', to=MUC, + predicate=lambda e: get_state_notification(e.stanza) is not None) + ]) + + # User starts typing again but nothing should be seen or heard on the stream. + chan.ChatState.SetChatState(cs.CHAT_STATE_COMPOSING) + sync_stream(q, stream) if __name__ == '__main__': exec_test(test) diff --git a/tests/twisted/muc/send-error.py b/tests/twisted/muc/send-error.py index 545d64285..33f376bf5 100644 --- a/tests/twisted/muc/send-error.py +++ b/tests/twisted/muc/send-error.py @@ -2,10 +2,11 @@ Test incoming error messages in MUC channels. """ +import warnings import dbus from gabbletest import exec_test -from servicetest import EventPattern +from servicetest import EventPattern, assertEquals, assertLength, assertContains import constants as cs import ns @@ -18,7 +19,47 @@ def test(q, bus, conn, stream): # Suppose we don't have permission to speak in this MUC. Send a message to # the channel, and have the MUC reject it as unauthorized. - content = u"hi r ther ne warez n this chanel?" + send_message_and_expect_error(q, stream, + text_chan, test_handle, bob_handle, + u"hi r ther ne warez n this chanel?", + '401', 'auth', 'not-authorized', + delivery_status=cs.DELIVERY_STATUS_PERMANENTLY_FAILED, + send_error_value=cs.SendError.PERMISSION_DENIED) + + # This time, we get rate-limited. + # <https://bugs.freedesktop.org/show_bug.cgi?id=43166> + send_message_and_expect_error(q, stream, + text_chan, test_handle, bob_handle, + "faster faster", + '500', 'wait', 'resource-constraint', + delivery_status=cs.DELIVERY_STATUS_TEMPORARILY_FAILED, + # Yuck this isn't a very good name is it? + send_error_value=cs.SendError.TOO_LONG) + + # How about an error message in the reply? This is from Prosody. See + # https://bugs.freedesktop.org/show_bug.cgi?id=43166#c9 + send_message_and_expect_error(q, stream, + text_chan, test_handle, bob_handle, + content=u"fair enough", + code=None, + type_='wait', + element='policy-violation', + error_message='The room is currently overactive, please try again later', + delivery_status=cs.DELIVERY_STATUS_TEMPORARILY_FAILED, + # Maybe we should expand the SendError codes some day, because this one + # is l-a-m-e. + send_error_value=cs.SendError.PERMISSION_DENIED) + + +def send_message_and_expect_error(q, stream, + text_chan, test_handle, bob_handle, + content, + code=None, + type_=None, + element=None, + error_message=None, + delivery_status=None, + send_error_value=None): greeting = [ dbus.Dictionary({ }, signature='sv'), { 'content-type': 'text/plain', @@ -26,8 +67,7 @@ def test(q, bus, conn, stream): } ] - sent_token = dbus.Interface(text_chan, cs.CHANNEL_IFACE_MESSAGES) \ - .SendMessage(greeting, dbus.UInt32(0)) + sent_token = text_chan.Messages.SendMessage(greeting, dbus.UInt32(0)) stream_message, _, _ = q.expect_many( EventPattern('stream-message'), @@ -41,9 +81,14 @@ def test(q, bus, conn, stream): elem['to'] = 'chat@conf.localhost/test' elem['type'] = 'error' error = elem.addElement('error') - error['code'] = '401' - error['type'] = 'auth' - error.addElement((ns.STANZA, 'not-authorized')) + if code is not None: + error['code'] = code + if type_ is not None: + error['type'] = type_ + if element is not None: + error.addElement((ns.STANZA, element)) + if error_message is not None: + error.addElement((ns.STANZA, 'text')).addContent(error_message) stream.send(elem) @@ -54,48 +99,69 @@ def test(q, bus, conn, stream): EventPattern('dbus-signal', signal='MessageReceived'), ) - PERMISSION_DENIED = 3 - err, timestamp, type, text = send_error.args - assert err == PERMISSION_DENIED, send_error.args + assertEquals(send_error_value, err) # there's no way to tell when the original message was sent from the error stanza - assert timestamp == 0, send_error.args + assertEquals(0, timestamp) # Gabble can't determine the type of the original message; see muc/test-muc.py # assert type == 0, send_error.args - assert text == content, send_error.args + assertEquals(content, text) # The Text.Received signal should be a "you're not tall enough" stub id, timestamp, sender, type, flags, text = received.args - assert sender == 0, received.args - assert type == 4, received.args # Message_Type_Delivery_Report - assert flags == 2, received.args # Non_Text_Content - assert text == '', received.args + + assertEquals(0, sender) + assertEquals(type, cs.MT_DELIVERY_REPORT) + + if flags == 0: + warnings.warn("ignoring tp-glib bug #61254") + else: + assertEquals(cs.MessageFlag.NON_TEXT_CONTENT, flags) + + if error_message is None: + assertEquals('', text) + else: + assertEquals(error_message, text) # Check that the Messages.MessageReceived signal was a failed delivery report - assert len(message_received.args) == 1, message_received.args + assertLength(1, message_received.args) parts = message_received.args[0] - # The delivery report should just be a header, no body. - assert len(parts) == 1, parts + + if error_message is None: + # The delivery report should just be a header, no body. + assertLength(1, parts) + else: + assertLength(2, parts) + part = parts[0] # The intended recipient was the MUC, so there's no contact handle # suitable for being 'message-sender'. - assert 'message-sender' not in part or part['message-sender'] == 0, part - assert part['message-type'] == 4, part # Message_Type_Delivery_Report - assert part['delivery-status'] == 3, part # Delivery_Status_Permanently_Failed - assert part['delivery-error'] == PERMISSION_DENIED, part - assert part['delivery-token'] == sent_token, part + assertEquals(0, part.get('message-sender', 0)) + assertEquals(cs.MT_DELIVERY_REPORT, part['message-type']) + assertEquals(delivery_status, part['delivery-status']) + assertEquals(send_error_value, part['delivery-error']) + assertEquals(sent_token, part['delivery-token']) # Check that the included echo is from us, and matches all the keys in the # message we sent. - assert 'delivery-echo' in part, part + assertContains('delivery-echo', part) echo = part['delivery-echo'] - assert len(echo) == len(greeting), (echo, greeting) - assert echo[0]['message-sender'] == test_handle, echo[0] - assert echo[0]['message-token'] == sent_token, echo[0] + assertLength(len(greeting), echo) + echo_header = echo[0] + assertEquals(test_handle, echo_header['message-sender']) + assertEquals(sent_token, echo_header['message-token']) + for i in range(0, len(echo)): for key in greeting[i]: assert key in echo[i], (i, key, echo) assert echo[i][key] == greeting[i][key], (i, key, echo, greeting) + if error_message is not None: + body = parts[1] + + assertEquals('text/plain', body['content-type']) + assertEquals(error_message, body['content']) + + if __name__ == '__main__': exec_test(test) |