Bug 39933 - Gabble advertises tube services as supported with just a Chan.Requested=true Client filter
Summary: Gabble advertises tube services as supported with just a Chan.Requested=true ...
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Olli Salli
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/og...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-08-08 08:35 UTC by Olli Salli
Modified: 2011-08-10 11:42 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Olli Salli 2011-08-08 08:35:22 UTC
1) I start my example tube initiator application for the new TpQt4 plug-in StreamTube API (#30584), which registers a Handler with


      dict entry(
         string "HandlerChannelFilter"
         variant             array [
               array [
                  dict entry(
                     string "org.freedesktop.Telepathy.Channel.ChannelType"
                     variant                         string "org.freedesktop.Telepathy.Channel.Type.StreamTube"
                  )
                  dict entry(
                     string "org.freedesktop.Telepathy.Channel.Requested"
                     variant                         boolean true
                  )
                  dict entry(
                     string "org.freedesktop.Telepathy.Channel.TargetHandleType"
                     variant                         uint32 1
                  )
                  dict entry(
                     string "org.freedesktop.Telepathy.Channel.Type.StreamTube.Service"
                     variant                         string "tp-qt4-stube-example"
                  )
               ]
            ]
         )

2) MC calls UpdateCapabilities on all connections with that filter verbatim as expected:

method call sender=:1.1354 -> dest=:1.1353 serial=155 path=/org/freedesktop/Telepathy/Connection/gabble/jabber/test2_40test_2ecollabora_2eco_2euk_2fda6967aa; interface=org.freedesktop.Telepathy.Connection.Interface.ContactCapabilities; member=UpdateCapabilities
   array [
      struct {
         string "org.freedesktop.Telepathy.Client.TpQt4STubeServer__1_1360_18f0410"
         array [
            array [
               dict entry(
                  string "org.freedesktop.Telepathy.Channel.Requested"
                  variant                      boolean true
               )
               dict entry(
                  string "org.freedesktop.Telepathy.Channel.ChannelType"
                  variant                      string "org.freedesktop.Telepathy.Channel.Type.StreamTube"
               )
               dict entry(
                  string "org.freedesktop.Telepathy.Channel.TargetHandleType"
                  variant                      uint64 1
               )
               dict entry(
                  string "org.freedesktop.Telepathy.Channel.Type.StreamTube.Service"
                  variant                      string "tp-qt4-stube-example"
               )
            ]
         ]
         array [
         ]
      }
   ]

3) Gabble gets all raved up

(telepathy-gabble:11783): gabble-DEBUG: gabble_connection_update_capabilities (connection.c:3170): enter
(telepathy-gabble:11783): gabble-DEBUG: gabble_private_tubes_factory_add_cap (private-tubes-factory.c:598): org.freedesktop.Telepathy.Client.TpQt4STubeServer__1_1358_e5a410: adding capability http://telepathy.freedesktop.org/xmpp/tubes/stream#tp-qt4-stube-example
(telepathy-gabble:11783): gabble-DEBUG: gabble_media_factory_add_caps (media-factory.c:1018): Client org.freedesktop.Telepathy.Client.TpQt4STubeServer__1_1358_e5a410 media capabilities:
(telepathy-gabble:11783): gabble-DEBUG: gabble_connection_update_capabilities (connection.c:3215): client org.freedesktop.Telepathy.Client.TpQt4STubeServer__1_1358_e5a410 contributes:
  --begin--
  Feature: http://telepathy.freedesktop.org/xmpp/tubes/stream#tp-qt4-stube-example
  --end--

(telepathy-gabble:11783): gabble-DEBUG: gabble_connection_refresh_capabilities (connection.c:2369): incorporating caps for org.freedesktop.Telepathy.Client.TpQt4STubeServer__1_1358_e5a410:
  --begin--
  Feature: http://telepathy.freedesktop.org/xmpp/tubes/stream#tp-qt4-stube-example
  --end--

4) Anybody in disco terms with the local Gabble account here sees http://telepathy.freedesktop.org/xmpp/tubes/stream#tp-qt4-stube-example as a supported capability, which would mean that they can now receive stream tubes with the service tp-qt4-stube-example

Similar issues might exist for other kinds of channels, but for stream tubes this is especially important, as the initiator (classically, a TCP server) and receiver (~TCP client) applications are often separate and not dependent on each other. With the current Gabble behavior, just installing the server application would make it appear that something usable as a client through a tube is available.

Of course, this could be fixed in MC by ignoring Requested=true channel classes from the UpdateCapabilities round, but that seems like a short-sighted hack; after all, as Will pointed out, on some protocols it might be perfectly sensible to advertise to the network e.g. that "this person here might call you" ( there is a Handler with a Requested=true, Type=Call class in its filters).

I'll cook a patch for Gabble - specifically the tube factories.
Comment 1 Olli Salli 2011-08-08 08:36:14 UTC
(In reply to comment #0)
> StreamTube API (#30584), which registers a Handler with

bug 35084, rather
Comment 2 Olli Salli 2011-08-10 08:26:12 UTC
done?
Comment 3 Simon McVittie 2011-08-10 08:54:45 UTC
> + if (tp_asv_get_boolean (cap, TP_IFACE_CHANNEL ".Requested", FALSE) == TRUE)

Ideally TP_PROP_CHANNEL_REQUESTED or whatever we called it. I'd prefer without the "== TRUE".

Could we have a regression test for a channel class that doesn't mention Requested (= matches both incoming and outgoing), just to be sure? I think your implementation will get that right too, though.
Comment 4 Olli Salli 2011-08-10 09:24:13 UTC
(In reply to comment #3)
> > + if (tp_asv_get_boolean (cap, TP_IFACE_CHANNEL ".Requested", FALSE) == TRUE)
> 
> Ideally TP_PROP_CHANNEL_REQUESTED or whatever we called it. I'd prefer without

Ok, ported the whole file to TP_PROP constants for consistency.

> the "== TRUE".

Done

> 
> Could we have a regression test for a channel class that doesn't mention
> Requested (= matches both incoming and outgoing), just to be sure? I think your
> implementation will get that right too, though.

We do now, and it did get it right.
Comment 5 Simon McVittie 2011-08-10 09:43:51 UTC
Looks good to me.

(In reply to comment #0)
> for stream tubes
> this is especially important, as the initiator (classically, a TCP server) and
> receiver (~TCP client) applications are often separate and not dependent on
> each other

In recent MC, you only need to add a { Requested = TRUE } filter if you want to be eligible to handle channels requested by others. If you are the PreferredHandler (as you ought to be for channels you request yourself), you will receive the channel anyway, and being a Handler with a zero-length list of filters (i.e. match nothing) is often the right thing...

(Note that there's a difference between HandlerChannelFilters = [], which matches nothing, and HandlerChannelFilters = [{}], which matches everything.)

> after all, as Will pointed out, on some protocols it might be perfectly
> sensible to advertise to the network e.g. that "this person here might call
> you" ( there is a Handler with a Requested=true, Type=Call class in its
> filters)

... although that usage would break the ability to do this.
Comment 6 Olli Salli 2011-08-10 11:42:01 UTC
(In reply to comment #5)
> Looks good to me.
> 

Ok thanks, will merge.

> (In reply to comment #0)
> > for stream tubes
> > this is especially important, as the initiator (classically, a TCP server) and
> > receiver (~TCP client) applications are often separate and not dependent on
> > each other
> 
> In recent MC, you only need to add a { Requested = TRUE } filter if you want to
> be eligible to handle channels requested by others. If you are the
> PreferredHandler (as you ought to be for channels you request yourself), you
> will receive the channel anyway, and being a Handler with a zero-length list of
> filters (i.e. match nothing) is often the right thing...
> 
> (Note that there's a difference between HandlerChannelFilters = [], which
> matches nothing, and HandlerChannelFilters = [{}], which matches everything.)

Sure. In TpQt4 we have the {create,ensure}AndHandle* APIs which work in exactly this fashion (create a behind-the-scenes Handler with an empty filter and a random name, pass it as the PreferredHandler). However, many interesting use cases (in KDE telepathy mainly) have the channels requested from an another application, most often the contact list. We've had some ideas to enable starting arbitrary kinds of sessions from the contact list, including tubes.

> 
> > after all, as Will pointed out, on some protocols it might be perfectly
> > sensible to advertise to the network e.g. that "this person here might call
> > you" ( there is a Handler with a Requested=true, Type=Call class in its
> > filters)
> 
> ... although that usage would break the ability to do this.

Yeah. Perhaps we should try and see whether there are protocols which actually support that, and try and make the spec steer people towards registering the Requested=true handlers anyway, if so.


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.