1142142 - Implement service discovery (XEP-0030)

Status: RESOLVED FIXED

(Chat Core :: XMPP, defect)

Description

[Abdelrahman Ahmed [:abdelrahman]]

clients must advertise they support direct MUC invitations in response to information requests.

http://xmpp.org/extensions/xep-0249.html#support

Comment 1

[aleth [:aleth]]

It's possible service discovery is not needed if entity capabilities is implemented (which should be done first, as it is the preferred route).

Comment 2

[aleth [:aleth]]

We should also respond to requests like

<iq xmlns="jabber:client" id="453" type="get" to="[e-mail]/Instantbird" from="[e-mail]/Asterix">
<query xmlns="http://jabber.org/protocol/disco#info"/>
</iq>

Comment 3

[Abdelrahman Ahmed [:abdelrahman]]

We need also to determine support for Software Version (XEP-0092) [1]

[1] http://xmpp.org/extensions/xep-0092.html#disco

Comment 4

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: v1 - respond to service discovery queries

Comment 5

[aleth [:aleth]]

Comment on attachment 8773368 [details] [diff] [review]
v1 - respond to service discovery queries

Review of attachment 8773368 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/protocols/xmpp/xmpp.jsm
@@ +42,5 @@
>    return aTxt => cs.scanTXT(aTxt, cs.kEntities);
>  });
>  
> +// Features that we support in XMPP.
> +// TODO: Add missing or new features.

I don't think you need this as a TODO - just put "Don't forget to add your new features here."

@@ +51,5 @@
> +  Stanza.NS.last,
> +  Stanza.NS.vcard,
> +  Stanza.NS.ping,
> +  Stanza.NS.chatstates
> +];

Better order these alphabetically...

How about moving this to xmpp-xml.jsm together with the namespace stuff? When a new namespace is added, it will be easier to remember to add it as a feature too if needed. (Then you only need NS.*)

@@ +1721,5 @@
> +          Stanza.node("feature", null, {var: feature})
> +        );
> +
> +        let discoveryQuery = Stanza.node("query", Stanza.NS.disco_info, null,
> +                                       children);

indent, or inline children.

Comment 6

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: v2 - respond to service discovery queries

Comment 7

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: v3 - respond to service discovery queries

Order the list alphabetically.

Comment 8

[aleth [:aleth]]

Comment on attachment 8773380 [details] [diff] [review]
v2 - respond to service discovery queries

Review of attachment 8773380 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/protocols/xmpp/xmpp-xml.jsm
@@ +92,5 @@
>  };
>  
> +// Features that we support in XMPP.
> +// Don't forget to add your new features here.
> +var supportedFeatures = [

SupportedFeatures

@@ +107,4 @@
>  /* Stanza Builder */
>  var Stanza = {
>    NS: NS,
> +  supportedFeatures: supportedFeatures,

This has nothing to do with stanzas. Export it directly.

::: chat/protocols/xmpp/xmpp.jsm
@@ +1709,5 @@
> +          Stanza.node("feature", null, {var: feature})
> +        );
> +
> +        let discoveryQuery =
> +          Stanza.node("query", Stanza.NS.disco_info, null, children);

Do you need an identity node too?

Is there any difference between requests to MUC jids and requests to normal jids?

Do you have an example of a typical response from another client? (Just to compare the supported list)

Comment 9

[aleth [:aleth]]

Take a look at xep-0045.html 6.7 etc

Comment 10

[aleth [:aleth]]

Comment on attachment 8773382 [details] [diff] [review]
v3 - respond to service discovery queries

Review of attachment 8773382 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/protocols/xmpp/xmpp-xml.jsm
@@ +100,5 @@
> +  NS.last,
> +  NS.muc,
> +  NS.ping,
> +  NS.vcard,
> +  NS.version

How did you pick these? Is there a list of what to include in disco_info responses somewhere?

Comment 11

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: v4 - respond to service discovery queries

(In reply to aleth [:aleth] from comment #8)
> Do you need an identity node too?

Yes

> Is there any difference between requests to MUC jids and requests to normal
> jids?

I think xep-0045 (section 6.7) 

> Do you have an example of a typical response from another client? (Just to
> compare the supported list)

http://pastebin.instantbird.com/3244019

(In reply to aleth [:aleth] from comment #10)
> How did you pick these? Is there a list of what to include in disco_info
> responses somewhere?

I checked |xmpp.jsm| and get added them according to implemented methods.

Comment 12

[aleth [:aleth]]

(In reply to Abdelrhman Ahmed [:abdelrhman] from comment #11)
> Created attachment 8773425 [details] [diff] [review]
> (In reply to aleth [:aleth] from comment #10)
> > How did you pick these? Is there a list of what to include in disco_info
> > responses somewhere?
> 
> I checked |xmpp.jsm| and get added them according to implemented methods.

OK. 

How about asking on [e-mail] if it is not clear what namespaces are to be added?

Comment 13

[aleth [:aleth]]

Comment on attachment 8773425 [details] [diff] [review]
v4 - respond to service discovery queries

Review of attachment 8773425 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/protocols/xmpp/xmpp.jsm
@@ +1704,5 @@
>          return;
>        }
> +      if (query && query.uri == Stanza.NS.disco_info) {
> +        // XEP-0030: Service Discovery.
> +        // TODO: Handle room query in XEP-0045 (6.7).

You should at least return a blank query for this, the TODO will be to add the rooms.

Comment 14

[aleth [:aleth]]

(In reply to aleth [:aleth] from comment #12)
> How about asking on [e-mail] if it is not clear what
> namespaces are to be added?

This may be the current list https://github.com/xsf/registrar/blob/master/disco-features.xml

Comment 15

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: v5 - respond to service discovery queries

Comment 16

[Abdelrahman Ahmed [:abdelrahman]]

https://hg.mozilla.org/comm-central/rev/4bfbb09e3643