Closed Bug 1142142 Opened 10 years ago Closed 8 years ago

Implement service discovery (XEP-0030)

Categories

(Chat Core :: XMPP, defect)

x86_64
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 50

People

(Reporter: abdelrahman, Assigned: abdelrahman)

References

(Blocks 1 open bug)

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
Attached patch v1 - respond to service discovery queries (obsolete) (deleted) — Splinter Review
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.
Attached patch v2 - respond to service discovery queries (obsolete) (deleted) — Splinter Review
Attachment #8773368 - Attachment is obsolete: true
Attachment #8773368 - Flags: review?(aleth)
Attachment #8773380 - Flags: review?(aleth)
Attached patch v3 - respond to service discovery queries (obsolete) (deleted) — Splinter Review
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?
Attached patch v4 - respond to service discovery queries (obsolete) (deleted) — Splinter Review
(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+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 50
Blocks: 1616923
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: