Bug 32390 - Creating a search channel on a "" (empty string) server fails without an error
Summary: Creating a search channel on a "" (empty string) server fails without an error
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/wj...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2010-12-14 07:17 UTC by Emilio Pozuelo Monfort
Modified: 2011-02-02 08:31 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Emilio Pozuelo Monfort 2010-12-14 07:17:34 UTC
I was opening a search channel with the server set to "", resulting on my callback not being called. The gabble debug shows:


* iq xmlns='jabber:client' type='get' to='' id='8122680337'
    * query xmlns='jabber:iq:search'
gabble/connection-DEBUG: 14/12/10 15:12:03.80893: gabble_connection_update_capabilities: enter
gabble/media-channel-DEBUG: 14/12/10 15:12:03.80957: gabble_media_factory_add_caps: Client org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e206.n1 media capabilities:
gabble/connection-DEBUG: 14/12/10 15:12:03.81010: gabble_connection_update_capabilities: client org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e206.n1 has no interesting capabilities
gabble/presence-DEBUG: 14/12/10 15:12:03.81083: gabble_presence_set_capabilities: about to add caps to resource 5ab67c28 with serial 18
gabble/presence-DEBUG: 14/12/10 15:12:03.81135: gabble_presence_set_capabilities: found resource 5ab67c28
gabble/presence-DEBUG: 14/12/10 15:12:03.81182: gabble_presence_set_capabilities: new serial 18, old 17, clearing caps
gabble/presence-DEBUG: 14/12/10 15:12:03.81233: gabble_presence_set_capabilities: updating caps for resource 5ab67c28
gabble/connection-DEBUG: 14/12/10 15:12:03.81284: gabble_connection_refresh_capabilities: nothing to do
wocky-DEBUG: 14/12/10 15:12:03.86035: _end_element_ns: Received stanza
* iq xmlns='jabber:client' from='emilio.pozuelo@collabora.co.uk' to='emilio.pozuelo@collabora.co.uk/5ab67c28' type='error' id='8122680337'
    * query xmlns='jabber:iq:search'
    * error code='503' type='cancel'
        * service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'
wocky-DEBUG: 14/12/10 15:12:03.86137: check_spoofing: wocky-porter.c:946: 'emilio.pozuelo@collabora.co.uk' (normal: 'emilio.pozuelo@collabora.co.uk') attempts to spoof an IQ reply from ''
wocky-DEBUG: 14/12/10 15:12:03.86184: check_spoofing: wocky-porter.c:948: Our full JID is 'emilio.pozuelo@collabora.co.uk/5ab67c28' and our bare JID is 'emilio.pozuelo@collabora.co.uk'
gabble/connection-DEBUG: 14/12/10 15:12:03.86256: connection_iq_unknown_cb: got unknown iq:
* iq xmlns='jabber:client' from='emilio.pozuelo@collabora.co.uk' to='emilio.pozuelo@collabora.co.uk/5ab67c28' type='error' id='8122680337'
    * query xmlns='jabber:iq:search'
    * error code='503' type='cancel'
        * service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'


Notice the to=''.


15:08 <       wjt> pochu: the bug in gabble is that it should handle the server being set to "" specially
Comment 1 Will Thompson 2011-01-24 02:57:32 UTC
Here's a fix.

I believe the client should omit Server entirely rather than setting it to the empty string; but Gabble should also be more tolerant.

This branch also deals with clients passing invalid JIDs as the server.
Comment 2 Emilio Pozuelo Monfort 2011-01-24 03:35:22 UTC
Looks good to me, except the first commit, which I can't parse (my Python sucks). If you're confident about it, just go for it.
Comment 3 Jonny Lamb 2011-01-24 03:49:07 UTC
Yes the first commit is confusing to me too.

-    call_create(q, conn, server=None)
-    answer_field_query(q, stream, JUD_SERVER)
+    call_create(q, conn, server=server)
+    ret, _ = answer_field_query(q, stream, JUD_SERVER)

This looks wrong in this context alone and looking at the commit message, but actually am I right in thinking you changed the test and then committed patch-by-patch? I don't think "server" has been set by now and so this code belongs in the top commit, no?

If so, then it'd be nice if you just changed "server=server" back to "server=None" so that test at that point doesn't fail (so we don't have broken bisects).

But maybe I'm wrong?!

Other than that it looks good to me too.
Comment 4 Will Thompson 2011-01-24 06:16:46 UTC
(In reply to comment #3)
> If so, then it'd be nice if you just changed "server=server" back to
> "server=None" so that test at that point doesn't fail (so we don't have broken
> bisects).
> 
> But maybe I'm wrong?!

Good catch. I've rebased my mistake out of existence.
Comment 5 Jonny Lamb 2011-01-25 02:06:06 UTC
+            cs.CONTACT_SEARCH_SERVER: 'this is literally bullshit',

I love it.
Comment 6 Will Thompson 2011-02-02 08:31:21 UTC
This was merged, and will be fixed in 0.11.7.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.