roster: test handling pushes without id='' attribute.

I previously believed that I had added code to WockyPorter such that, if
someone sent us an IQ with no id='' attribute, it would be silently
dropped rather than being handed up to the application. (This would
technically be valid behaviour; RFC3920 §9.2.3 “IQ Semantics” says that
“The 'id' attribute is REQUIRED for IQ stanzas.”
<http://xmpp.org/rfcs/rfc3920.html#stanzas-semantics-iq>)

I mentioned this to Rob, who was horrified, because Gabble's roster code
specifically handles roster pushes lacking an id='' attribute because
this happens in the wild. I checked, and I had apparently imagined that
I had made the above change to WockyPorter.

This patch adds a regression test to check that Gabble correctly
processes roster pushes without an id='' attribute, and doesn't crash
in the process.

3 files changed, 54 insertions, 1 deletions

diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am
index b5fbda631..9b53332ad 100644
--- a/tests/twisted/Makefile.am
+++ b/tests/twisted/Makefile.am
@@ -57,6 +57,7 @@ TWISTED_TESTS = \
 	roster/ensure.py \
 	roster/groups.py \
 	roster/groups-12791.py \
+	roster/push-without-id.py \
 	roster/removed-from-rp-subscribe.py \
 	roster/request-group-before-roster.py \
 	roster/request-group-after-roster.py \
diff --git a/tests/twisted/roster/push-without-id.py b/tests/twisted/roster/push-without-id.py
new file mode 100644
index 000000000..0ba4f53b9
--- /dev/null
+++ b/tests/twisted/roster/push-without-id.py
@@ -0,0 +1,47 @@
+"""
+Ensure that Gabble correctly handles broken roster pushes from servers that
+omit id='', in flagrant violation of XMPP Core. (Sadly these servers exist, so
+we should interop with them. Think of it of like No_Reply in D-Bus...
+"""
+
+from servicetest import EventPattern, sync_dbus
+from gabbletest import exec_test, acknowledge_iq, sync_stream
+from rostertest import (
+    make_roster_push, expect_contact_list_signals, check_contact_list_signals,
+)
+import ns
+import constants as cs
+
+jid = 'moonboots@xsf.lit'
+
+def test(q, bus, conn, stream):
+    # Gabble asks for the roster; the server sends back an empty roster.
+    event = q.expect('stream-iq', query_ns=ns.ROSTER)
+    acknowledge_iq(stream, event.stanza)
+
+    pairs = expect_contact_list_signals(q, bus, conn,
+            ['stored'])
+    stored = check_contact_list_signals(q, bus, conn, pairs.pop(0),
+            cs.HT_LIST, 'stored', [])
+
+    # The server sends us a roster push without an id=''. WTF!
+    iq = make_roster_push(stream, jid, 'both')
+    del iq['id']
+    stream.send(iq)
+
+    h = conn.RequestHandles(cs.HT_CONTACT, [jid])[0]
+    q.expect_many(
+        EventPattern('dbus-signal', signal='MembersChanged',
+            args=['', [h], [], [], [], 0, 0], path=stored.object_path),
+        EventPattern('dbus-signal', signal='ContactsChanged',
+            args=[{ h: (cs.SUBSCRIPTION_STATE_YES,
+                    cs.SUBSCRIPTION_STATE_YES, ''), },
+                []],
+            ),
+        )
+
+    # Verify that Gabble didn't crash while trying to ack the push.
+    sync_stream(q, stream)
+
+if __name__ == '__main__':
+    exec_test(test)
diff --git a/tests/twisted/rostertest.py b/tests/twisted/rostertest.py
index 89eba68cd..335da18d2 100644
--- a/tests/twisted/rostertest.py
+++ b/tests/twisted/rostertest.py
@@ -7,7 +7,7 @@ from servicetest import (assertEquals, assertLength, EventPattern,
 import constants as cs
 import ns
 
-def send_roster_push(stream, jid, subscription, ask_subscribe=False):
+def make_roster_push(stream, jid, subscription, ask_subscribe=False):
     iq = IQ(stream, "set")
     iq['id'] = 'push'
     query = iq.addElement('query')
@@ -19,6 +19,11 @@ def send_roster_push(stream, jid, subscription, ask_subscribe=False):
     if ask_subscribe:
         item['ask'] = 'subscribe'
 
+    return iq
+
+def send_roster_push(stream, jid, subscription, ask_subscribe=False):
+    iq = make_roster_push(stream, jid, subscription,
+        ask_subscribe=ask_subscribe)
     stream.send(iq)
 
 def get_contact_list_event_patterns(q, bus, conn, expected_handle_type, name):