plugin XEP-0115: Fix disco duplicate on caps node query + check own generation:
When caps node was requested, disco infos was returned from cache + infos from normal disco gathering, resulting in doubled items and other entities rejecting the disco info. This is fixed by subclassing the disco handler to add a trigger. This trigger is used by XEP-0115 to short circuit the disco gathering and returned its cached data instead. Our own hash is re-generated on each connection, and validated, so if there is a bug, this helps see and catch it.
This commit is contained in:
parent
58834f15ae
commit
18d3f0167f
3 changed files with 109 additions and 47 deletions
|
|
@ -383,7 +383,7 @@ class SatXMPPEntity(core_types.SatXMPPEntity):
|
|||
entity.identityHandler = SatIdentityHandler()
|
||||
entity.identityHandler.setHandlerParent(entity)
|
||||
|
||||
entity.discoHandler = disco.DiscoHandler()
|
||||
entity.discoHandler = LiberviaDiscoHandler()
|
||||
entity.discoHandler.setHandlerParent(entity)
|
||||
|
||||
log.debug(_("setting plugins parents"))
|
||||
|
|
@ -2290,8 +2290,7 @@ class SatPresenceProtocol(xmppim.PresenceClientProtocol):
|
|||
server_jid = jid.JID(from_jid.host)
|
||||
self.host.memory.disco._jid_disco_cache[server_jid] = (None, time.time())
|
||||
log.debug(
|
||||
f"Server {server_jid} marked as unreachable from "
|
||||
"presence error"
|
||||
f"Server {server_jid} marked as unreachable from presence error"
|
||||
)
|
||||
except Exception:
|
||||
log.exception("Can't handle presence error")
|
||||
|
|
@ -2436,3 +2435,26 @@ class SatIdentityHandler(XMPPHandler):
|
|||
|
||||
def getDiscoItems(self, requestor, target, nodeIdentifier=""):
|
||||
return []
|
||||
|
||||
|
||||
class LiberviaDiscoHandler(disco.DiscoHandler):
|
||||
|
||||
def info(
|
||||
self,
|
||||
requestor: jid.JID | None,
|
||||
target: jid.JID | None,
|
||||
nodeIdentifier: str,
|
||||
) -> defer.Deferred[list]:
|
||||
"""DiscoHandler.info with a trigger.
|
||||
|
||||
The trigger let plugins modify disco behaviour, notably used by
|
||||
``plugin_xep_0115`` to return cached disco infos.
|
||||
"""
|
||||
cont, trigger_ret = G.host.trigger.return_point(
|
||||
"disco_info", requestor, target, nodeIdentifier
|
||||
)
|
||||
if not cont:
|
||||
trigger_ret = cast(defer.Deferred[list], trigger_ret)
|
||||
return trigger_ret
|
||||
|
||||
return super().info(requestor, target, nodeIdentifier)
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@
|
|||
from base64 import b64encode
|
||||
from dataclasses import dataclass, field
|
||||
from hashlib import sha1
|
||||
from typing import TYPE_CHECKING
|
||||
from typing import TYPE_CHECKING, Literal
|
||||
|
||||
from twisted.internet import defer, error
|
||||
from twisted.words.protocols.jabber import jid
|
||||
|
|
@ -30,6 +30,7 @@ from tx_xmpp import data_form, disco, itx_xmpp
|
|||
from zope.interface import implementer
|
||||
|
||||
from libervia.backend import G
|
||||
from libervia.backend.core import exceptions
|
||||
from libervia.backend.core.constants import Const as C
|
||||
from libervia.backend.core.core_types import SatXMPPEntity
|
||||
from libervia.backend.core.i18n import _
|
||||
|
|
@ -97,14 +98,20 @@ class XEP_0115:
|
|||
self.host = host
|
||||
log.info(_("Plugin XEP_0115 initialization"))
|
||||
host.trigger.add("Presence send", self._presence_trigger)
|
||||
host.trigger.add("disco_info", self._disco_info_trigger)
|
||||
|
||||
def get_handler(self, client: SatXMPPEntity) -> "XEP_0115_handler":
|
||||
return XEP_0115_handler(self)
|
||||
|
||||
async def _prepare_caps(self, client: SatXMPPEntity) -> None:
|
||||
# we have to calculate hash for client
|
||||
# because disco infos/identities may change between clients
|
||||
"""Generate our own capability hash for a client.
|
||||
|
||||
Hash is generated per client instance as disco infos/identities may change
|
||||
(plugins loaded/unloaded, config changes, etc.).
|
||||
|
||||
@param client: the client entity to generate the hash for.
|
||||
@raises exceptions.InternalError: The client's own disco info is malformed.
|
||||
"""
|
||||
data = client.get_plugin_data(IMPORT_NAME, CapsClientData)
|
||||
|
||||
# optimize check
|
||||
|
|
@ -115,19 +122,23 @@ class XEP_0115:
|
|||
else:
|
||||
log.warning(_("Caps optimisation not available"))
|
||||
|
||||
# hash generation
|
||||
assert data.caps_hash is None, "caps_hash already set"
|
||||
old_hash = data.caps_hash
|
||||
_infos = await client.discoHandler.info(client.jid, client.jid, "")
|
||||
disco_infos = disco.DiscoInfo()
|
||||
for item in _infos:
|
||||
disco_infos.append(item)
|
||||
cap_hash = self.generate_hash(disco_infos)
|
||||
data.caps_hash = cap_hash
|
||||
log.info(
|
||||
"Our capability hash has been generated: [{cap_hash}]".format(
|
||||
cap_hash=cap_hash
|
||||
|
||||
# Validate our own disco info before hashing.
|
||||
if not self._validate_caps(client.jid, "", "", disco_infos):
|
||||
raise exceptions.InternalError(
|
||||
"Our own disco#info response is malformed. Caps cannot be generated."
|
||||
)
|
||||
)
|
||||
|
||||
cap_hash = self.generate_hash(disco_infos)
|
||||
if cap_hash != old_hash:
|
||||
log.info(f"Capability hash changed: [{old_hash}] -> [{cap_hash}]")
|
||||
data.caps_hash = cap_hash
|
||||
log.info(f"Our capability hash has been generated: [{cap_hash}]")
|
||||
log.debug("Generating capability domish.Element")
|
||||
c_elt = domish.Element((NS_ENTITY_CAPABILITY, "c"))
|
||||
c_elt["hash"] = "sha-1"
|
||||
|
|
@ -142,6 +153,34 @@ class XEP_0115:
|
|||
client, client.jid, C.ENTITY_CAP_HASH, cap_hash
|
||||
)
|
||||
|
||||
def _disco_info_trigger(
|
||||
self,
|
||||
requestor: jid.JID | None,
|
||||
target: jid.JID | None,
|
||||
nodeIdentifier: str,
|
||||
) -> tuple[Literal[True], None] | tuple[Literal[False], defer.Deferred[list]]:
|
||||
"""Callback for the ``disco_info`` trigger.
|
||||
|
||||
Intercepts caps node queries to return the cached disco info directly.
|
||||
|
||||
@param requestor: entity that sent the disco request
|
||||
@param target: entity the disco request was sent to
|
||||
@param nodeIdentifier: optional node being queried
|
||||
@return: ``(False, deferred)`` with cached disco items for known caps queries, or
|
||||
``(True, None)`` to let normal flow continue
|
||||
"""
|
||||
# Check if this is a caps node query (format: node#ver)
|
||||
if nodeIdentifier and "#" in nodeIdentifier:
|
||||
parts = nodeIdentifier.rsplit("#", 1)
|
||||
if len(parts) == 2:
|
||||
caps_node, cap_ver = parts
|
||||
# Only respond if this matches our caps node
|
||||
if caps_node == C.APP_URL and cap_ver in G.host.memory.disco.hashes:
|
||||
disco_infos = G.host.memory.disco.hashes[cap_ver]
|
||||
# Return all items from the cached disco info
|
||||
return False, defer.succeed(list(disco_infos))
|
||||
return True, None
|
||||
|
||||
def _presence_add_elt(self, client: SatXMPPEntity, obj: domish.Element) -> None:
|
||||
data = client.get_plugin_data(IMPORT_NAME, CapsClientData)
|
||||
if data.caps_optimize:
|
||||
|
|
@ -164,6 +203,7 @@ class XEP_0115:
|
|||
)
|
||||
|
||||
presence_d.addCallback(lambda __: self._presence_add_elt(client, obj))
|
||||
presence_d.addErrback(G.host.log_errback)
|
||||
return True
|
||||
|
||||
def _update(self, presence_elt: domish.Element, client: SatXMPPEntity) -> None:
|
||||
|
|
@ -179,7 +219,10 @@ class XEP_0115:
|
|||
@param presence_elt: <presence/> element to parse.
|
||||
"""
|
||||
from_jid = jid.JID(presence_elt["from"])
|
||||
c_elem = next(presence_elt.elements(NS_ENTITY_CAPABILITY, "c"))
|
||||
c_elem = next(presence_elt.elements(NS_ENTITY_CAPABILITY, "c"), None)
|
||||
if c_elem is None:
|
||||
# Presence without caps, nothing to do
|
||||
return
|
||||
try:
|
||||
c_ver = c_elem["ver"]
|
||||
c_hash = c_elem["hash"]
|
||||
|
|
@ -422,9 +465,7 @@ class XEP_0115_handler(XMPPHandler):
|
|||
|
||||
def _caps_node_eb(self, failure_, server_jid, ver):
|
||||
"""Errback for caps node query."""
|
||||
log.warning(
|
||||
f"Failed to query caps node for {server_jid} (ver={ver}): {failure_}"
|
||||
)
|
||||
log.warning(f"Failed to query caps node for {server_jid} (ver={ver}): {failure_}")
|
||||
|
||||
def connectionInitialized(self) -> None:
|
||||
assert self.xmlstream is not None
|
||||
|
|
@ -449,10 +490,10 @@ class XEP_0115_handler(XMPPHandler):
|
|||
else:
|
||||
# Query the caps node to get disco info (XEP-0115 §6.2)
|
||||
caps_node = f"{node}#{ver}" if node else ver
|
||||
log.debug(
|
||||
f"Querying caps node {caps_node} for server {server_jid}"
|
||||
log.debug(f"Querying caps node {caps_node} for server {server_jid}")
|
||||
d = self.client.disco.requestInfo(
|
||||
server_jid, nodeIdentifier=caps_node
|
||||
)
|
||||
d = self.client.disco.requestInfo(server_jid, nodeIdentifier=caps_node)
|
||||
d.addCallback(self._caps_node_cb, ver)
|
||||
d.addErrback(self._caps_node_eb, server_jid, ver)
|
||||
|
||||
|
|
@ -479,19 +520,6 @@ class XEP_0115_handler(XMPPHandler):
|
|||
@param nodeIdentifier: The optional node being queried.
|
||||
@return: List of disco items (features, identities, extensions).
|
||||
"""
|
||||
# Check if this is a caps node query (format: node#ver)
|
||||
if nodeIdentifier and "#" in nodeIdentifier:
|
||||
parts = nodeIdentifier.rsplit("#", 1)
|
||||
if len(parts) == 2:
|
||||
caps_node, cap_ver = parts
|
||||
# Only respond if this matches our caps node
|
||||
if (
|
||||
caps_node == C.APP_URL
|
||||
and cap_ver in self.plugin_parent.host.memory.disco.hashes
|
||||
):
|
||||
disco_infos = self.plugin_parent.host.memory.disco.hashes[cap_ver]
|
||||
# Return all items from the cached disco info
|
||||
return list(disco_infos)
|
||||
|
||||
# For non-caps queries, return our caps features
|
||||
return [
|
||||
|
|
|
|||
|
|
@ -146,10 +146,8 @@ class TestXEP0115Handler:
|
|||
assert disco.DiscoFeature(NS_ENTITY_CAPABILITY) in result
|
||||
assert disco.DiscoFeature(NS_CAPS_OPTIMIZE) in result
|
||||
|
||||
def test_getDiscoInfo_caps_node_query(self, host, xep_0115):
|
||||
"""getDiscoInfo returns cached disco info for caps node queries."""
|
||||
handler = xep_0115.get_handler(MagicMock())
|
||||
|
||||
def test_disco_info_trigger_caps_node_query(self, host, xep_0115):
|
||||
"""_disco_info_trigger returns cached disco info for caps node queries."""
|
||||
# Create and cache some disco info
|
||||
disco_infos = disco.DiscoInfo()
|
||||
disco_infos.append(disco.DiscoIdentity("client", "pc", "Test Client"))
|
||||
|
|
@ -161,9 +159,14 @@ class TestXEP0115Handler:
|
|||
|
||||
# Query with caps node format
|
||||
node_identifier = f"{C.APP_URL}#{cap_hash}"
|
||||
result = handler.getDiscoInfo(MagicMock(), MagicMock(), node_identifier)
|
||||
cont, deferred = xep_0115._disco_info_trigger(
|
||||
MagicMock(), MagicMock(), node_identifier
|
||||
)
|
||||
|
||||
# Should return the cached disco info items
|
||||
# Should not continue, return cached disco info
|
||||
assert cont is False
|
||||
assert deferred is not None
|
||||
result = deferred.result
|
||||
assert len(result) > 0
|
||||
assert any(isinstance(item, disco.DiscoIdentity) for item in result)
|
||||
assert any(isinstance(item, disco.DiscoFeature) for item in result)
|
||||
|
|
@ -177,17 +180,26 @@ class TestXEP0115Handler:
|
|||
assert disco.DiscoFeature("http://jabber.org/protocol/caps") in features
|
||||
assert disco.DiscoFeature("http://jabber.org/protocol/disco#info") in features
|
||||
|
||||
def test_getDiscoInfo_unknown_caps_node(self, xep_0115):
|
||||
"""getDiscoInfo returns caps features for unknown caps nodes."""
|
||||
handler = xep_0115.get_handler(MagicMock())
|
||||
|
||||
def test_disco_info_trigger_unknown_caps_node(self, xep_0115):
|
||||
"""_disco_info_trigger lets unknown caps nodes pass through."""
|
||||
# Query with unknown hash
|
||||
node_identifier = f"{C.APP_URL}#unknownhash123"
|
||||
result = handler.getDiscoInfo(MagicMock(), MagicMock(), node_identifier)
|
||||
cont, deferred = xep_0115._disco_info_trigger(
|
||||
MagicMock(), MagicMock(), node_identifier
|
||||
)
|
||||
|
||||
# Should return just the caps features, not cached info
|
||||
assert disco.DiscoFeature(NS_ENTITY_CAPABILITY) in result
|
||||
assert disco.DiscoFeature(NS_CAPS_OPTIMIZE) in result
|
||||
# Should continue, let normal flow proceed
|
||||
assert cont is True
|
||||
assert deferred is None
|
||||
|
||||
def test_disco_info_trigger_non_caps_node(self, xep_0115):
|
||||
"""_disco_info_trigger ignores non-caps nodes."""
|
||||
cont, deferred = xep_0115._disco_info_trigger(
|
||||
MagicMock(), MagicMock(), "some:other:node"
|
||||
)
|
||||
|
||||
assert cont is True
|
||||
assert deferred is None
|
||||
|
||||
def test_getDiscoItems_empty(self, xep_0115):
|
||||
"""getDiscoItems returns empty list (caps nodes are virtual)."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue