Search: handle Server='' as if it were omitted entirely

Previously, if you included the Server property in a channel request,
but set it to the empty string, Gabble would try to send the search to
the empty JID.

Instead, it ought to either reject the request outright, or pretend that
the property was omitted. I chose the latter: it's kinder to application
authors.

Fixes: <https://bugs.freedesktop.org/show_bug.cgi?id=32390>

2 files changed, 18 insertions, 8 deletions

diff --git a/src/search-manager.c b/src/search-manager.c
index a872aac86..8e5424439 100644
--- a/src/search-manager.c
+++ b/src/search-manager.c
@@ -480,6 +480,10 @@ gabble_search_manager_create_channel (TpChannelManager *manager,
   server = tp_asv_get_string (request_properties,
       TP_IFACE_CHANNEL_TYPE_CONTACT_SEARCH ".Server");
 
+  /* Treat an empty server as equivalent to omitting the server entirely. */
+  if (tp_str_empty (server))
+    server = NULL;
+
   if (server == NULL)
     {
       if (self->priv->default_jud == NULL)
diff --git a/tests/twisted/search/no-server-property.py b/tests/twisted/search/no-server-property.py
index 72018603f..c7df517db 100644
--- a/tests/twisted/search/no-server-property.py
+++ b/tests/twisted/search/no-server-property.py
@@ -6,6 +6,7 @@ passing the Server property
 import dbus
 
 from twisted.words.xish import xpath
+from functools import partial
 
 from gabbletest import exec_test, sync_stream, make_result_iq, acknowledge_iq, elem_iq, elem, disconnect_conn
 from servicetest import EventPattern, assertEquals
@@ -16,7 +17,7 @@ import ns
 
 JUD_SERVER = 'jud.localhost'
 
-def server_discovered(q, bus, conn, stream):
+def server_discovered(q, bus, conn, stream, server=None):
     iq_event, disco_event = q.expect_many(
         EventPattern('stream-iq', to=None, query_ns='vcard-temp',
             query_name='vCard'),
@@ -26,7 +27,7 @@ def server_discovered(q, bus, conn, stream):
 
     # no search server has been discovered yet. The CreateChannel operation
     # will be completed once the disco process is finished.
-    call_create(q, conn, server=None)
+    call_create(q, conn, server=server)
 
     # reply to IQ query
     reply = make_result_iq(stream, disco_event.stanza)
@@ -52,12 +53,12 @@ def server_discovered(q, bus, conn, stream):
     assertEquals(JUD_SERVER, properties[cs.CONTACT_SEARCH_SERVER])
 
     # Now that the search server has been discovered, it is used right away.
-    call_create(q, conn, server=None)
+    call_create(q, conn, server=server)
     ret, _ = answer_field_query(q, stream, JUD_SERVER)
     _, properties = ret.value
     assertEquals(JUD_SERVER, properties[cs.CONTACT_SEARCH_SERVER])
 
-def no_server_discovered(q, bus, conn, stream):
+def no_server_discovered(q, bus, conn, stream, server=None):
     iq_event, disco_event = q.expect_many(
         EventPattern('stream-iq', to=None, query_ns='vcard-temp',
             query_name='vCard'),
@@ -67,7 +68,7 @@ def no_server_discovered(q, bus, conn, stream):
 
     # no search server has been discovered yet. The CreateChannel operation
     # will fail once the disco process is finished.
-    call_create(q, conn, server=None)
+    call_create(q, conn, server=server)
 
     # reply to IQ query. No search server is present
     reply = make_result_iq(stream, disco_event.stanza)
@@ -78,11 +79,11 @@ def no_server_discovered(q, bus, conn, stream):
 
     # This server doesn't have a search server. We can't create Search channel
     # without specifying a Server property
-    call_create(q, conn, server=None)
+    call_create(q, conn, server=server)
     e = q.expect('dbus-error', method='CreateChannel')
     assertEquals(cs.INVALID_ARGUMENT, e.error.get_dbus_name())
 
-def disconnect_before_disco(q, bus, conn, stream):
+def disconnect_before_disco(q, bus, conn, stream, server=None):
     iq_event, disco_event = q.expect_many(
         EventPattern('stream-iq', to=None, query_ns='vcard-temp',
             query_name='vCard'),
@@ -92,7 +93,7 @@ def disconnect_before_disco(q, bus, conn, stream):
 
     # try to create a channel before the disco process is completed.
     # This creation will fail
-    call_create(q, conn, server=None)
+    call_create(q, conn, server=server)
 
     # connection is disconnected. CreateChannel fails
     disconnect_conn(q, conn, stream, [
@@ -102,3 +103,8 @@ if __name__ == '__main__':
     exec_test(server_discovered)
     exec_test(no_server_discovered)
     exec_test(disconnect_before_disco)
+    # We also test that Gabble does something sensible if you set Server to the
+    # empty string, rather than omitting it.
+    exec_test(partial(server_discovered, server=''))
+    exec_test(partial(no_server_discovered, server=''))
+    exec_test(partial(disconnect_before_disco, server=''))