#997 XEP-0030: Service Discovery - Ordering of identities and features
Reporter
Syndace
Owner
Nobody
Created
Updated
Stars
★ (1)
Tags
Compliance
Status-WontFix
Type-Defect
Patch
Priority-Medium
Syndace
on
Link to the spec: https://xmpp.org/extensions/xep-0030.html
From section 11.1 disco#info (the XML schema):
<xs:element name='query'>
<xs:complexType>
<xs:sequence minOccurs='0'>
<xs:element ref='identity' maxOccurs='unbounded'/>
<xs:element ref='feature' maxOccurs='unbounded'/>
</xs:sequence>
<xs:attribute name='node' type='xs:string' use='optional'/>
</xs:complexType>
</xs:element>
Detailed issue description:
The given XML schema excerpt defines that identities must be listed before features.
Currently prosody might list some features before it lists identities, which results in schema validations to fail.
Waqas
on
Example stanza provided in jdev@:
<iq xmlns="jabber:client" to="me@host/Resource" type="result" id="dummy_#38499#-6" from="me@host">
<query xmlns="http://jabber.org/protocol/disco#info">
<feature var="urn:xmpp:mam:0" />
<identity category="pubsub" type="pep" />
<feature var="http://jabber.org/protocol/pubsub#publish" />
</query>
</iq>
The issue is the order in which different modules' event handlers get the disco#info reply stanza and add elements to it.
Waqas
on
Untested patch. This sorts the elements in mod_disco before sending. There is no burden on individual plugins to worry about sorting themselves.
diff -r ea2667fc6781 plugins/mod_disco.lua
--- a/plugins/mod_disco.lua Sun Sep 17 13:33:38 2017 -0400
+++ b/plugins/mod_disco.lua Sat Sep 23 10:24:08 2017 -0400
@@ -153,6 +153,26 @@
end
end);
+-- returns true when a is less than b (i.e., in correct order)
+-- works for stanza elements AND strings
+local function _sorter(a, b)
+ -- check for <identity/> first
+ if a.name == "identity" then return true ; end -- it's always first
+ if b.name == "identity" then return false; end -- and never last
+
+ -- check for <feature/> next
+ if a.name == "feature" then return true ; end -- it's always first
+ if b.name == "feature" then return false; end -- and never last
+
+ -- nothing else matters
+ return true;
+end
+function _sort_disco_info(stanza)
+ local info = stanza:get_child('query', 'http://jabber.org/protocol/disco#info');
+ table.sort(stanza, _sorter);
+ table.sort(stanza.tags, _sorter);
+end
+
-- Handle disco requests to user accounts
module:hook("iq/bare/http://jabber.org/protocol/disco#info:query", function(event)
local origin, stanza = event.origin, event.stanza;
@@ -167,6 +187,7 @@
local ret = module:fire_event("account-disco-info-node", node_event);
if ret ~= nil then return ret; end
if node_event.exists then
+ _sort_disco_info(reply);
origin.send(reply);
else
origin.send(st.error_reply(stanza, "cancel", "item-not-found", "Node does not exist"));
@@ -177,6 +198,7 @@
if not reply.attr.from then reply.attr.from = origin.username.."@"..origin.host; end -- COMPAT To satisfy Psi when querying own account
reply:tag('identity', {category='account', type='registered'}):up();
module:fire_event("account-disco-info", { origin = origin, reply = reply });
+ _sort_disco_info(reply);
origin.send(reply);
return true;
end
Zash
on
Wasn't consensus that the schemas are just unable to express what the normative text says, and the order of elements isn't important?
Changes
tags Patch
Zash
on
I don't think we'll do this for the above reasons.
Link to the spec: https://xmpp.org/extensions/xep-0030.html From section 11.1 disco#info (the XML schema): <xs:element name='query'> <xs:complexType> <xs:sequence minOccurs='0'> <xs:element ref='identity' maxOccurs='unbounded'/> <xs:element ref='feature' maxOccurs='unbounded'/> </xs:sequence> <xs:attribute name='node' type='xs:string' use='optional'/> </xs:complexType> </xs:element> Detailed issue description: The given XML schema excerpt defines that identities must be listed before features. Currently prosody might list some features before it lists identities, which results in schema validations to fail.
Example stanza provided in jdev@: <iq xmlns="jabber:client" to="me@host/Resource" type="result" id="dummy_#38499#-6" from="me@host"> <query xmlns="http://jabber.org/protocol/disco#info"> <feature var="urn:xmpp:mam:0" /> <identity category="pubsub" type="pep" /> <feature var="http://jabber.org/protocol/pubsub#publish" /> </query> </iq> The issue is the order in which different modules' event handlers get the disco#info reply stanza and add elements to it.
Untested patch. This sorts the elements in mod_disco before sending. There is no burden on individual plugins to worry about sorting themselves. diff -r ea2667fc6781 plugins/mod_disco.lua --- a/plugins/mod_disco.lua Sun Sep 17 13:33:38 2017 -0400 +++ b/plugins/mod_disco.lua Sat Sep 23 10:24:08 2017 -0400 @@ -153,6 +153,26 @@ end end); +-- returns true when a is less than b (i.e., in correct order) +-- works for stanza elements AND strings +local function _sorter(a, b) + -- check for <identity/> first + if a.name == "identity" then return true ; end -- it's always first + if b.name == "identity" then return false; end -- and never last + + -- check for <feature/> next + if a.name == "feature" then return true ; end -- it's always first + if b.name == "feature" then return false; end -- and never last + + -- nothing else matters + return true; +end +function _sort_disco_info(stanza) + local info = stanza:get_child('query', 'http://jabber.org/protocol/disco#info'); + table.sort(stanza, _sorter); + table.sort(stanza.tags, _sorter); +end + -- Handle disco requests to user accounts module:hook("iq/bare/http://jabber.org/protocol/disco#info:query", function(event) local origin, stanza = event.origin, event.stanza; @@ -167,6 +187,7 @@ local ret = module:fire_event("account-disco-info-node", node_event); if ret ~= nil then return ret; end if node_event.exists then + _sort_disco_info(reply); origin.send(reply); else origin.send(st.error_reply(stanza, "cancel", "item-not-found", "Node does not exist")); @@ -177,6 +198,7 @@ if not reply.attr.from then reply.attr.from = origin.username.."@"..origin.host; end -- COMPAT To satisfy Psi when querying own account reply:tag('identity', {category='account', type='registered'}):up(); module:fire_event("account-disco-info", { origin = origin, reply = reply }); + _sort_disco_info(reply); origin.send(reply); return true; end
Wasn't consensus that the schemas are just unable to express what the normative text says, and the order of elements isn't important?
ChangesI don't think we'll do this for the above reasons.
Changes