Recipient of a roster/subscription requests is not notified of that request’s cancellation

[michalrus]

trafficstars

What version of ejabberd are you using?

18.06.

What operating system (version) are you using?

NixOS 18.03.

How did you install ejabberd (source, package, distribution)?

Package.

What did not work as expected? Are there error messages in the log? What was the unexpected behavior? What was the expected result?

I’ve got two users, A and B. Now:

1. A sends a request to B (more/less, adds them to A’s roster and sends <presence to='B' type='subscribe'/>).

2. B is notified of that (receives <presence lang='en' to='B' from='A' type='subscribe'/>) but does nothing.

3. Now, A cancels that request (sends <presence to='B' type='unsubscribe'/> and <iq type='set' id='…'><query xmlns='jabber:iq:roster'><item jid='B' subscription='remove'/></query></iq>).

4. A’s roster doesn’t contain B anymore.

5. However, B is not notified in any way of the fact that the request was cancelled. — wrong?

I suppose 5. should not be happening. Or am I getting something wrong?

---

Also note that in 3. it should be enough to just remove B from A’s roster (2nd message), but then the subscription request would be resent to B after B reconnects.

[michalrus]

So, for example, Prosody, after 3., sends these 2 messages to B:

<presence xmlns='jabber:client' to='B' type='unsubscribe' from='A'/>

<iq xmlns='jabber:client' type='set' id='…'>
  <query xmlns='jabber:iq:roster' ver='2'>
    <item jid='A' subscription='remove'/>
  </query>
</iq>

And thanks to that B can know in 5. that the subscription request was cancelled.

[zinid]

I suppose 5. should not be happening. Or am I getting something wrong?

Did you check what RFC6121 says?

[michalrus]

@zinid yes, and this part (cancelling an unresponded request — called Pending out in that RFC) is not specified at all…

But it makes sense to notify the recipient’s client that the request is cancelled, like Prosody does, to not confuse the user, right? =)

[michalrus]

@zinid here’s the closest we’ll get to this issue probably: https://xmpp.org/rfcs/rfc6121.html#sub-cancel-outbound

[michalrus]

Also this part is a related sub-bug, IMO:

Also note that in 3. it should be enough to just remove B from A’s roster (2nd message), but then the subscription request would be resent to B after B reconnects.

So if I send only removal from roster (without <presence type="unsubscribe"/>), the request stays Pending In for B after B reconnects, which is rather wrong.

So, all in all, after A requests and cancels by just removing B from A’s roster:

• the subscription request from A to B should be cancelled (and never resent to B after B reconnects),
• B should be notified of the subscription from A being cancelled, like Prosody does.

That flow makes sense UX-wise and, I think, is left at the discretion of implementors in RFC-6121.

[badlop]

I've tried your steps with ejabberd 18.06, and I also look at the database content, the 'roster' table (I use Mnesia):

1. A sends a request to B (more/less, adds them to A’s roster and sends <presence to='B' type='subscribe'/>).

Yes, and ejabberd adds to the 'roster' table two entries:

• new entry: user A, contact B, subscription none, ask out
• new entry: user B, contact A, subscription none, ask in

Due to this second entry (the one with ask in), everytime user B logins, ejabberd will remember him that he has an INCOMING subscription request waiting to be replied. That is, step 2 repeats everytime user B logins, as you explained.

3. Now, A cancels that request (sends <presence to='B' type='unsubscribe'/>

When user A sends that presence, ejabberd makes two changes in the roster table:

• changed entry: user A, contact B, subscription none, ask out is changed to ask none.
• deleted entry: user B, contact A, subscription none, ask in

As the second entry is removed in the database, when user B logins the next time, ejabberd does not notify him anymore about any pending subscription.

So, I cannot reproduce the problem you mention. You can try to repeat your steps, this time looking at the database, to see if the entries are added and removed as I described.

[michalrus]

Hey, @badlop, thank you for taking time to investigate this.

You write:

As the second entry is removed in the database, when user B logins the next time, ejabberd does not notify him anymore about any pending subscription.

The issue is that at the very moment of A cancelling the request, B is not notified in real time of the cancellation. They can only know about the fact after they re-login.

It makes sense for B to be notified in real time, to update its UI etc.

E.g. compare with what messages Prosody sends instantly to B in my https://github.com/processone/ejabberd/issues/2598#issuecomment-419710547.

[badlop]

Aha. Play with this small patch and report your results:

diff --git a/src/mod_roster.erl b/src/mod_roster.erl
index 1f42b69e0..f5b8e6a19 100644
--- a/src/mod_roster.erl
+++ b/src/mod_roster.erl
@@ -623,6 +623,8 @@ process_subscription(Direction, User, Server, JID1,
 			    push_item(jid:make(User, Server), OldItem, NewItem)
 		    end,
 		    true;
+		none when (Direction == in) and (Type == unsubscribe) ->
+		    true;
 		none ->
 		    false
 	    end;

[michalrus]

@badlop very cool! With the patch, B is now getting:

<presence xmlns="jabber:client" xml:lang="en"
   to="B@chat/xmpp-poc-PHHPBS0MDVT"
   from="A@chat"
   type="subscribe"/>

… and after A cancels, B gets:

<presence xmlns="jabber:client" xml:lang="en"
   to="B@chat/xmpp-poc-PHHPBS0MDVT"
   from="A@chat"
   type="unsubscribe"/>

Thank you!

Prosody, besides this 2nd message, sends a roster update to B, telling them to remove A. Is this important?

[badlop]

Prosody, besides this 2nd message, sends a roster update to B, telling them to remove A. Is this important?

That seems user-friendly, because probably the user doesn't want that contact anymore. But what if the user wants to have the item in his roster, even if there is not presence subscription in any direction? If the protocol specifies nothing, I prefer to be strict: a presence unsubscription from A does not produce anything else than a presence unsubscription to B.

[michalrus]

I see. Thank you. =)

So perhaps your patch could go in into the next release?

[Neustradamus]

@badlop: Why have you done a revert?

@prefiks: What do you think?

[badlop]

@Neustradamus: because my initial fix added other problems, so better to revert it