Issue with AMP when enabling message carbon (#451)

Closed

Activities

Artur Hefczyc opened 9 years ago

Due Date
2015-03-30

Problem described in message#4690

Andrzej Wójcik (Tigase) commented 9 years ago

I changed way of processing stanza by AMP so now they work properly with Message Carbons feature.

However I think there may be still issue with to attribute of amp element as XEP-0079 AMP specifies that it must be full jid of recipient of message:

The 'to' attribute specifies the original (intended) recipient of the containing stanza. This attribute MUST be specified for any stanza sent from a supporting server, > regardless of the recipient. It SHOULD NOT be specified otherwise. The value of the 'to' attribute MUST be the full JID (node@domain/resource) of the intended recipient for the original stanza.

but Tigase XMPP Sever deliveres messages sent to bare jid to every connected resource, so we cannot return particular resource as we should list more than one and XEP-0079 do not defined a way to do this.

We could change behavior to sent multiple notifications for one message which was delivered to but this would also be not desired as XEP-0079 states that:

The value of the 'to' attribute MUST be the full JID (node@domain/resource) of the intended recipient for the original stanza

but in our case intended recipient do not have full jid only bare one.

Due to that I left to attribute as bare jid in this case.

Example of AMP notification sent to sender:

<message xmlns="jabber:client" id="1427201906165" from="192.168.1.94" to="550bc7a38acd14171215f3cb@192.168.1.94/opsbus_1427201770886" isofflinemsg="false" type="chat">
        <body>Test mesage</body>
        <custom msgid="534870c5-fe61-4359-b3b4-2d6ea16edbd6" />
        <active xmlns="http://jabber.org/protocol/chatstates" />
        <request xmlns="urn:xmpp:receipts" />
        <amp xmlns="http://jabber.org/protocol/amp" status="notify" from="550bc7a38acd14171215f3cb@192.168.1.94/opsbus_1427201770886" to="550bc7b08acd14171215f3ce@192.168.1.94">
            <rule condition="deliver" action="notify" value="direct" />
        </amp>
    </message>

Artur Hefczyc commented 9 years ago

I used to have packed (message) delivery logic which selected one resource based on the following criteria:

The logic first attempted to find a resource with the highest priority
If all resources had the same priority it picked up the most recent active resource

This way we could have a full JID selected somehow. What do you think? From the practical point of view it seems to make sense.

Andrzej Wójcik (Tigase) commented 9 years ago

As for RFC 6121 event now we are properly routing message stanzas:

If there is more than one resource with a non-negative presence priority then the server MUST either (a) deliver the message to the "most available" resource or resources (according to the server's implementation-specific algorithm, e.g., treating the resource or resources with the highest presence priority as "most available") or (b) deliver the message to all of the non-negative resources that have opted in to receive chat messages.

as our implementation matches (b).

I would say that we could make this change but then we would need to change routing for every message stanza (not only AMP related stanza) so that our way of routing packets will be always the same. I think I should also mark that this change may be difficult to implement in clustered environment using default clustering strategy as we would deliver message sent to bare jid to every highest priority resource on each cluster node which would also be not a good solution.

Also this would be rather huge change for every one using Tigase XMPP Server right now.

From my point of view we should keep sending bare jid as to attribute of "notification" AMP element as intended recipient was in this case bare jid and not full jid or if really needed then maybe we should sent notification for every resource for which we deliver this message, but I would be careful with change of logic as I think that many our usages may depend on current way of handling messages sent to bare jid (ie. clients now receive messages sent to bare jid if more than one are connected for same user without usage of Message Carbons, which will change if we change routing logic. Same change would also be done for PubSub notifications and so on).

Artur Hefczyc commented 9 years ago

Ok.

Type	Bug
Priority	Normal
Assignee	Artur Hefczyc
RedmineID	2877
Version	tigase-server-7.1.0
Spent time	66h

Issue Votes (0)

Watchers (0)

Reference

tigase/_server/server-core#451