Closed Bug 1142142 Opened 5 years ago Closed 4 years ago

Implement service discovery (XEP-0030)

Categories

(Chat Core :: XMPP, defect)

x86_64
All
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 50

People

(Reporter: abdelrahman, Assigned: abdelrahman)

References

Details

Attachments

(1 file, 4 obsolete files)

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

http://xmpp.org/extensions/xep-0249.html#support
It's possible service discovery is not needed if entity capabilities is implemented (which should be done first, as it is the preferred route).
Summary: Implement service discovery information → Implement service discovery (XEP-0030)
We should also respond to requests like

<iq xmlns="jabber:client" id="453" type="get" to="c@ch3kr.net/Instantbird" from="jdev@conference.jabber.org/Asterix">
<query xmlns="http://jabber.org/protocol/disco#info"/>
</iq>
Depends on: 1010342
Assignee: nobody → a.ahmed1026
We need also to determine support for Software Version (XEP-0092) [1]

[1] http://xmpp.org/extensions/xep-0092.html#disco
Attachment #8773368 - Flags: review?(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.
Attachment #8773368 - Attachment is obsolete: true
Attachment #8773368 - Flags: review?(aleth)
Attachment #8773380 - Flags: review?(aleth)
Order the list alphabetically.
Attachment #8773380 - Attachment is obsolete: true
Attachment #8773380 - Flags: review?(aleth)
Attachment #8773382 - Flags: feedback?(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)
Attachment #8773380 - Attachment is obsolete: false
Take a look at xep-0045.html 6.7 etc
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?
(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.
Attachment #8773380 - Attachment is obsolete: true
Attachment #8773382 - Attachment is obsolete: true
Attachment #8773382 - Flags: feedback?(aleth)
Attachment #8773425 - Flags: review?(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 jdev@conference.jabber.org if it is not clear what namespaces are to be added?
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.
Attachment #8773425 - Flags: review?(aleth) → review-
(In reply to aleth [:aleth] from comment #12)
> How about asking on jdev@conference.jabber.org 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
Attachment #8773425 - Attachment is obsolete: true
Attachment #8773727 - Flags: review?(aleth)
Attachment #8773727 - Flags: review?(aleth) → review+
https://hg.mozilla.org/comm-central/rev/4bfbb09e3643
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 50
You need to log in before you can comment on or make changes to this bug.