telepathy-gabble: bytestream-socks5: send empty offer if we can't initiate.

[Will Thompson]

Module: telepathy-gabble
Branch: master
Commit: 3fe57e731b2518e22ba21efb4d16f50564363a2f
URL:    http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=3fe57e731b2518e22ba21efb4d16f50564363a2f

Author: Will Thompson <will.thompson at collabora.co.uk>
Date:   Thu Mar 29 15:27:11 2012 +0100

bytestream-socks5: send empty offer if we can't initiate.

This looks pretty ridiculous, so needs some explanation. If you have no
network connection (suppose you're on a train, testing tubes using
Prosody on localhost), then get_local_interfaces_ips() returns NULL
(the loopback interface is explicitly skipped). So previously, after the
recipient has accepted the tube, gabble_bytestream_socks5_initiate()
would fail for the initiator. bytestream-multiple doesn't fall back if
it fails immediately; so the whole bytestream-establishment fails. The
initiator's tube channel would close, but no stanza gets sent to the
peer to tell it to stop waiting for the bytestream offer, so it's just
hanging there waiting forever.

Just making bytestream-multiple fall back immediately doesn't help,
because the peer is still expecting a SOCKS5 offer, rather than a
in-band bytestream offer, so rejects it. Unlike Jingle, SI doesn't have
a way to cancel an offer once it's been accepted but before the
transport has been negotiated. So… if we send the peer an empty offer,
they'll send back an error, and both sides know to fall back (if they
understand bytestream-multiple) or give up (if not).

There is no test case for this, because it's difficult to make
get_local_interfaces_ips() or gibber_listener_listen_tcp() fail from the
test suite. I tested it with a pair of simple D-Bus tube clients and no
network connection, and/or hacking get_local_interfaces_ips() to always
return NULL.

Although this trick is schema-compliant
<http://xmpp.org/extensions/xep-0065.html#schema>, it's not compliant
with the wording of XEP-0065. Quoth §5.3.1
<http://xmpp.org/extensions/xep-0065.html#direct-proto-initiate>:

> The <query/> element MUST contain one or more <streamhost/> elements,
> each of which MUST possess the 'host', 'jid', and 'port' attributes.

But it also says:

> If the request is malformed …, the Target MUST return an error of
> <bad-request/>.

So this should work with any (robust) implementation.

The option would be to (try to) start listening before sending the
SI offer—if it fails, don't offer to use SOCKS5 bytestreams.

https://bugs.freedesktop.org/show_bug.cgi?id=48050

---

 src/bytestream-socks5.c |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/bytestream-socks5.c b/src/bytestream-socks5.c
index 0d34a8d..48686d3 100644
--- a/src/bytestream-socks5.c
+++ b/src/bytestream-socks5.c
@@ -1878,15 +1878,14 @@ new_connection_cb (GibberListener *listener,
 static void
 send_streamhosts (
     GabbleBytestreamSocks5 *self,
-    GSList *ips)
+    GSList *ips,
+    gint port_num)
 {
   GabbleBytestreamSocks5Private *priv = self->priv;
   gchar *port;
-  gint port_num;
   WockyStanza *msg;
   WockyNode *query_node;
 
-  port_num = gibber_listener_get_port (priv->listener);
   port = g_strdup_printf ("%d", port_num);
 
   msg = wocky_stanza_build (WOCKY_STANZA_TYPE_IQ, WOCKY_STANZA_SUB_TYPE_SET,
@@ -1898,7 +1897,7 @@ send_streamhosts (
         '*', &query_node,
       ')', NULL);
 
-  for (; ips != NULL; ips = g_slist_delete_link (ips, ips))
+  for (; port_num != 0 && ips != NULL; ips = g_slist_delete_link (ips, ips))
     {
       WockyNode *node = wocky_node_add_child (query_node, "streamhost");
 
@@ -1963,6 +1962,7 @@ gabble_bytestream_socks5_initiate (GabbleBytestreamIface *iface)
   GabbleBytestreamSocks5Private *priv =
       GABBLE_BYTESTREAM_SOCKS5_GET_PRIVATE (self);
   GSList *ips;
+  guint port_num = 0;
 
   if (priv->bytestream_state != GABBLE_BYTESTREAM_STATE_INITIATING)
     {
@@ -1974,23 +1974,28 @@ gabble_bytestream_socks5_initiate (GabbleBytestreamIface *iface)
   ips = get_local_interfaces_ips ();
   if (ips == NULL)
     {
-      DEBUG ("Can't get IP addresses");
-      return FALSE;
+      DEBUG ("Can't get IP addresses; will send empty offer.");
     }
+  else
+    {
+      g_assert (priv->listener == NULL);
+      priv->listener = gibber_listener_new ();
 
-  g_assert (priv->listener == NULL);
-  priv->listener = gibber_listener_new ();
+      g_signal_connect (priv->listener, "new-connection",
+          G_CALLBACK (new_connection_cb), self);
 
-  g_signal_connect (priv->listener, "new-connection",
-      G_CALLBACK (new_connection_cb), self);
+      if (!gibber_listener_listen_tcp (priv->listener, 0, NULL))
+        {
+          DEBUG ("can't listen for incoming connection; will send empty offer.");
+          g_slist_foreach (ips, (GFunc) g_free, NULL);
+          g_slist_free (ips);
+          ips = NULL;
+        }
 
-  if (!gibber_listener_listen_tcp (priv->listener, 0, NULL))
-    {
-      DEBUG ("can't listen for incoming connection");
-      return FALSE;
+      port_num = gibber_listener_get_port (priv->listener);
     }
 
-  send_streamhosts (self, ips);
+  send_streamhosts (self, ips, port_num);
   return TRUE;
 }